Proxy VCS SSH requests

Summary: Fixes T7034. Like HTTP, proxy requests to the correct host if a repository has an Almanac service host.

Test Plan: Ran VCS requests through the proxy.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7034

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

scripts/ssh/ssh-auth.php

@@ -8,15 +8,6 @@ $keys = id(new PhabricatorAuthSSHKeyQuery())
   ->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 @@ if (!$keys) {
 
 $bin = $root.'/bin/ssh-exec';
 foreach ($keys as $ssh_key) {
-  $user = $ssh_key->getObject()->getUsername();
-
   $key_argv = array();
+  $object = $ssh_key->getObject();
+  if ($object instanceof PhabricatorUser) {
     $key_argv[] = '--phabricator-ssh-user';
-  $key_argv[] = $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);
 

scripts/ssh/ssh-exec.php

@@ -8,12 +8,14 @@ require_once $root.'/scripts/__init_script__.php';
 
 $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 @@ $args->parse(
     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 {
-  $user_name = $args->getArg('phabricator-ssh-user');
+  $remote_address = null;
-  if (!strlen($user_name)) {
+  $ssh_client = getenv('SSH_CLIENT');
-    throw new Exception('No username.');
+  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,
+      ));
   }
 
-  $user = id(new PhabricatorUser())->loadOneWhere(
+  $key_id = $args->getArg('phabricator-ssh-key');
-    'userName = %s',
+  if ($key_id) {
-    $user_name);
+    $ssh_log->setData(
+      array(
+        'k' => $key_id,
+      ));
+  }
+
+  $user_name = $args->getArg('phabricator-ssh-user');
+  $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('Invalid username.');
+      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');
+  }
+
+  $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 @@ try {
     ));
 
   if (!$user->isUserActivated()) {
-    throw new Exception(pht('Your account is not activated.'));
+    throw new Exception(
-  }
+      pht(
-
+        'Your account ("%s") is not activated. Visit the web interface '.
-  if ($args->getArg('ssh-command')) {
+        'for more information.',
-    $original_command = $args->getArg('ssh-command');
+        $user->getUsername()));
-  } else {
-    $original_command = getenv('SSH_ORIGINAL_COMMAND');
   }
 
   $workflows = id(new PhutilSymbolLoader())
@@ -64,9 +190,6 @@ try {
 
   $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 @@ try {
         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 @@ try {
     ));
 
   $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 @@ try {
 
   $rethrow = null;
   try {
-    $err = $workflow->execute($original_args);
+    $err = $workflow->execute($parsed_args);
 
     $metrics_channel->flush();
     $error_channel->flush();

src/applications/config/option/PhabricatorAccessLogConfigOptions.php

@@ -37,6 +37,7 @@ final class PhabricatorAccessLogConfigOptions
     $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(

src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php

@@ -19,7 +19,11 @@ final class DiffusionGitReceivePackSSHWorkflow extends DiffusionGitSSHWorkflow {
     // This is a write, and must have write access.
     $this->requireWriteAccess();
 
+    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))

src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php

@@ -16,7 +16,11 @@ final class DiffusionGitUploadPackSSHWorkflow extends DiffusionGitSSHWorkflow {
   protected function executeRepositoryOperations() {
     $repository = $this->getRepository();
 
+    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))

src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php

@@ -42,7 +42,13 @@ final class DiffusionMercurialServeSSHWorkflow
       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))

src/applications/diffusion/ssh/DiffusionSSHWorkflow.php

@@ -5,6 +5,7 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
   private $args;
   private $repository;
   private $hasWriteAccess;
+  private $proxyURI;
 
   public function getRepository() {
     if (!$this->repository) {
@@ -49,14 +50,82 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
     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
+    $is_cluster_request = $this->getIsClusterRequest();
-    // proxying yet.
+    $uri = $repository->getAlmanacServiceURI(
+      $this->getUser(),
+      $is_cluster_request,
+      array(
+        'ssh',
+      ));
+
+    if ($uri) {
+      $this->proxyURI = $uri;
+    }
 
     try {
       return $this->executeRepositoryOperations();

src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php

@@ -144,11 +144,15 @@ final class DiffusionSubversionServeSSHWorkflow
       throw new Exception('Expected `svnserve -t`!');
     }
 
+    if ($this->shouldProxy()) {
+      $command = $this->getProxyCommand();
+    } else {
       $command = csprintf(
         'svnserve -t --tunnel-user=%s',
         $this->getUser()->getUsername());
-    $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
+    }
 
+    $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
     $future = new ExecFuture('%C', $command);
 
     $this->inProtocol = new DiffusionSubversionWireProtocol();

src/infrastructure/ssh/PhabricatorSSHWorkflow.php

@@ -5,6 +5,8 @@ abstract class PhabricatorSSHWorkflow extends PhabricatorManagementWorkflow {
   private $user;
   private $iochannel;
   private $errorChannel;
+  private $isClusterRequest;
+  private $originalArguments;
 
   public function isExecutable() {
     return false;
@@ -63,4 +65,22 @@ abstract class PhabricatorSSHWorkflow extends PhabricatorManagementWorkflow {
       ->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;
+  }
+
 }