Page MenuHomePhabricator

D10805.diff
No OneTemporary

D10805.diff

diff --git a/resources/sql/autopatches/20141107.ssh.1.colname.sql b/resources/sql/autopatches/20141107.ssh.1.colname.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141107.ssh.1.colname.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ CHANGE userPHID objectPHID VARBINARY(64) NOT NULL;
diff --git a/resources/sql/autopatches/20141107.ssh.2.keyhash.sql b/resources/sql/autopatches/20141107.ssh.2.keyhash.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141107.ssh.2.keyhash.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ DROP COLUMN keyHash;
diff --git a/resources/sql/autopatches/20141107.ssh.3.keyindex.sql b/resources/sql/autopatches/20141107.ssh.3.keyindex.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141107.ssh.3.keyindex.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ ADD COLUMN keyIndex BINARY(12);
diff --git a/resources/sql/autopatches/20141107.ssh.4.keymig.php b/resources/sql/autopatches/20141107.ssh.4.keymig.php
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141107.ssh.4.keymig.php
@@ -0,0 +1,50 @@
+<?php
+
+$table = new PhabricatorAuthSSHKey();
+$conn_w = $table->establishConnection('w');
+
+echo "Updating SSH public key indexes...\n";
+
+$keys = new LiskMigrationIterator($table);
+foreach ($keys as $key) {
+ $id = $key->getID();
+
+ echo "Updating key {$id}...\n";
+
+ try {
+ $hash = $key->toPublicKey()->getHash();
+ } catch (Exception $ex) {
+ echo "Key has bad format! Removing key.\n";
+ queryfx(
+ $conn_w,
+ 'DELETE FROM %T WHERE id = %d',
+ $table->getTableName(),
+ $id);
+ continue;
+ }
+
+ $collision = queryfx_all(
+ $conn_w,
+ 'SELECT * FROM %T WHERE keyIndex = %s AND id < %d',
+ $table->getTableName(),
+ $hash,
+ $key->getID());
+ if ($collision) {
+ echo "Key is a duplicate! Removing key.\n";
+ queryfx(
+ $conn_w,
+ 'DELETE FROM %T WHERE id = %d',
+ $table->getTableName(),
+ $id);
+ continue;
+ }
+
+ queryfx(
+ $conn_w,
+ 'UPDATE %T SET keyIndex = %s WHERE id = %d',
+ $table->getTableName(),
+ $hash,
+ $key->getID());
+}
+
+echo "Done.\n";
diff --git a/resources/sql/autopatches/20141107.ssh.5.indexnull.sql b/resources/sql/autopatches/20141107.ssh.5.indexnull.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141107.ssh.5.indexnull.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ CHANGE keyIndex keyIndex BINARY(12) NOT NULL;
diff --git a/resources/sql/autopatches/20141107.ssh.6.indexkey.sql b/resources/sql/autopatches/20141107.ssh.6.indexkey.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141107.ssh.6.indexkey.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ ADD UNIQUE KEY `key_unique` (keyIndex);
diff --git a/resources/sql/autopatches/20141107.ssh.7.colnull.sql b/resources/sql/autopatches/20141107.ssh.7.colnull.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141107.ssh.7.colnull.sql
@@ -0,0 +1,23 @@
+UPDATE {$NAMESPACE}_auth.auth_sshkey
+ SET name = '' WHERE name IS NULL;
+
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ CHANGE name name VARCHAR(255) COLLATE {$COLLATE_TEXT} NOT NULL;
+
+UPDATE {$NAMESPACE}_auth.auth_sshkey
+ SET keyType = '' WHERE keyType IS NULL;
+
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ CHANGE keyType keyType VARCHAR(255) COLLATE {$COLLATE_TEXT} NOT NULL;
+
+UPDATE {$NAMESPACE}_auth.auth_sshkey
+ SET keyBody = '' WHERE keyBody IS NULL;
+
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ CHANGE keyBody keyBody LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL;
+
+UPDATE {$NAMESPACE}_auth.auth_sshkey
+ SET keyComment = '' WHERE keyComment IS NULL;
+
+ALTER TABLE {$NAMESPACE}_auth.auth_sshkey
+ CHANGE keyComment keyComment VARCHAR(255) COLLATE {$COLLATE_TEXT} NOT NULL;
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
@@ -34,9 +34,15 @@
$type = $ssh_key->getKeyType();
$type = preg_replace('@[\x00-\x20]+@', '', $type);
+ if (!strlen($type)) {
+ continue;
+ }
$key = $ssh_key->getKeyBody();
$key = preg_replace('@[\x00-\x20]+@', '', $key);
+ if (!strlen($key)) {
+ continue;
+ }
$options = array(
'command="'.$cmd.'"',
diff --git a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php
--- a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php
+++ b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php
@@ -73,21 +73,18 @@
if ($this->objectPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
- 'userPHID IN (%Ls)',
+ 'objectPHID 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)',
+ '(keyType = %s AND keyIndex = %s)',
$key->getType(),
- $key->getBody());
+ $key->getHash());
}
$where[] = implode(' OR ', $sql);
}
diff --git a/src/applications/auth/storage/PhabricatorAuthSSHKey.php b/src/applications/auth/storage/PhabricatorAuthSSHKey.php
--- a/src/applications/auth/storage/PhabricatorAuthSSHKey.php
+++ b/src/applications/auth/storage/PhabricatorAuthSSHKey.php
@@ -4,43 +4,45 @@
extends PhabricatorAuthDAO
implements PhabricatorPolicyInterface {
- protected $userPHID;
+ protected $objectPHID;
protected $name;
protected $keyType;
+ protected $keyIndex;
protected $keyBody;
- protected $keyHash;
- protected $keyComment;
+ protected $keyComment = '';
private $object = self::ATTACHABLE;
- public function getObjectPHID() {
- return $this->getUserPHID();
- }
-
public function getConfiguration() {
return array(
self::CONFIG_COLUMN_SCHEMA => array(
- 'keyHash' => 'bytes32',
- 'keyComment' => 'text255?',
-
- // T6203/NULLABILITY
- // These seem like they should not be nullable.
- 'name' => 'text255?',
- 'keyType' => 'text255?',
- 'keyBody' => 'text?',
+ 'name' => 'text255',
+ 'keyType' => 'text255',
+ 'keyIndex' => 'bytes12',
+ 'keyBody' => 'text',
+ 'keyComment' => 'text255',
),
self::CONFIG_KEY_SCHEMA => array(
- 'userPHID' => array(
- 'columns' => array('userPHID'),
+ 'key_object' => array(
+ 'columns' => array('objectPHID'),
),
- 'keyHash' => array(
- 'columns' => array('keyHash'),
+ 'key_unique' => array(
+ 'columns' => array('keyIndex'),
'unique' => true,
),
),
) + parent::getConfiguration();
}
+ public function save() {
+ $this->setKeyIndex($this->toPublicKey()->getHash());
+ return parent::save();
+ }
+
+ public function toPublicKey() {
+ return PhabricatorAuthSSHPublicKey::newFromStoredKey($this);
+ }
+
public function getEntireKey() {
$parts = array(
$this->getKeyType(),
@@ -60,6 +62,8 @@
}
+
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
diff --git a/src/applications/auth/storage/PhabricatorAuthSSHPublicKey.php b/src/applications/auth/storage/PhabricatorAuthSSHPublicKey.php
--- a/src/applications/auth/storage/PhabricatorAuthSSHPublicKey.php
+++ b/src/applications/auth/storage/PhabricatorAuthSSHPublicKey.php
@@ -13,6 +13,15 @@
// <internal>
}
+ public static function newFromStoredKey(PhabricatorAuthSSHKey $key) {
+ $public_key = new PhabricatorAuthSSHPublicKey();
+ $public_key->type = $key->getKeyType();
+ $public_key->body = $key->getKeyBody();
+ $public_key->comment = $key->getKeyComment();
+
+ return $public_key;
+ }
+
public static function newFromRawKey($entire_key) {
$entire_key = trim($entire_key);
if (!strlen($entire_key)) {
@@ -83,4 +92,11 @@
return $this->comment;
}
+ public function getHash() {
+ $body = $this->getBody();
+ $body = trim($body);
+ $body = rtrim($body, '=');
+ return PhabricatorHash::digestForIndex($body);
+ }
+
}
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
--- a/src/applications/people/storage/PhabricatorUser.php
+++ b/src/applications/people/storage/PhabricatorUser.php
@@ -897,7 +897,7 @@
}
$keys = id(new PhabricatorAuthSSHKey())->loadAllWhere(
- 'userPHID = %s',
+ 'objectPHID = %s',
$this->getPHID());
foreach ($keys as $key) {
$key->delete();
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
@@ -44,8 +44,7 @@
$this->getPanelURI());
$id = nonempty($edit, $delete);
-
- if ($id) {
+ if ($id && (int)$id) {
$key = id(new PhabricatorAuthSSHKeyQuery())
->setViewer($viewer)
->withIDs(array($id))
@@ -59,8 +58,8 @@
return new Aphront404Response();
}
} else {
- $key = new PhabricatorAuthSSHKey();
- $key->setUserPHID($user->getPHID());
+ $key = id(new PhabricatorAuthSSHKey())
+ ->setObjectPHID($user->getPHID());
}
if ($delete) {
@@ -89,7 +88,6 @@
$key->setKeyType($type);
$key->setKeyBody($body);
- $key->setKeyHash(md5($body));
$key->setKeyComment($comment);
$e_key = null;
@@ -309,11 +307,10 @@
$body = $public_key->getBody();
$key = id(new PhabricatorAuthSSHKey())
- ->setUserPHID($user->getPHID())
+ ->setObjectPHID($user->getPHID())
->setName('id_rsa_phabricator')
->setKeyType($type)
->setKeyBody($body)
- ->setKeyHash(md5($body))
->setKeyComment(pht('Generated'))
->save();

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 24, 10:56 PM (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6719153
Default Alt Text
D10805.diff (10 KB)

Event Timeline