Skip to content

Commit 6df278b

Browse files
author
epriestley
committedAug 9, 2018
In "bin/ssh-auth", cache a structure instead of a flat file because paths may change at runtime
Summary: Fixes T12397. Ref T13164. See PHI801. Several installs have hit various use cases where the path on disk where Phabricator lives changes at runtime. Currently, `bin/ssh-auth` caches a flat file which includes the path to `bin/ssh-exec`, so this may fall out of date if `phabricator/` moves. These use cases have varying strengths of legitimacy, but "we're migrating to a new set of hosts and the pool is half old machines and half new machines" seems reasonably compelling and not a problem entirely of one's own making. Test Plan: - Compared output on `master` to output after change, found them byte-for-byte identical. - Moved `phabricator/` to `phabricator2/`, ran `bin/ssh-auth`, got updated output. - Added a new SSH key, saw it appear in the output. - Grepped for `AUTHFILE_CACHEKEY` (no hits). - Dropped the cache, verified that the file regenerates cleanly. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13164, T12397 Differential Revision: https://secure.phabricator.com/D19568
1 parent df31405 commit 6df278b

File tree

2 files changed

+64
-29
lines changed

2 files changed

+64
-29
lines changed
 

‎scripts/ssh/ssh-auth.php

+62-27
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,27 @@
22
<?php
33

44
$root = dirname(dirname(dirname(__FILE__)));
5-
require_once $root.'/scripts/__init_script__.php';
5+
require_once $root.'/scripts/init/init-script.php';
6+
7+
// NOTE: We are caching a datastructure rather than the flat key file because
8+
// the path on disk to "ssh-exec" is arbitrarily mutable at runtime. See T12397.
69

710
$cache = PhabricatorCaches::getMutableCache();
8-
$authfile_key = PhabricatorAuthSSHKeyQuery::AUTHFILE_CACHEKEY;
9-
$authfile = $cache->getKey($authfile_key);
11+
$authstruct_key = PhabricatorAuthSSHKeyQuery::AUTHSTRUCT_CACHEKEY;
12+
$authstruct_raw = $cache->getKey($authstruct_key);
13+
14+
$authstruct = null;
15+
16+
if (strlen($authstruct_raw)) {
17+
try {
18+
$authstruct = phutil_json_decode($authstruct_raw);
19+
} catch (Exception $ex) {
20+
// Ignore any issues with the cached data; we'll just rebuild the
21+
// structure below.
22+
}
23+
}
1024

11-
if ($authfile === null) {
25+
if ($authstruct === null) {
1226
$keys = id(new PhabricatorAuthSSHKeyQuery())
1327
->setViewer(PhabricatorUser::getOmnipotentUser())
1428
->withIsActive(true)
@@ -19,7 +33,7 @@
1933
exit(1);
2034
}
2135

22-
$bin = $root.'/bin/ssh-exec';
36+
$key_list = array();
2337
foreach ($keys as $ssh_key) {
2438
$key_argv = array();
2539
$object = $ssh_key->getObject();
@@ -42,18 +56,7 @@
4256
$key_argv[] = '--phabricator-ssh-key';
4357
$key_argv[] = $ssh_key->getID();
4458

45-
$cmd = csprintf('%s %Ls', $bin, $key_argv);
46-
47-
$instance = PhabricatorEnv::getEnvConfig('cluster.instance');
48-
if (strlen($instance)) {
49-
$cmd = csprintf('PHABRICATOR_INSTANCE=%s %C', $instance, $cmd);
50-
}
51-
52-
// This is additional escaping for the SSH 'command="..."' string.
53-
$cmd = addcslashes($cmd, '"\\');
54-
5559
// Strip out newlines and other nonsense from the key type and key body.
56-
5760
$type = $ssh_key->getKeyType();
5861
$type = preg_replace('@[\x00-\x20]+@', '', $type);
5962
if (!strlen($type)) {
@@ -66,22 +69,54 @@
6669
continue;
6770
}
6871

69-
$options = array(
70-
'command="'.$cmd.'"',
71-
'no-port-forwarding',
72-
'no-X11-forwarding',
73-
'no-agent-forwarding',
74-
'no-pty',
72+
$key_list[] = array(
73+
'argv' => $key_argv,
74+
'type' => $type,
75+
'key' => $key,
7576
);
76-
$options = implode(',', $options);
77-
78-
$lines[] = $options.' '.$type.' '.$key."\n";
7977
}
8078

81-
$authfile = implode('', $lines);
79+
$authstruct = array(
80+
'keys' => $key_list,
81+
);
82+
83+
$authstruct_raw = phutil_json_encode($authstruct);
8284
$ttl = phutil_units('24 hours in seconds');
83-
$cache->setKey($authfile_key, $authfile, $ttl);
85+
$cache->setKey($authstruct_key, $authstruct_raw, $ttl);
8486
}
8587

88+
$bin = $root.'/bin/ssh-exec';
89+
$instance = PhabricatorEnv::getEnvConfig('cluster.instance');
90+
91+
$lines = array();
92+
foreach ($authstruct['keys'] as $key_struct) {
93+
$key_argv = $key_struct['argv'];
94+
$key = $key_struct['key'];
95+
$type = $key_struct['type'];
96+
97+
$cmd = csprintf('%s %Ls', $bin, $key_argv);
98+
99+
if (strlen($instance)) {
100+
$cmd = csprintf('PHABRICATOR_INSTANCE=%s %C', $instance, $cmd);
101+
}
102+
103+
// This is additional escaping for the SSH 'command="..."' string.
104+
$cmd = addcslashes($cmd, '"\\');
105+
106+
$options = array(
107+
'command="'.$cmd.'"',
108+
'no-port-forwarding',
109+
'no-X11-forwarding',
110+
'no-agent-forwarding',
111+
'no-pty',
112+
);
113+
$options = implode(',', $options);
114+
115+
$lines[] = $options.' '.$type.' '.$key."\n";
116+
}
117+
118+
$authfile = implode('', $lines);
119+
86120
echo $authfile;
121+
87122
exit(0);

‎src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
final class PhabricatorAuthSSHKeyQuery
44
extends PhabricatorCursorPagedPolicyAwareQuery {
55

6-
const AUTHFILE_CACHEKEY = 'ssh.authfile';
6+
const AUTHSTRUCT_CACHEKEY = 'ssh.authstruct';
77

88
private $ids;
99
private $phids;
@@ -13,7 +13,7 @@ final class PhabricatorAuthSSHKeyQuery
1313

1414
public static function deleteSSHKeyCache() {
1515
$cache = PhabricatorCaches::getMutableCache();
16-
$authfile_key = self::AUTHFILE_CACHEKEY;
16+
$authfile_key = self::AUTHSTRUCT_CACHEKEY;
1717
$cache->deleteKey($authfile_key);
1818
}
1919

0 commit comments

Comments
 (0)
Failed to load comments.