From c70f4815a958b7ee5cc4ccbead92bd10242af336 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 19 Apr 2016 06:19:39 -0700 Subject: [PATCH] Allow cluster devices to SSH to one another without acting as a user Summary: Ref T4292. When you run `git fetch` and connect to, say, `repo001.west.company.com`, we'll look at the current version of the repository in other nodes in the cluster. If `repo002.east.company.com` has a newer version of the repository, we'll fetch that version first, then respond to your request. To do this, we need to run `git fetch repo002.east.company.com ...` and have that connect to the other host and be able to fetch data. This change allows us to run `PHABRICATOR_AS_DEVICE=1 git fetch ...` to use device credentials to do this fetch. (Device credentials are already supported and used, they just always connect as a user right now, but these fetches should be doable without having a user. We will have a valid user when you run `git fetch` yourself, but we won't have one if the daemons notice that a repository is out of date and want to update it, so the update code should not depend on having a user.) Test Plan: ``` $ PHABRICATOR_AS_DEVICE=1 ./bin/ssh-connect local.phacility.com Warning: Permanently added 'local.phacility.com' (RSA) to the list of known hosts. PTY allocation request failed on channel 0 phabricator-ssh-exec: Welcome to Phabricator. You are logged in as device/daemon.phacility.net. You haven't specified a command to run. This means you're requesting an interactive shell, but Phabricator does not provide an interactive shell over SSH. Usually, you should run a command like `git clone` or `hg push` rather than connecting directly with SSH. Supported commands are: conduit, git-lfs-authenticate, git-receive-pack, git-upload-pack, hg, svnserve. Connection to local.phacility.com closed. ``` Reviewers: chad Reviewed By: chad Maniphest Tasks: T4292 Differential Revision: https://secure.phabricator.com/D15755 --- scripts/ssh/ssh-connect.php | 28 +++++++++++ scripts/ssh/ssh-exec.php | 49 ++++++++++--------- .../protocol/DiffusionCommandEngine.php | 46 ++++++++++++++++- 3 files changed, 100 insertions(+), 23 deletions(-) diff --git a/scripts/ssh/ssh-connect.php b/scripts/ssh/ssh-connect.php index 8a142090c3..fac8d19130 100755 --- a/scripts/ssh/ssh-connect.php +++ b/scripts/ssh/ssh-connect.php @@ -34,7 +34,28 @@ $pattern[] = 'StrictHostKeyChecking=no'; $pattern[] = '-o'; $pattern[] = 'UserKnownHostsFile=/dev/null'; +$as_device = getenv('PHABRICATOR_AS_DEVICE'); $credential_phid = getenv('PHABRICATOR_CREDENTIAL'); + +if ($as_device) { + $device = AlmanacKeys::getLiveDevice(); + if (!$device) { + throw new Exception( + pht( + 'Attempting to create an SSH connection that authenticates with '. + 'the current device, but this host is not configured as a cluster '. + 'device.')); + } + + if ($credential_phid) { + throw new Exception( + pht( + 'Attempting to proxy an SSH connection that authenticates with '. + 'both the current device and a specific credential. These options '. + 'are mutually exclusive.')); + } +} + if ($credential_phid) { $viewer = PhabricatorUser::getOmnipotentUser(); $key = PassphraseSSHKey::loadFromPHID($credential_phid, $viewer); @@ -45,6 +66,13 @@ if ($credential_phid) { $arguments[] = $key->getKeyfileEnvelope(); } +if ($as_device) { + $pattern[] = '-l %R'; + $arguments[] = AlmanacKeys::getClusterSSHUser(); + $pattern[] = '-i %R'; + $arguments[] = AlmanacKeys::getKeyPath('device.key'); +} + $port = $args->getArg('port'); if ($port) { $pattern[] = '-p %d'; diff --git a/scripts/ssh/ssh-exec.php b/scripts/ssh/ssh-exec.php index 2238bab478..5d8ede2913 100755 --- a/scripts/ssh/ssh-exec.php +++ b/scripts/ssh/ssh-exec.php @@ -153,32 +153,37 @@ try { ->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.')); + // If we're authenticating as a device, the first argument may be a + // "@username" argument to act as a particular user. + $first_argument = head($original_argv); + if (preg_match('/^@/', $first_argument)) { + $act_as_name = array_shift($original_argv); + $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)); + } + } else { + $user = PhabricatorUser::getOmnipotentUser(); } + } - $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)); - } + if ($user->isOmnipotent()) { + $user_name = 'device/'.$device->getName(); + } else { + $user_name = $user->getUsername(); } $ssh_log->setData( array( - 'u' => $user->getUsername(), + 'u' => $user_name, 'P' => $user->getPHID(), )); @@ -187,7 +192,7 @@ try { pht( 'Your account ("%s") does not have permission to establish SSH '. 'sessions. Visit the web interface for more information.', - $user->getUsername())); + $user_name)); } $workflows = id(new PhutilClassMapQuery()) @@ -206,7 +211,7 @@ try { "Usually, you should run a command like `%s` or `%s` ". "rather than connecting directly with SSH.\n\n". "Supported commands are: %s.", - $user->getUsername(), + $user_name, 'git clone', 'hg push', implode(', ', array_keys($workflows)))); diff --git a/src/applications/diffusion/protocol/DiffusionCommandEngine.php b/src/applications/diffusion/protocol/DiffusionCommandEngine.php index e5d54f110b..6894c2c4e8 100644 --- a/src/applications/diffusion/protocol/DiffusionCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionCommandEngine.php @@ -7,6 +7,7 @@ abstract class DiffusionCommandEngine extends Phobject { private $credentialPHID; private $argv; private $passthru; + private $connectAsDevice; public static function newCommandEngine(PhabricatorRepository $repository) { $engines = self::newCommandEngines(); @@ -82,6 +83,15 @@ abstract class DiffusionCommandEngine extends Phobject { return $this->passthru; } + public function setConnectAsDevice($connect_as_device) { + $this->connectAsDevice = $connect_as_device; + return $this; + } + + public function getConnectAsDevice() { + return $this->connectAsDevice; + } + public function newFuture() { $argv = $this->newCommandArgv(); $env = $this->newCommandEnvironment(); @@ -118,6 +128,8 @@ abstract class DiffusionCommandEngine extends Phobject { } private function newCommonEnvironment() { + $repository = $this->getRepository(); + $env = array(); // NOTE: Force the language to "en_US.UTF-8", which overrides locale // settings. This makes stuff print in English instead of, e.g., French, @@ -127,11 +139,43 @@ abstract class DiffusionCommandEngine extends Phobject { // Propagate PHABRICATOR_ENV explicitly. For discussion, see T4155. $env['PHABRICATOR_ENV'] = PhabricatorEnv::getSelectedEnvironmentName(); + $as_device = $this->getConnectAsDevice(); + $credential_phid = $this->getCredentialPHID(); + + if ($as_device) { + $device = AlmanacKeys::getLiveDevice(); + if (!$device) { + throw new Exception( + pht( + 'Attempting to build a reposiory command (for repository "%s") '. + 'as device, but this host ("%s") is not configured as a cluster '. + 'device.', + $repository->getDisplayName(), + php_uname('n'))); + } + + if ($credential_phid) { + throw new Exception( + pht( + 'Attempting to build a repository command (for repository "%s"), '. + 'but the CommandEngine is configured to connect as both the '. + 'current cluster device ("%s") and with a specific credential '. + '("%s"). These options are mutually exclusive. Connections must '. + 'authenticate as one or the other, not both.', + $repository->getDisplayName(), + $device->getName(), + $credential_phid)); + } + } + + if ($this->isAnySSHProtocol()) { - $credential_phid = $this->getCredentialPHID(); if ($credential_phid) { $env['PHABRICATOR_CREDENTIAL'] = $credential_phid; } + if ($as_device) { + $env['PHABRICATOR_AS_DEVICE'] = 1; + } } return $env;