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(<< '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 " ". 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
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; + } + }