Skip to content

Commit

Permalink
Add a query/policy layer on top of SSH keys for Almanac
Browse files Browse the repository at this point in the history
Summary:
Ref T5833. Currently, SSH keys are associated only with users, and are a bit un-modern. I want to let Almanac Devices have SSH keys so devices in a cluster can identify to one another.

For example, with hosted installs, initialization will go something like this:

  - A request comes in for `company.phacility.com`.
  - A SiteSource (from D10787) makes a Conduit call to Almanac on the master install to check if `company` is a valid install and pull config if it is.
  - This call can be signed with an SSH key which identifies a trusted Almanac Device.

In the cluster case, a web host can make an authenticated call to a repository host with similar key signing.

To move toward this, put a proper Query class on top of SSH key access (this diff). In following diffs, I'll:

  - Rename `userPHID` to `objectPHID`.
  - Move this to the `auth` database.
  - Provide UI for device/key association.

An alternative approach would be to build some kind of special token layer in Conduit, but I think that would be a lot harder to manage in the hosting case. This gives us a more direct attack on trusting requests from machines and recognizing machines as first (well, sort of second-class) actors without needing things like fake user accounts.

Test Plan:
  - Added and removed SSH keys.
  - Added and removed SSH keys from a bot account.
  - Tried to edit an unonwned SSH key (denied).
  - Ran `bin/ssh-auth`, got sensible output.
  - Ran `bin/ssh-auth-key`, got sensible output.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5833

Differential Revision: https://secure.phabricator.com/D10790
  • Loading branch information
epriestley committed Nov 6, 2014
1 parent 3ea31c9 commit 6f0d3b0
Show file tree
Hide file tree
Showing 7 changed files with 293 additions and 111 deletions.
44 changes: 12 additions & 32 deletions scripts/ssh/ssh-auth-key.php
Expand Up @@ -4,48 +4,28 @@
$root = dirname(dirname(dirname(__FILE__)));
require_once $root.'/scripts/__init_script__.php';

$cert = file_get_contents('php://stdin');

if (!$cert) {
exit(1);
}

