Skip to content

Commit c70f481

Browse files
author
epriestley
committedApr 19, 2016
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
1 parent 0db6eac commit c70f481

File tree

3 files changed

+100
-23
lines changed

3 files changed

+100
-23
lines changed
 

‎scripts/ssh/ssh-connect.php

+28
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,28 @@
3434
$pattern[] = '-o';
3535
$pattern[] = 'UserKnownHostsFile=/dev/null';
3636

37+
$as_device = getenv('PHABRICATOR_AS_DEVICE');
3738
$credential_phid = getenv('PHABRICATOR_CREDENTIAL');
39+
40+
if ($as_device) {
41+
$device = AlmanacKeys::getLiveDevice();
42+
if (!$device) {
43+
throw new Exception(
44+
pht(
45+
'Attempting to create an SSH connection that authenticates with '.
46+
'the current device, but this host is not configured as a cluster '.
47+
'device.'));
48+
}
49+
50+
if ($credential_phid) {
51+
throw new Exception(
52+
pht(
53+
'Attempting to proxy an SSH connection that authenticates with '.
54+
'both the current device and a specific credential. These options '.
55+
'are mutually exclusive.'));
56+
}
57+
}
58+
3859
if ($credential_phid) {
3960
$viewer = PhabricatorUser::getOmnipotentUser();
4061
$key = PassphraseSSHKey::loadFromPHID($credential_phid, $viewer);
@@ -45,6 +66,13 @@
4566
$arguments[] = $key->getKeyfileEnvelope();
4667
}
4768

69+
if ($as_device) {
70+
$pattern[] = '-l %R';
71+
$arguments[] = AlmanacKeys::getClusterSSHUser();
72+
$pattern[] = '-i %R';
73+
$arguments[] = AlmanacKeys::getKeyPath('device.key');
74+
}
75+
4876
$port = $args->getArg('port');
4977
if ($port) {
5078
$pattern[] = '-p %d';

‎scripts/ssh/ssh-exec.php

+27-22
Original file line numberDiff line numberDiff line change
@@ -153,32 +153,37 @@
153153
->splitArguments($original_command);
154154

155155
if ($device) {
156-
$act_as_name = array_shift($original_argv);
157-
if (!preg_match('/^@/', $act_as_name)) {
158-
throw new Exception(
159-
pht(
160-
'Commands executed by devices must identify an acting user in the '.
161-
'first command argument. This request was not constructed '.
162-
'properly.'));
156+
// If we're authenticating as a device, the first argument may be a
157+
// "@username" argument to act as a particular user.
158+
$first_argument = head($original_argv);
159+
if (preg_match('/^@/', $first_argument)) {
160+
$act_as_name = array_shift($original_argv);
161+
$act_as_name = substr($act_as_name, 1);
162+
$user = id(new PhabricatorPeopleQuery())
163+
->setViewer(PhabricatorUser::getOmnipotentUser())
164+
->withUsernames(array($act_as_name))
165+
->executeOne();
166+
if (!$user) {
167+
throw new Exception(
168+
pht(
169+
'Device request identifies an acting user with an invalid '.
170+
'username ("%s"). There is no user with this username.',
171+
$act_as_name));
172+
}
173+
} else {
174+
$user = PhabricatorUser::getOmnipotentUser();
163175
}
176+
}
164177

165-
$act_as_name = substr($act_as_name, 1);
166-
$user = id(new PhabricatorPeopleQuery())
167-
->setViewer(PhabricatorUser::getOmnipotentUser())
168-
->withUsernames(array($act_as_name))
169-
->executeOne();
170-
if (!$user) {
171-
throw new Exception(
172-
pht(
173-
'Device request identifies an acting user with an invalid '.
174-
'username ("%s"). There is no user with this username.',
175-
$act_as_name));
176-
}
178+
if ($user->isOmnipotent()) {
179+
$user_name = 'device/'.$device->getName();
180+
} else {
181+
$user_name = $user->getUsername();
177182
}
178183

179184
$ssh_log->setData(
180185
array(
181-
'u' => $user->getUsername(),
186+
'u' => $user_name,
182187
'P' => $user->getPHID(),
183188
));
184189

@@ -187,7 +192,7 @@
187192
pht(
188193
'Your account ("%s") does not have permission to establish SSH '.
189194
'sessions. Visit the web interface for more information.',
190-
$user->getUsername()));
195+
$user_name));
191196
}
192197

