recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

In Git, always "sudo" to the daemon user if a daemon user is configured

Summary:
See T13673. Recent versions of Git (and older versions with backported security patches) now refuse to run Git commands if the top-level repository directory is not owned by the user running the command.

Currently, we "sudo" to that user only when performing writes, so upgrading Git can aggressively break a Phabricator system by knocking out essentially all Diffusion/Conduit read pathways.

As an immediate mitigation, just "sudo" in all cases where a daemon user is available. This fixes the problem, and seems like the least-bad approach. The downside is that the web user may theoretically have fewer privileges than the daemon user and this could reduce the number of layers an attacker armed with some other Git vulnerability might have to get through to do something dangerous (e.g., perform a write on a pathway where only reads are expected), but any separation between the web and daemon accounts is essentially theoretical and has never been enforced.

Test Plan: Applied patch to impacted Phacility shard, saw Diffusion work properly again.

Differential Revision: https://secure.phabricator.com/D21756

epriestley 3125d7a5 4dae3e7e

+19 -1
+5 -1
src/applications/diffusion/protocol/DiffusionCommandEngine.php
··· 117 117 return $this->sudoAsDaemon; 118 118 } 119 119 120 + protected function shouldAlwaysSudo() { 121 + return false; 122 + } 123 + 120 124 public function newFuture() { 121 125 $argv = $this->newCommandArgv(); 122 126 $env = $this->newCommandEnvironment(); 123 127 $is_passthru = $this->getPassthru(); 124 128 125 - if ($this->getSudoAsDaemon()) { 129 + if ($this->getSudoAsDaemon() || $this->shouldAlwaysSudo()) { 126 130 $command = call_user_func_array('csprintf', $argv); 127 131 $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command); 128 132 $argv = array('%C', $command);
+14
src/applications/diffusion/protocol/DiffusionGitCommandEngine.php
··· 13 13 return array($pattern, $argv); 14 14 } 15 15 16 + protected function shouldAlwaysSudo() { 17 + 18 + // See T13673. In Git, always try to use "sudo" to execute commands as the 19 + // daemon user (if such a user is configured), because Git 2.35.2 and newer 20 + // (and some older versions of Git with backported security patches) refuse 21 + // to execute if the top level repository directory is not owned by the 22 + // current user. 23 + 24 + // Previously, we used "sudo" only when performing writes to the 25 + // repository directory. 26 + 27 + return true; 28 + } 29 + 16 30 protected function newCustomEnvironment() { 17 31 $env = array(); 18 32