$parts = preg_split('/\s+/', $cert);
if (count($parts) < 2) {
try {
$cert = file_get_contents('php://stdin');
$public_key = PhabricatorAuthSSHPublicKey::newFromRawKey($cert);
} catch (Exception $ex) {
exit(1);
}

list($type, $body) = $parts;

$user_dao = new PhabricatorUser();
$ssh_dao = new PhabricatorUserSSHKey();
$conn_r = $user_dao->establishConnection('r');

$row = queryfx_one(
$conn_r,
'SELECT userName FROM %T u JOIN %T ssh ON u.phid = ssh.userPHID
WHERE ssh.keyType = %s AND ssh.keyBody = %s',
$user_dao->getTableName(),
$ssh_dao->getTableName(),
$type,
$body);

if (!$row) {
exit(1);
}

$user = idx($row, 'userName');

if (!$user) {
$key = id(new PhabricatorAuthSSHKeyQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withKeys(array($public_key))
->executeOne();
if (!$key) {
exit(1);
}

if (!PhabricatorUser::validateUsername($user)) {
$object = $key->getObject();
if (!($object instanceof PhabricatorUser)) {
exit(1);
}

$bin = $root.'/bin/ssh-exec';
$cmd = csprintf('%s --phabricator-ssh-user %s', $bin, $user);
$cmd = csprintf('%s --phabricator-ssh-user %s', $bin, $object->getUsername());
// This is additional escaping for the SSH 'command="..."' string.
$cmd = addcslashes($cmd, '"\\');

Expand Down
35 changes: 18 additions & 17 deletions scripts/ssh/ssh-auth.php
Expand Up @@ -4,39 +4,40 @@
$root = dirname(dirname(dirname(__FILE__)));
require_once $root.'/scripts/__init_script__.php';

$user_dao = new PhabricatorUser();
$ssh_dao = new PhabricatorUserSSHKey();
$conn_r = $user_dao->establishConnection('r');

$rows = queryfx_all(
$conn_r,
'SELECT userName, keyBody, keyType FROM %T u JOIN %T ssh
ON u.phid = ssh.userPHID',
$user_dao->getTableName(),
$ssh_dao->getTableName());

if (!$rows) {
$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);
}

$bin = $root.'/bin/ssh-exec';
foreach ($rows as $row) {
$user = $row['userName'];
foreach ($keys as $ssh_key) {
$user = $ssh_key->getObject()->getUsername();

$cmd = csprintf('%s --phabricator-ssh-user %s', $bin, $user);
// This is additional escaping for the SSH 'command="..."' string.
$cmd = addcslashes($cmd, '"\\');

// Strip out newlines and other nonsense from the key type and key body.

$type = $row['keyType'];
$type = $ssh_key->getKeyType();
$type = preg_replace('@[\x00-\x20]+@', '', $type);

$key = $row['keyBody'];
$key = $ssh_key->getKeyBody();
$key = preg_replace('@[\x00-\x20]+@', '', $key);


$options = array(
'command="'.$cmd.'"',
'no-port-forwarding',
Expand Down
9 changes: 8 additions & 1 deletion src/__phutil_library_map__.php
Expand Up @@ -1317,6 +1317,8 @@
'PhabricatorAuthProviderConfigTransactionQuery' => 'applications/auth/query/PhabricatorAuthProviderConfigTransactionQuery.php',
'PhabricatorAuthRegisterController' => 'applications/auth/controller/PhabricatorAuthRegisterController.php',
'PhabricatorAuthRevokeTokenController' => 'applications/auth/controller/PhabricatorAuthRevokeTokenController.php',
'PhabricatorAuthSSHKeyQuery' => 'applications/auth/query/PhabricatorAuthSSHKeyQuery.php',
'PhabricatorAuthSSHPublicKey' => 'applications/auth/storage/PhabricatorAuthSSHPublicKey.php',
'PhabricatorAuthSession' => 'applications/auth/storage/PhabricatorAuthSession.php',
'PhabricatorAuthSessionEngine' => 'applications/auth/engine/PhabricatorAuthSessionEngine.php',
'PhabricatorAuthSessionGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthSessionGarbageCollector.php',
Expand Down Expand Up @@ -4381,6 +4383,8 @@
'PhabricatorAuthProviderConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorAuthRegisterController' => 'PhabricatorAuthController',
'PhabricatorAuthRevokeTokenController' => 'PhabricatorAuthController',
'PhabricatorAuthSSHKeyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorAuthSSHPublicKey' => 'Phobject',
'PhabricatorAuthSession' => array(
'PhabricatorAuthDAO',
'PhabricatorPolicyInterface',
Expand Down Expand Up @@ -5627,7 +5631,10 @@
'PhabricatorUserProfileEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorUserRealNameField' => 'PhabricatorUserCustomField',
'PhabricatorUserRolesField' => 'PhabricatorUserCustomField',
'PhabricatorUserSSHKey' => 'PhabricatorUserDAO',
'PhabricatorUserSSHKey' => array(
'PhabricatorUserDAO',
'PhabricatorPolicyInterface',
),
'PhabricatorUserSchemaSpec' => 'PhabricatorConfigSchemaSpec',
'PhabricatorUserSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
'PhabricatorUserSinceField' => 'PhabricatorUserCustomField',
Expand Down
104 changes: 104 additions & 0 deletions src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php
@@ -0,0 +1,104 @@
<?php

final class PhabricatorAuthSSHKeyQuery
extends PhabricatorCursorPagedPolicyAwareQuery {

private $ids;
private $objectPHIDs;
private $keys;

public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
}

public function withObjectPHIDs(array $object_phids) {
$this->objectPHIDs = $object_phids;
return $this;
}

public function withKeys(array $keys) {
assert_instances_of($keys, 'PhabricatorAuthSSHPublicKey');
$this->keys = $keys;
return $this;
}

protected function loadPage() {
$table = new PhabricatorUserSSHKey();
$conn_r = $table->establishConnection('r');

$data = queryfx_all(
$conn_r,
'SELECT * FROM %T %Q %Q %Q',
$table->getTableName(),
$this->buildWhereClause($conn_r),
$this->buildOrderClause($conn_r),
$this->buildLimitClause($conn_r));

return $table->loadAllFromArray($data);
}

protected function willFilterPage(array $keys) {
$object_phids = mpull($keys, 'getObjectPHID');

$objects = id(new PhabricatorObjectQuery())
->setViewer($this->getViewer())
->setParentQuery($this)
->withPHIDs($object_phids)
->execute();
$objects = mpull($objects, null, 'getPHID');

foreach ($keys as $key => $ssh_key) {
$object = idx($objects, $ssh_key->getObjectPHID());
if (!$object) {
unset($keys[$key]);
continue;
}
$ssh_key->attachObject($object);
}

return $keys;
}

protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
$where = array();

if ($this->ids !== null) {
$where[] = qsprintf(
$conn_r,
'id IN (%Ld)',
$this->ids);
}

if ($this->objectPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'userPHID IN (%Ls)',
$this->objectPHIDs);
}

if ($this->keys !== null) {
// TODO: This could take advantage of a better key, and the hashing
// scheme for this table is a bit nonstandard and questionable.

$sql = array();
foreach ($this->keys as $key) {
$sql[] = qsprintf(
$conn_r,
'(keyType = %s AND keyBody = %s)',
$key->getType(),
$key->getBody());
}
$where[] = implode(' OR ', $sql);
}

$where[] = $this->buildPagingClause($conn_r);

return $this->formatWhereClause($where);
}

public function getQueryApplicationClass() {
return 'PhabricatorAuthApplication';
}

}
86 changes: 86 additions & 0 deletions src/applications/auth/storage/PhabricatorAuthSSHPublicKey.php
@@ -0,0 +1,86 @@
<?php

/**
* Data structure representing a raw public key.
*/
final class PhabricatorAuthSSHPublicKey extends Phobject {

private $type;
private $body;
private $comment;

private function __construct() {
// <internal>
}

public static function newFromRawKey($entire_key) {
$entire_key = trim($entire_key);
if (!strlen($entire_key)) {
throw new Exception(pht('No public key was provided.'));
}

$parts = str_replace("\n", '', $entire_key);

// The third field (the comment) can have spaces in it, so split this
// into a maximum of three parts.
$parts = preg_split('/\s+/', $parts, 3);

if (preg_match('/private\s*key/i', $entire_key)) {
// Try to give the user a better error message if it looks like
// they uploaded a private key.
throw new Exception(pht('Provide a public key, not a private key!'));
}

switch (count($parts)) {
case 1:
throw new Exception(
pht('Provided public key is not properly formatted.'));
case 2:
// Add an empty comment part.
$parts[] = '';
break;
case 3:
// This is the expected case.
break;
}

list($type, $body, $comment) = $parts;

$recognized_keys = array(
'ssh-dsa',
'ssh-dss',
'ssh-rsa',
'ecdsa-sha2-nistp256',
'ecdsa-sha2-nistp384',
'ecdsa-sha2-nistp521',
);

if (!in_array($type, $recognized_keys)) {
$type_list = implode(', ', $recognized_keys);
throw new Exception(
pht(
'Public key type should be one of: %s',
$type_list));
}

$public_key = new PhabricatorAuthSSHPublicKey();
$public_key->type = $type;
$public_key->body = $body;
$public_key->comment = $comment;

return $public_key;
}

public function getType() {
return $this->type;
}

public function getBody() {
return $this->body;
}

public function getComment() {
return $this->comment;
}

}

0 comments on commit 6f0d3b0

Please sign in to comment.