193198
$workflows = id(new PhutilClassMapQuery())
@@ -206,7 +211,7 @@
206211
"Usually, you should run a command like `%s` or `%s` ".
207212
"rather than connecting directly with SSH.\n\n".
208213
"Supported commands are: %s.",
209-
$user->getUsername(),
214+
$user_name,
210215
'git clone',
211216
'hg push',
212217
implode(', ', array_keys($workflows))));

‎src/applications/diffusion/protocol/DiffusionCommandEngine.php

+45-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ abstract class DiffusionCommandEngine extends Phobject {
77
private $credentialPHID;
88
private $argv;
99
private $passthru;
10+
private $connectAsDevice;
1011

1112
public static function newCommandEngine(PhabricatorRepository $repository) {
1213
$engines = self::newCommandEngines();
@@ -82,6 +83,15 @@ public function getPassthru() {
8283
return $this->passthru;
8384
}
8485

86+
public function setConnectAsDevice($connect_as_device) {
87+
$this->connectAsDevice = $connect_as_device;
88+
return $this;
89+
}
90+
91+
public function getConnectAsDevice() {
92+
return $this->connectAsDevice;
93+
}
94+
8595
public function newFuture() {
8696
$argv = $this->newCommandArgv();
8797
$env = $this->newCommandEnvironment();
@@ -118,6 +128,8 @@ private function newCommandEnvironment() {
118128
}
119129

120130
private function newCommonEnvironment() {
131+
$repository = $this->getRepository();
132+
121133
$env = array();
122134
// NOTE: Force the language to "en_US.UTF-8", which overrides locale
123135
// settings. This makes stuff print in English instead of, e.g., French,
@@ -127,11 +139,43 @@ private function newCommonEnvironment() {
127139
// Propagate PHABRICATOR_ENV explicitly. For discussion, see T4155.
128140
$env['PHABRICATOR_ENV'] = PhabricatorEnv::getSelectedEnvironmentName();
129141

142+
$as_device = $this->getConnectAsDevice();
143+
$credential_phid = $this->getCredentialPHID();
144+
145+
if ($as_device) {
146+
$device = AlmanacKeys::getLiveDevice();
147+
if (!$device) {
148+
throw new Exception(
149+
pht(
150+
'Attempting to build a reposiory command (for repository "%s") '.
151+
'as device, but this host ("%s") is not configured as a cluster '.
152+
'device.',
153+
$repository->getDisplayName(),
154+
php_uname('n')));
155+
}
156+
157+
if ($credential_phid) {
158+
throw new Exception(
159+
pht(
160+
'Attempting to build a repository command (for repository "%s"), '.
161+
'but the CommandEngine is configured to connect as both the '.
162+
'current cluster device ("%s") and with a specific credential '.
163+
'("%s"). These options are mutually exclusive. Connections must '.
164+
'authenticate as one or the other, not both.',
165+
$repository->getDisplayName(),
166+
$device->getName(),
167+
$credential_phid));
168+
}
169+
}
170+
171+
130172
if ($this->isAnySSHProtocol()) {
131-
$credential_phid = $this->getCredentialPHID();
132173
if ($credential_phid) {
133174
$env['PHABRICATOR_CREDENTIAL'] = $credential_phid;
134175
}
176+
if ($as_device) {
177+
$env['PHABRICATOR_AS_DEVICE'] = 1;
178+
}
135179
}
136180

137181
return $env;

0 commit comments

Comments
 (0)
Failed to load comments.