Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14475858
D10790.id.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
15 KB
Referenced Files
None
Subscribers
None
D10790.id.diff
View Options
diff --git a/scripts/ssh/ssh-auth-key.php b/scripts/ssh/ssh-auth-key.php
--- a/scripts/ssh/ssh-auth-key.php
+++ b/scripts/ssh/ssh-auth-key.php
@@ -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, '"\\');
diff --git a/scripts/ssh/ssh-auth.php b/scripts/ssh/ssh-auth.php
--- a/scripts/ssh/ssh-auth.php
+++ b/scripts/ssh/ssh-auth.php
@@ -4,25 +4,27 @@
$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.
@@ -30,13 +32,12 @@
// 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',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -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',
@@ -4381,6 +4383,8 @@
'PhabricatorAuthProviderConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorAuthRegisterController' => 'PhabricatorAuthController',
'PhabricatorAuthRevokeTokenController' => 'PhabricatorAuthController',
+ 'PhabricatorAuthSSHKeyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+ 'PhabricatorAuthSSHPublicKey' => 'Phobject',
'PhabricatorAuthSession' => array(
'PhabricatorAuthDAO',
'PhabricatorPolicyInterface',
@@ -5627,7 +5631,10 @@
'PhabricatorUserProfileEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorUserRealNameField' => 'PhabricatorUserCustomField',
'PhabricatorUserRolesField' => 'PhabricatorUserCustomField',
- 'PhabricatorUserSSHKey' => 'PhabricatorUserDAO',
+ 'PhabricatorUserSSHKey' => array(
+ 'PhabricatorUserDAO',
+ 'PhabricatorPolicyInterface',
+ ),
'PhabricatorUserSchemaSpec' => 'PhabricatorConfigSchemaSpec',
'PhabricatorUserSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
'PhabricatorUserSinceField' => 'PhabricatorUserCustomField',
diff --git a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php
new file mode 100644
--- /dev/null
+++ b/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';
+ }
+
+}
diff --git a/src/applications/auth/storage/PhabricatorAuthSSHPublicKey.php b/src/applications/auth/storage/PhabricatorAuthSSHPublicKey.php
new file mode 100644
--- /dev/null
+++ b/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;
+ }
+
+}
diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php b/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php
--- a/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php
+++ b/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php
@@ -45,12 +45,16 @@
$id = nonempty($edit, $delete);
- if ($id && is_numeric($id)) {
- // NOTE: This prevents editing/deleting of keys not owned by the user.
- $key = id(new PhabricatorUserSSHKey())->loadOneWhere(
- 'userPHID = %s AND id = %d',
- $user->getPHID(),
- (int)$id);
+ if ($id) {
+ $key = id(new PhabricatorAuthSSHKeyQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($id))
+ ->requireCapabilities(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ ))
+ ->executeOne();
if (!$key) {
return new Aphront404Response();
}
@@ -77,7 +81,11 @@
} else {
try {
- list($type, $body, $comment) = self::parsePublicKey($entire_key);
+ $public_key = PhabricatorAuthSSHPublicKey::newFromRawKey($entire_key);
+
+ $type = $public_key->getType();
+ $body = $public_key->getBody();
+ $comment = $public_key->getComment();
$key->setKeyType($type);
$key->setKeyBody($body);
@@ -153,9 +161,10 @@
$user = $this->getUser();
$viewer = $request->getUser();
- $keys = id(new PhabricatorUserSSHKey())->loadAllWhere(
- 'userPHID = %s',
- $user->getPHID());
+ $keys = id(new PhabricatorAuthSSHKeyQuery())
+ ->setViewer($viewer)
+ ->withObjectPHIDs(array($user->getPHID()))
+ ->execute();
$rows = array();
foreach ($keys as $key) {
@@ -294,7 +303,10 @@
'viewPolicy' => $viewer->getPHID(),
));
- list($type, $body, $comment) = self::parsePublicKey($public_key);
+ $public_key = PhabricatorAuthSSHPublicKey::newFromRawKey($public_key);
+
+ $type = $public_key->getType();
+ $body = $public_key->getBody();
$key = id(new PhabricatorUserSSHKey())
->setUserPHID($user->getPHID())
@@ -377,53 +389,4 @@
->setDialog($dialog);
}
- private static function parsePublicKey($entire_key) {
- $parts = str_replace("\n", '', trim($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 your public key, not your 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));
- }
-
- return array($type, $body, $comment);
- }
-
}
diff --git a/src/applications/settings/storage/PhabricatorUserSSHKey.php b/src/applications/settings/storage/PhabricatorUserSSHKey.php
--- a/src/applications/settings/storage/PhabricatorUserSSHKey.php
+++ b/src/applications/settings/storage/PhabricatorUserSSHKey.php
@@ -1,6 +1,8 @@
<?php
-final class PhabricatorUserSSHKey extends PhabricatorUserDAO {
+final class PhabricatorUserSSHKey
+ extends PhabricatorUserDAO
+ implements PhabricatorPolicyInterface {
protected $userPHID;
protected $name;
@@ -9,6 +11,12 @@
protected $keyHash;
protected $keyComment;
+ private $object = self::ATTACHABLE;
+
+ public function getObjectPHID() {
+ return $this->getUserPHID();
+ }
+
public function getConfiguration() {
return array(
self::CONFIG_COLUMN_SCHEMA => array(
@@ -42,4 +50,37 @@
return trim(implode(' ', $parts));
}
+ public function getObject() {
+ return $this->assertAttached($this->object);
+ }
+
+ public function attachObject($object) {
+ $this->object = $object;
+ return $this;
+ }
+
+
+/* -( PhabricatorPolicyInterface )----------------------------------------- */
+
+
+ public function getCapabilities() {
+ return array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ );
+ }
+
+ public function getPolicy($capability) {
+ return $this->getObject()->getPolicy($capability);
+ }
+
+ public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+ return $this->getObject()->hasAutomaticCapability($capability, $viewer);
+ }
+
+ public function describeAutomaticCapability($capability) {
+ return pht(
+ 'SSH keys inherit the policies of the user or object they authenticate.');
+ }
+
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Dec 28, 9:28 PM (1 h, 19 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6941194
Default Alt Text
D10790.id.diff (15 KB)
Attached To
Mode
D10790: Add a query/policy layer on top of SSH keys for Almanac
Attached
Detach File
Event Timeline
Log In to Comment