mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-23 07:12:41 +01:00
Add "phd.user" with sudo
hooks for SSH/HTTP writes
Summary: Ref T2230. When fully set up, we have up to three users who all need to write into the repositories: - The webserver needs to write for HTTP receives. - The SSH user needs to write for SSH receives. - The daemons need to write for "git fetch", "git clone", etc. These three users don't need to be different, but in practice they are often not likely to all be the same user. If for no other reason, making them all the same user requires you to "git clone httpd@host.com", and installs are likely to prefer "git clone git@host.com". Using three different users also allows better privilege separation. Particularly, the daemon user can be the //only// user with write access to the repositories. The webserver and SSH user can accomplish their writes through `sudo`, with a whitelisted set of commands. This means that even if you compromise the `ssh` user, you need to find a way to escallate from there to the daemon user in order to, e.g., write arbitrary stuff into the repository or bypass commit hooks. This lays some of the groundwork for a highly-separated configuration where the SSH and HTTP users have the fewest privileges possible and use `sudo` to interact with repositories. Some future work which might make sense: - Make `bin/phd` respect this (require start as the right user, or as root and drop privileges, if this configuration is set). - Execute all `git/hg/svn` commands via sudo? Users aren't expected to configure this yet so I haven't written any documentation. Test Plan: Added an SSH user ("dweller") and gave it sudo by adding this to `/etc/sudoers`: dweller ALL=(epriestley) SETENV: NOPASSWD: /usr/bin/git-upload-pack, /usr/bin/git-receive-pack Then I ran git pushes and pulls over SSH via "dweller@localhost". They successfully interacted with the repository on disk as the "epriestley" user. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2230 Differential Revision: https://secure.phabricator.com/D7589
This commit is contained in:
parent
40c0e3529d
commit
476b27d9c8
8 changed files with 64 additions and 12 deletions
|
@ -41,6 +41,14 @@ final class PhabricatorPHDConfigOptions
|
||||||
"of output, but can help debug issues. Daemons launched in debug ".
|
"of output, but can help debug issues. Daemons launched in debug ".
|
||||||
"mode with 'phd debug' are always launched in verbose mode. See ".
|
"mode with 'phd debug' are always launched in verbose mode. See ".
|
||||||
"also 'phd.trace'.")),
|
"also 'phd.trace'.")),
|
||||||
|
$this->newOption('phd.user', 'string', null)
|
||||||
|
->setSummary(pht("System user to run daemons as."))
|
||||||
|
->setDescription(
|
||||||
|
pht(
|
||||||
|
"Specify a system user to run the daemons as. Primarily, this ".
|
||||||
|
"user will own the working copies of any repositories that ".
|
||||||
|
"Phabricator imports or manages. This option is new and ".
|
||||||
|
"experimental.")),
|
||||||
$this->newOption('phd.trace', 'bool', false)
|
$this->newOption('phd.trace', 'bool', false)
|
||||||
->setBoolOptions(
|
->setBoolOptions(
|
||||||
array(
|
array(
|
||||||
|
|
|
@ -326,7 +326,10 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
|
|
||||||
$input = PhabricatorStartup::getRawInput();
|
$input = PhabricatorStartup::getRawInput();
|
||||||
|
|
||||||
list($err, $stdout, $stderr) = id(new ExecFuture('%s', $bin))
|
$command = csprintf('%s', $bin);
|
||||||
|
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
|
||||||
|
|
||||||
|
list($err, $stdout, $stderr) = id(new ExecFuture('%C', $command))
|
||||||
->setEnv($env, true)
|
->setEnv($env, true)
|
||||||
->write($input)
|
->write($input)
|
||||||
->resolve();
|
->resolve();
|
||||||
|
@ -422,7 +425,10 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
$input = strlen($input)."\n".$input."0\n";
|
$input = strlen($input)."\n".$input."0\n";
|
||||||
}
|
}
|
||||||
|
|
||||||
list($err, $stdout, $stderr) = id(new ExecFuture('%s serve --stdio', $bin))
|
$command = csprintf('%s serve --stdio', $bin);
|
||||||
|
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
|
||||||
|
|
||||||
|
list($err, $stdout, $stderr) = id(new ExecFuture('%C', $command))
|
||||||
->setEnv($env, true)
|
->setEnv($env, true)
|
||||||
->setCWD($repository->getLocalPath())
|
->setCWD($repository->getLocalPath())
|
||||||
->write("{$cmd}\n{$args}{$input}")
|
->write("{$cmd}\n{$args}{$input}")
|
||||||
|
@ -545,5 +551,6 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
|
|
||||||
return $has_pack && $is_hangup;
|
return $has_pack && $is_hangup;
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -22,9 +22,10 @@ final class DiffusionSSHGitReceivePackWorkflow
|
||||||
// This is a write, and must have write access.
|
// This is a write, and must have write access.
|
||||||
$this->requireWriteAccess();
|
$this->requireWriteAccess();
|
||||||
|
|
||||||
$future = new ExecFuture(
|
$command = csprintf('git-receive-pack %s', $repository->getLocalPath());
|
||||||
'git-receive-pack %s',
|
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
|
||||||
$repository->getLocalPath());
|
|
||||||
|
$future = new ExecFuture('%C', $command);
|
||||||
|
|
||||||
$err = $this->newPassthruCommand()
|
$err = $this->newPassthruCommand()
|
||||||
->setIOChannel($this->getIOChannel())
|
->setIOChannel($this->getIOChannel())
|
||||||
|
|
|
@ -19,7 +19,10 @@ final class DiffusionSSHGitUploadPackWorkflow
|
||||||
$path = head($args->getArg('dir'));
|
$path = head($args->getArg('dir'));
|
||||||
$repository = $this->loadRepository($path);
|
$repository = $this->loadRepository($path);
|
||||||
|
|
||||||
$future = new ExecFuture('git-upload-pack %s', $repository->getLocalPath());
|
$command = csprintf('git-upload-pack -- %s', $repository->getLocalPath());
|
||||||
|
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
|
||||||
|
|
||||||
|
$future = new ExecFuture('%C', $command);
|
||||||
|
|
||||||
$err = $this->newPassthruCommand()
|
$err = $this->newPassthruCommand()
|
||||||
->setIOChannel($this->getIOChannel())
|
->setIOChannel($this->getIOChannel())
|
||||||
|
|
|
@ -39,12 +39,12 @@ final class DiffusionSSHMercurialServeWorkflow
|
||||||
throw new Exception("Expected `hg ... serve`!");
|
throw new Exception("Expected `hg ... serve`!");
|
||||||
}
|
}
|
||||||
|
|
||||||
$future = new ExecFuture(
|
$command = csprintf('hg -R %s serve --stdio', $repository->getLocalPath());
|
||||||
'hg -R %s serve --stdio',
|
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
|
||||||
$repository->getLocalPath());
|
|
||||||
|
$future = new ExecFuture('%C', $command);
|
||||||
|
|
||||||
$io_channel = $this->getIOChannel();
|
$io_channel = $this->getIOChannel();
|
||||||
|
|
||||||
$protocol_channel = new DiffusionSSHMercurialWireClientProtocolChannel(
|
$protocol_channel = new DiffusionSSHMercurialWireClientProtocolChannel(
|
||||||
$io_channel);
|
$io_channel);
|
||||||
|
|
||||||
|
|
|
@ -38,7 +38,10 @@ final class DiffusionSSHSubversionServeWorkflow
|
||||||
throw new Exception("Expected `svnserve -t`!");
|
throw new Exception("Expected `svnserve -t`!");
|
||||||
}
|
}
|
||||||
|
|
||||||
$future = new ExecFuture('svnserve -t');
|
$command = csprintf('svnserve -t');
|
||||||
|
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
|
||||||
|
|
||||||
|
$future = new ExecFuture('%C', $command);
|
||||||
|
|
||||||
$this->inProtocol = new DiffusionSubversionWireProtocol();
|
$this->inProtocol = new DiffusionSubversionWireProtocol();
|
||||||
$this->outProtocol = new DiffusionSubversionWireProtocol();
|
$this->outProtocol = new DiffusionSubversionWireProtocol();
|
||||||
|
|
|
@ -120,5 +120,4 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
|
||||||
return $this->hasWriteAccess;
|
return $this->hasWriteAccess;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -19,4 +19,35 @@ abstract class PhabricatorDaemon extends PhutilDaemon {
|
||||||
return PhabricatorUser::getOmnipotentUser();
|
return PhabricatorUser::getOmnipotentUser();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Format a command so it executes as the daemon user, if a daemon user is
|
||||||
|
* defined. This wraps the provided command in `sudo -u ...`, roughly.
|
||||||
|
*
|
||||||
|
* @param PhutilCommandString Command to execute.
|
||||||
|
* @return PhutilCommandString `sudo` version of the command.
|
||||||
|
*/
|
||||||
|
public static function sudoCommandAsDaemonUser($command) {
|
||||||
|
$user = PhabricatorEnv::getEnvConfig('phd.user');
|
||||||
|
if (!$user) {
|
||||||
|
// No daemon user is set, so just run this as ourselves.
|
||||||
|
return $command;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Get the absolute path so we're safe against the caller wiping out
|
||||||
|
// PATH.
|
||||||
|
$sudo = Filesystem::resolveBinary('sudo');
|
||||||
|
if (!$sudo) {
|
||||||
|
throw new Exception(pht("Unable to find 'sudo'!"));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Flags here are:
|
||||||
|
//
|
||||||
|
// -E: Preserve the environment.
|
||||||
|
// -n: Non-interactive. Exit with an error instead of prompting.
|
||||||
|
// -u: Which user to sudo to.
|
||||||
|
|
||||||
|
return csprintf('%s -E -n -u %s -- %C', $sudo, $user, $command);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue