Skip to content

Commit 888b383

Browse files
author
epriestley
committed
Prepare to route VCS connections through SSH
Summary: Fixes T2229. This sets the stage for a patch similar to D7417, but for SSH. In particular, SSH 6.2 introduced an `AuthorizedKeysCommand` directive, which lets us do this in a mostly-reasonable way without needing users to patch sshd (if they have a recent enough version, at least). The way the `AuthorizedKeysCommand` works is that it gets run and produces an `authorized_keys`-style file fragment. This isn't ideal, because we have to dump every key into the result, but should be fine for most installs. The earlier patch against `sshd` passes the public key itself, which allows the script to just look up the key. We might use this eventually, since it can scale much better, so I haven't removed it. Generally, auth is split into two scripts now which mostly do the same thing: - `ssh-auth` is the AuthorizedKeysCommand auth, which takes nothing and dumps the whole keyfile. - `ssh-auth-key` is the slightly cleaner and more scalable (but patch-dependent) version, which takes the public key and dumps only matching options. I also reworked the argument parsing to be a bit more sane. Test Plan: This is somewhat-intentionally a bit obtuse since I don't really want anyone using it yet, but basically: - Copy `phabricator-ssh-hook.sh` to somewhere like `/usr/libexec/openssh/`, chown it `root` and chmod it `500`. - This script should probably also do a username check in the future. - Create a copy of `sshd_config` and fix the paths/etc. Point the KeyScript at your copy of the hook. - Start a copy of sshd (6.2 or newer) with `-f <your config file>` and maybe `-d -d -d` to foreground and debug. - Run `ssh -p 2222 localhost` or similar. Specifically, I did this setup and then ran a bunch of commands like: - `ssh host` (denied, no command) - `ssh host ls` (denied, not supported) - `echo '{}' | ssh host conduit conduit.ping` (works) Reviewers: btrahan Reviewed By: btrahan CC: hach-que, aran Maniphest Tasks: T2229, T2230 Differential Revision: https://secure.phabricator.com/D7419
1 parent c7f23f5 commit 888b383

File tree

7 files changed

+153
-67
lines changed

7 files changed

+153
-67
lines changed

bin/ssh-auth-key

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../scripts/ssh/ssh-auth-key.php
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#!/bin/sh
2+
3+
###
4+
### WARNING: This feature is new and experimental. Use it at your own risk!
5+
###
6+
7+
ROOT=/INSECURE/devtools/phabricator
8+
exec "$ROOT/bin/ssh-auth" $@

resources/sshd/sshd_config.example

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
###
2+
### WARNING: This feature is new and experimental. Use it at your own risk!
3+
###
4+
5+
# You must have OpenSSHD 6.2 or newer; support for AuthorizedKeysCommand was
6+
# added in this version.
7+
8+
Port 2222
9+
AuthorizedKeysCommand /etc/phabricator-ssh-hook.sh
10+
AuthorizedKeysCommandUser some-unprivileged-user
11+
12+
# You may need to tweak these options, but mostly they just turn off everything
13+
# dangerous.
14+
15+
Protocol 2
16+
PermitRootLogin no
17+
AllowAgentForwarding no
18+
AllowTcpForwarding no
19+
PrintMotd no
20+
PrintLastLog no
21+
PasswordAuthentication no
22+
AuthorizedKeysFile none
23+
24+
PidFile /var/run/sshd-phabricator.pid

scripts/ssh/ssh-auth-key.php

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
#!/usr/bin/env php
2+
<?php
3+
4+
$root = dirname(dirname(dirname(__FILE__)));
5+
require_once $root.'/scripts/__init_script__.php';
6+
7+
$cert = file_get_contents('php://stdin');
8+
9+
if (!$cert) {
10+
exit(1);
11+
}
12+
13+
$parts = preg_split('/\s+/', $cert);
14+
if (count($parts) < 2) {
15+
exit(1);
16+
}
17+
18+
list($type, $body) = $parts;
19+
20+
$user_dao = new PhabricatorUser();
21+
$ssh_dao = new PhabricatorUserSSHKey();
22+
$conn_r = $user_dao->establishConnection('r');
23+
24+
$row = queryfx_one(
25+
$conn_r,
26+
'SELECT userName FROM %T u JOIN %T ssh ON u.phid = ssh.userPHID
27+
WHERE ssh.keyType = %s AND ssh.keyBody = %s',
28+
$user_dao->getTableName(),
29+
$ssh_dao->getTableName(),
30+
$type,
31+
$body);
32+
33+
if (!$row) {
34+
exit(1);
35+
}
36+
37+
$user = idx($row, 'userName');
38+
39+
if (!$user) {
40+
exit(1);
41+
}
42+
43+
if (!PhabricatorUser::validateUsername($user)) {
44+
exit(1);
45+
}
46+
47+
$bin = $root.'/bin/ssh-exec';
48+
$cmd = csprintf('%s --phabricator-ssh-user %s', $bin, $user);
49+
// This is additional escaping for the SSH 'command="..."' string.
50+
$cmd = addcslashes($cmd, '"\\');
51+
52+
$options = array(
53+
'command="'.$cmd.'"',
54+
'no-port-forwarding',
55+
'no-X11-forwarding',
56+
'no-agent-forwarding',
57+
'no-pty',
58+
);
59+
60+
echo implode(',', $options);
61+
exit(0);

scripts/ssh/ssh-auth.php

Lines changed: 29 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,58 +4,45 @@
44
$root = dirname(dirname(dirname(__FILE__)));
55
require_once $root.'/scripts/__init_script__.php';
66

7-
$cert = file_get_contents('php://stdin');
8-
9-
if (!$cert) {
10-
exit(1);
11-
}
12-
13-
$parts = preg_split('/\s+/', $cert);
14-
if (count($parts) < 2) {
15-
exit(1);
16-
}
17-
18-
list($type, $body) = $parts;
19-
207
$user_dao = new PhabricatorUser();
218
$ssh_dao = new PhabricatorUserSSHKey();
229
$conn_r = $user_dao->establishConnection('r');
2310

24-
$row = queryfx_one(
11+
$rows = queryfx_all(
2512
$conn_r,
26-
'SELECT userName FROM %T u JOIN %T ssh ON u.phid = ssh.userPHID
27-
WHERE ssh.keyType = %s AND ssh.keyBody = %s',
13+
'SELECT userName, keyBody, keyType FROM %T u JOIN %T ssh
14+
ON u.phid = ssh.userPHID',
2815
$user_dao->getTableName(),
29-
$ssh_dao->getTableName(),
30-
$type,
31-
$body);
16+
$ssh_dao->getTableName());
3217

33-
if (!$row) {
34-
exit(1);
35-
}
18+
$bin = $root.'/bin/ssh-exec';
19+
foreach ($rows as $row) {
20+
$user = $row['userName'];
3621

37-
$user = idx($row, 'userName');
22+
$cmd = csprintf('%s --phabricator-ssh-user %s', $bin, $user);
23+
// This is additional escaping for the SSH 'command="..."' string.
24+
$cmd = addcslashes($cmd, '"\\');
3825

39-
if (!$user) {
40-
exit(1);
41-
}
26+
// Strip out newlines and other nonsense from the key type and key body.
27+
28+
$type = $row['keyType'];
29+
$type = preg_replace('@[\x00-\x20]+@', '', $type);
30+
31+
$key = $row['keyBody'];
32+
$key = preg_replace('@[\x00-\x20]+@', '', $key);
4233

43-
if (!PhabricatorUser::validateUsername($user)) {
44-
exit(1);
34+
35+
$options = array(
36+
'command="'.$cmd.'"',
37+
'no-port-forwarding',
38+
'no-X11-forwarding',
39+
'no-agent-forwarding',
40+
'no-pty',
41+
);
42+
$options = implode(',', $options);
43+
44+
$lines[] = $options.' '.$type.' '.$key."\n";
4545
}
4646

47-
$bin = $root.'/bin/ssh-exec';
48-
$cmd = csprintf('%s --phabricator-ssh-user %s', $bin, $user);
49-
// This is additional escaping for the SSH 'command="..."' string.
50-
$cmd = str_replace('"', '\\"', $cmd);
51-
52-
$options = array(
53-
'command="'.$cmd.'"',
54-
'no-port-forwarding',
55-
'no-X11-forwarding',
56-
'no-agent-forwarding',
57-
'no-pty',
58-
);
59-
60-
echo implode(',', $options);
47+
echo implode('', $lines);
6148
exit(0);

scripts/ssh/ssh-exec.php

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,25 @@
44
$root = dirname(dirname(dirname(__FILE__)));
55
require_once $root.'/scripts/__init_script__.php';
66

7-
$original_command = getenv('SSH_ORIGINAL_COMMAND');
8-
$original_argv = id(new PhutilShellLexer())->splitArguments($original_command);
9-
$argv = array_merge($argv, $original_argv);
10-
7+
// First, figure out the authenticated user.
118
$args = new PhutilArgumentParser($argv);
129
$args->setTagline('receive SSH requests');
1310
$args->setSynopsis(<<<EOSYNOPSIS
14-
**ssh-exec** --phabricator-ssh-user __user__ __commmand__ [__options__]
11+
**ssh-exec** --phabricator-ssh-user __user__ [--ssh-command __commmand__]
1512
Receive SSH requests.
16-
1713
EOSYNOPSIS
1814
);
1915

20-
// NOTE: Do NOT parse standard arguments. Arguments are coming from a remote
21-
// client over SSH, and they should not be able to execute "--xprofile",
22-
// "--recon", etc.
23-
24-
$args->parsePartial(
16+
$args->parse(
2517
array(
2618
array(
2719
'name' => 'phabricator-ssh-user',
2820
'param' => 'username',
2921
),
22+
array(
23+
'name' => 'ssh-command',
24+
'param' => 'command',
25+
),
3026
));
3127

3228
try {
@@ -46,24 +42,33 @@
4642
throw new Exception("You have been exiled.");
4743
}
4844

45+
if ($args->getArg('ssh-command')) {
46+
$original_command = $args->getArg('ssh-command');
47+
} else {
48+
$original_command = getenv('SSH_ORIGINAL_COMMAND');
49+
}
50+
51+
// Now, rebuild the original command.
52+
$original_argv = id(new PhutilShellLexer())
53+
->splitArguments($original_command);
54+
if (!$original_argv) {
55+
throw new Exception("No interactive logins.");
56+
}
57+
$command = head($original_argv);
58+
array_unshift($original_argv, 'phabricator-ssh-exec');
59+
60+
$original_args = new PhutilArgumentParser($original_argv);
61+
4962
$workflows = array(
5063
new ConduitSSHWorkflow(),
5164
);
5265

53-
// This duplicates logic in parseWorkflows(), but allows us to raise more
54-
// concise/relevant exceptions when the client is a remote SSH.
55-
$remain = $args->getUnconsumedArgumentVector();
56-
if (empty($remain)) {
57-
throw new Exception("No interactive logins.");
58-
} else {
59-
$command = head($remain);
60-
$workflow_names = mpull($workflows, 'getName', 'getName');
61-
if (empty($workflow_names[$command])) {
62-
throw new Exception("Invalid command.");
63-
}
66+
$workflow_names = mpull($workflows, 'getName', 'getName');
67+
if (empty($workflow_names[$command])) {
68+
throw new Exception("Invalid command.");
6469
}
6570

66-
$workflow = $args->parseWorkflows($workflows);
71+
$workflow = $original_args->parseWorkflows($workflows);
6772
$workflow->setUser($user);
6873

6974
$sock_stdin = fopen('php://stdin', 'r');
@@ -82,7 +87,7 @@
8287
$metrics_channel = new PhutilMetricsChannel($socket_channel);
8388
$workflow->setIOChannel($metrics_channel);
8489

85-
$err = $workflow->execute($args);
90+
$err = $workflow->execute($original_args);
8691

8792
$metrics_channel->flush();
8893
} catch (Exception $ex) {

src/applications/conduit/ssh/ConduitSSHWorkflow.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function execute(PhutilArgumentParser $args) {
3131
throw new Exception("Invalid JSON input.");
3232
}
3333

34-
$params = idx($raw_params, 'params', array());
34+
$params = idx($raw_params, 'params', '[]');
3535
$params = json_decode($params, true);
3636
$metadata = idx($params, '__conduit__', array());
3737
unset($params['__conduit__']);

0 commit comments

Comments
 (0)