Page MenuHomePhabricator

D11543.id.diff
No OneTemporary

D11543.id.diff

diff --git a/scripts/ssh/ssh-auth.php b/scripts/ssh/ssh-auth.php
--- a/scripts/ssh/ssh-auth.php
+++ b/scripts/ssh/ssh-auth.php
@@ -8,15 +8,6 @@
->setViewer(PhabricatorUser::getOmnipotentUser())
->execute();
-foreach ($keys as $key => $ssh_key) {
- // For now, filter out any keys which don't belong to users. Eventually we
- // may allow devices to use this channel.
- if (!($ssh_key->getObject() instanceof PhabricatorUser)) {
- unset($keys[$key]);
- continue;
- }
-}
-
if (!$keys) {
echo pht('No keys found.')."\n";
exit(1);
@@ -24,11 +15,26 @@
$bin = $root.'/bin/ssh-exec';
foreach ($keys as $ssh_key) {
- $user = $ssh_key->getObject()->getUsername();
-
$key_argv = array();
- $key_argv[] = '--phabricator-ssh-user';
- $key_argv[] = $user;
+ $object = $ssh_key->getObject();
+ if ($object instanceof PhabricatorUser) {
+ $key_argv[] = '--phabricator-ssh-user';
+ $key_argv[] = $object->getUsername();
+ } else if ($object instanceof AlmanacDevice) {
+ if (!$ssh_key->getIsTrusted()) {
+ // If this key is not a trusted device key, don't allow SSH
+ // authentication.
+ continue;
+ }
+ $key_argv[] = '--phabricator-ssh-device';
+ $key_argv[] = $object->getName();
+ } else {
+ // We don't know what sort of key this is; don't permit SSH auth.
+ continue;
+ }
+
+ $key_argv[] = '--phabricator-ssh-key';
+ $key_argv[] = $ssh_key->getID();
$cmd = csprintf('%s %Ls', $bin, $key_argv);
diff --git a/scripts/ssh/ssh-exec.php b/scripts/ssh/ssh-exec.php
--- a/scripts/ssh/ssh-exec.php
+++ b/scripts/ssh/ssh-exec.php
@@ -8,12 +8,14 @@
$ssh_log = PhabricatorSSHLog::getLog();
-// First, figure out the authenticated user.
$args = new PhutilArgumentParser($argv);
-$args->setTagline('receive SSH requests');
+$args->setTagline('execute SSH requests');
$args->setSynopsis(<<<EOSYNOPSIS
**ssh-exec** --phabricator-ssh-user __user__ [--ssh-command __commmand__]
- Receive SSH requests.
+**ssh-exec** --phabricator-ssh-device __device__ [--ssh-command __commmand__]
+ Execute authenticated SSH requests. This script is normally invoked
+ via SSHD, but can be invoked manually for testing.
+
EOSYNOPSIS
);
@@ -22,24 +24,150 @@
array(
'name' => 'phabricator-ssh-user',
'param' => 'username',
+ 'help' => pht(
+ 'If the request authenticated with a user key, the name of the '.
+ 'user.'),
+ ),
+ array(
+ 'name' => 'phabricator-ssh-device',
+ 'param' => 'name',
+ 'help' => pht(
+ 'If the request authenticated with a device key, the name of the '.
+ 'device.'),
+ ),
+ array(
+ 'name' => 'phabricator-ssh-key',
+ 'param' => 'id',
+ 'help' => pht(
+ 'The ID of the SSH key which authenticated this request. This is '.
+ 'used to allow logs to report when specific keys were used, to make '.
+ 'it easier to manage credentials.'),
),
array(
'name' => 'ssh-command',
'param' => 'command',
+ 'help' => pht(
+ 'Provide a command to execute. This makes testing this script '.
+ 'easier. When running normally, the command is read from the '.
+ 'environment (SSH_ORIGINAL_COMMAND), which is populated by sshd.'),
),
));
try {
+ $remote_address = null;
+ $ssh_client = getenv('SSH_CLIENT');
+ if ($ssh_client) {
+ // This has the format "<ip> <remote-port> <local-port>". Grab the IP.
+ $remote_address = head(explode(' ', $ssh_client));
+ $ssh_log->setData(
+ array(
+ 'r' => $remote_address,
+ ));
+ }
+
+ $key_id = $args->getArg('phabricator-ssh-key');
+ if ($key_id) {
+ $ssh_log->setData(
+ array(
+ 'k' => $key_id,
+ ));
+ }
+
$user_name = $args->getArg('phabricator-ssh-user');
- if (!strlen($user_name)) {
- throw new Exception('No username.');
+ $device_name = $args->getArg('phabricator-ssh-device');
+
+ $user = null;
+ $device = null;
+ $is_cluster_request = false;
+
+ if ($user_name && $device_name) {
+ throw new Exception(
+ pht(
+ 'The --phabricator-ssh-user and --phabricator-ssh-device flags are '.
+ 'mutually exclusive. You can not authenticate as both a user ("%s") '.
+ 'and a device ("%s"). Specify one or the other, but not both.',
+ $user_name,
+ $device_name));
+ } else if (strlen($user_name)) {
+ $user = id(new PhabricatorPeopleQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withUsernames(array($user_name))
+ ->executeOne();
+ if (!$user) {
+ throw new Exception(
+ pht(
+ 'Invalid username ("%s"). There is no user with this username.',
+ $user_name));
+ }
+ } else if (strlen($device_name)) {
+ if (!$remote_address) {
+ throw new Exception(
+ pht(
+ 'Unable to identify remote address from the SSH_CLIENT environment '.
+ 'variable. Device authentication is accepted only from trusted '.
+ 'sources.'));
+ }
+
+ if (!PhabricatorEnv::isClusterAddress($remote_address)) {
+ throw new Exception(
+ pht(
+ 'This request originates from outside of the Phabricator cluster '.
+ 'address range. Requests signed with a trusted device key must '.
+ 'originate from trusted hosts.'));
+ }
+
+ $device = id(new AlmanacDeviceQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withNames(array($device_name))
+ ->executeOne();
+ if (!$device) {
+ throw new Exception(
+ pht(
+ 'Invalid device name ("%s"). There is no device with this name.',
+ $device->getName()));
+ }
+
+ // We're authenticated as a device, but we're going to read the user out of
+ // the command below.
+ $is_cluster_request = true;
+ } else {
+ throw new Exception(
+ pht(
+ 'This script must be invoked with either the --phabricator-ssh-user '.
+ 'or --phabricator-ssh-device flag.'));
+ }
+
+ if ($args->getArg('ssh-command')) {
+ $original_command = $args->getArg('ssh-command');
+ } else {
+ $original_command = getenv('SSH_ORIGINAL_COMMAND');
}
- $user = id(new PhabricatorUser())->loadOneWhere(
- 'userName = %s',
- $user_name);
- if (!$user) {
- throw new Exception('Invalid username.');
+ $original_argv = id(new PhutilShellLexer())
+ ->splitArguments($original_command);
+
+ if ($device) {
+ $act_as_name = array_shift($original_argv);
+ if (!preg_match('/^@/', $act_as_name)) {
+ throw new Exception(
+ pht(
+ 'Commands executed by devices must identify an acting user in the '.
+ 'first command argument. This request was not constructed '.
+ 'properly.'));
+ }
+
+ $act_as_name = substr($act_as_name, 1);
+ $user = id(new PhabricatorPeopleQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withUsernames(array($act_as_name))
+ ->executeOne();
+ if (!$user) {
+ throw new Exception(
+ pht(
+ 'Device request identifies an acting user with an invalid '.
+ 'username ("%s"). There is no user with this username.',
+ $act_as_name));
+ }
}
$ssh_log->setData(
@@ -49,13 +177,11 @@
));
if (!$user->isUserActivated()) {
- throw new Exception(pht('Your account is not activated.'));
- }
-
- if ($args->getArg('ssh-command')) {
- $original_command = $args->getArg('ssh-command');
- } else {
- $original_command = getenv('SSH_ORIGINAL_COMMAND');
+ throw new Exception(
+ pht(
+ 'Your account ("%s") is not activated. Visit the web interface '.
+ 'for more information.',
+ $user->getUsername()));
}
$workflows = id(new PhutilSymbolLoader())
@@ -64,9 +190,6 @@
$workflow_names = mpull($workflows, 'getName', 'getName');
- // Now, rebuild the original command.
- $original_argv = id(new PhutilShellLexer())
- ->splitArguments($original_command);
if (!$original_argv) {
throw new Exception(
pht(
@@ -82,7 +205,7 @@
implode(', ', $workflow_names)));
}
- $log_argv = implode(' ', array_slice($original_argv, 1));
+ $log_argv = implode(' ', $original_argv);
$log_argv = id(new PhutilUTF8StringTruncator())
->setMaximumCodepoints(128)
->truncateString($log_argv);
@@ -94,16 +217,20 @@
));
$command = head($original_argv);
- array_unshift($original_argv, 'phabricator-ssh-exec');
- $original_args = new PhutilArgumentParser($original_argv);
+ $parseable_argv = $original_argv;
+ array_unshift($parseable_argv, 'phabricator-ssh-exec');
+
+ $parsed_args = new PhutilArgumentParser($parseable_argv);
if (empty($workflow_names[$command])) {
throw new Exception('Invalid command.');
}
- $workflow = $original_args->parseWorkflows($workflows);
+ $workflow = $parsed_args->parseWorkflows($workflows);
$workflow->setUser($user);
+ $workflow->setOriginalArguments($original_argv);
+ $workflow->setIsClusterRequest($is_cluster_request);
$sock_stdin = fopen('php://stdin', 'r');
if (!$sock_stdin) {
@@ -130,7 +257,7 @@
$rethrow = null;
try {
- $err = $workflow->execute($original_args);
+ $err = $workflow->execute($parsed_args);
$metrics_channel->flush();
$error_channel->flush();
diff --git a/src/applications/config/option/PhabricatorAccessLogConfigOptions.php b/src/applications/config/option/PhabricatorAccessLogConfigOptions.php
--- a/src/applications/config/option/PhabricatorAccessLogConfigOptions.php
+++ b/src/applications/config/option/PhabricatorAccessLogConfigOptions.php
@@ -37,6 +37,7 @@
$ssh_map = $common_map + array(
's' => pht('The system user.'),
'S' => pht('The system sudo user.'),
+ 'k' => pht('ID of the SSH key used to authenticate the request.'),
);
$http_desc = pht(
diff --git a/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
@@ -19,7 +19,11 @@
// This is a write, and must have write access.
$this->requireWriteAccess();
- $command = csprintf('git-receive-pack %s', $repository->getLocalPath());
+ if ($this->shouldProxy()) {
+ $command = $this->getProxyCommand();
+ } else {
+ $command = csprintf('git-receive-pack %s', $repository->getLocalPath());
+ }
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
$future = id(new ExecFuture('%C', $command))
diff --git a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
@@ -16,7 +16,11 @@
protected function executeRepositoryOperations() {
$repository = $this->getRepository();
- $command = csprintf('git-upload-pack -- %s', $repository->getLocalPath());
+ if ($this->shouldProxy()) {
+ $command = $this->getProxyCommand();
+ } else {
+ $command = csprintf('git-upload-pack -- %s', $repository->getLocalPath());
+ }
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
$future = id(new ExecFuture('%C', $command))
diff --git a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php
@@ -42,7 +42,13 @@
throw new Exception('Expected `hg ... serve`!');
}
- $command = csprintf('hg -R %s serve --stdio', $repository->getLocalPath());
+ if ($this->shouldProxy()) {
+ $command = $this->getProxyCommand();
+ } else {
+ $command = csprintf(
+ 'hg -R %s serve --stdio',
+ $repository->getLocalPath());
+ }
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
$future = id(new ExecFuture('%C', $command))
diff --git a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php
@@ -5,6 +5,7 @@
private $args;
private $repository;
private $hasWriteAccess;
+ private $proxyURI;
public function getRepository() {
if (!$this->repository) {
@@ -49,14 +50,82 @@
return $this;
}
+ protected function shouldProxy() {
+ return (bool)$this->proxyURI;
+ }
+
+ protected function getProxyCommand() {
+ $uri = new PhutilURI($this->proxyURI);
+
+ $username = PhabricatorEnv::getEnvConfig('cluster.instance');
+ if (!strlen($username)) {
+ $username = PhabricatorEnv::getEnvConfig('diffusion.ssh-user');
+ if (!strlen($username)) {
+ throw new Exception(
+ pht(
+ 'Unable to determine the username to connect with when trying '.
+ 'to proxy an SSH request within the Phabricator cluster.'));
+ }
+ }
+
+ $port = $uri->getPort();
+ $host = $uri->getDomain();
+ $key_path = AlmanacKeys::getKeyPath('device.key');
+ if (!Filesystem::pathExists($key_path)) {
+ throw new Exception(
+ pht(
+ 'Unable to proxy this SSH request within the cluster: this device '.
+ 'is not registered and has a missing device key (expected to '.
+ 'find key at "%s").',
+ $key_path));
+ }
+
+ $options = array();
+ $options[] = '-o';
+ $options[] = 'StrictHostKeyChecking=no';
+ $options[] = '-o';
+ $options[] = 'UserKnownHostsFile=/dev/null';
+
+ // This is suppressing "added <address> to the list of known hosts"
+ // messages, which are confusing and irrelevant when they arise from
+ // proxied requests. It might also be suppressing lots of useful errors,
+ // of course. Ideally, we would enforce host keys eventually.
+ $options[] = '-o';
+ $options[] = 'LogLevel=quiet';
+
+ // NOTE: We prefix the command with "@username", which the far end of the
+ // connection will parse in order to act as the specified user. This
+ // behavior is only available to cluster requests signed by a trusted
+ // device key.
+
+ return csprintf(
+ 'ssh %Ls -l %s -i %s -p %s %s -- %s %Ls',
+ $options,
+ $username,
+ $key_path,
+ $port,
+ $host,
+ '@'.$this->getUser()->getUsername(),
+ $this->getOriginalArguments());
+ }
+
final public function execute(PhutilArgumentParser $args) {
$this->args = $args;
$repository = $this->identifyRepository();
$this->setRepository($repository);
- // TODO: Here, we would make a proxying decision, had I implemented
- // proxying yet.
+ $is_cluster_request = $this->getIsClusterRequest();
+ $uri = $repository->getAlmanacServiceURI(
+ $this->getUser(),
+ $is_cluster_request,
+ array(
+ 'ssh',
+ ));
+
+ if ($uri) {
+ $this->proxyURI = $uri;
+ }
try {
return $this->executeRepositoryOperations();
diff --git a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php
@@ -144,11 +144,15 @@
throw new Exception('Expected `svnserve -t`!');
}
- $command = csprintf(
- 'svnserve -t --tunnel-user=%s',
- $this->getUser()->getUsername());
- $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
+ if ($this->shouldProxy()) {
+ $command = $this->getProxyCommand();
+ } else {
+ $command = csprintf(
+ 'svnserve -t --tunnel-user=%s',
+ $this->getUser()->getUsername());
+ }
+ $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
$future = new ExecFuture('%C', $command);
$this->inProtocol = new DiffusionSubversionWireProtocol();
diff --git a/src/infrastructure/ssh/PhabricatorSSHWorkflow.php b/src/infrastructure/ssh/PhabricatorSSHWorkflow.php
--- a/src/infrastructure/ssh/PhabricatorSSHWorkflow.php
+++ b/src/infrastructure/ssh/PhabricatorSSHWorkflow.php
@@ -5,6 +5,8 @@
private $user;
private $iochannel;
private $errorChannel;
+ private $isClusterRequest;
+ private $originalArguments;
public function isExecutable() {
return false;
@@ -63,4 +65,22 @@
->setErrorChannel($this->getErrorChannel());
}
+ public function setIsClusterRequest($is_cluster_request) {
+ $this->isClusterRequest = $is_cluster_request;
+ return $this;
+ }
+
+ public function getIsClusterRequest() {
+ return $this->isClusterRequest;
+ }
+
+ public function setOriginalArguments(array $original_arguments) {
+ $this->originalArguments = $original_arguments;
+ return $this;
+ }
+
+ public function getOriginalArguments() {
+ return $this->originalArguments;
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Wed, May 22, 8:54 AM (3 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6288409
Default Alt Text
D11543.id.diff (16 KB)

Event Timeline