Page MenuHomePhabricator

D17630.diff
No OneTemporary

D17630.diff

diff --git a/resources/sql/autopatches/20170406.hmac.01.keystore.sql b/resources/sql/autopatches/20170406.hmac.01.keystore.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20170406.hmac.01.keystore.sql
@@ -0,0 +1,8 @@
+CREATE TABLE {$NAMESPACE}_auth.auth_hmackey (
+ id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+ keyName VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT},
+ keyValue VARCHAR(128) NOT NULL COLLATE {$COLLATE_TEXT},
+ dateCreated INT UNSIGNED NOT NULL,
+ dateModified INT UNSIGNED NOT NULL,
+ UNIQUE KEY `key_name` (keyName)
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};
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
@@ -1920,6 +1920,7 @@
'PhabricatorAuthFactorConfig' => 'applications/auth/storage/PhabricatorAuthFactorConfig.php',
'PhabricatorAuthFactorTestCase' => 'applications/auth/factor/__tests__/PhabricatorAuthFactorTestCase.php',
'PhabricatorAuthFinishController' => 'applications/auth/controller/PhabricatorAuthFinishController.php',
+ 'PhabricatorAuthHMACKey' => 'applications/auth/storage/PhabricatorAuthHMACKey.php',
'PhabricatorAuthHighSecurityRequiredException' => 'applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php',
'PhabricatorAuthHighSecurityToken' => 'applications/auth/data/PhabricatorAuthHighSecurityToken.php',
'PhabricatorAuthInvite' => 'applications/auth/storage/PhabricatorAuthInvite.php',
@@ -2864,6 +2865,7 @@
'PhabricatorGuideModule' => 'applications/guides/module/PhabricatorGuideModule.php',
'PhabricatorGuideModuleController' => 'applications/guides/controller/PhabricatorGuideModuleController.php',
'PhabricatorGuideQuickStartModule' => 'applications/guides/module/PhabricatorGuideQuickStartModule.php',
+ 'PhabricatorHMACTestCase' => 'infrastructure/util/__tests__/PhabricatorHMACTestCase.php',
'PhabricatorHTTPParameterTypeTableView' => 'applications/config/view/PhabricatorHTTPParameterTypeTableView.php',
'PhabricatorHandleList' => 'applications/phid/handle/pool/PhabricatorHandleList.php',
'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/PhabricatorHandleObjectSelectorDataView.php',
@@ -6900,6 +6902,7 @@
'PhabricatorAuthFactorConfig' => 'PhabricatorAuthDAO',
'PhabricatorAuthFactorTestCase' => 'PhabricatorTestCase',
'PhabricatorAuthFinishController' => 'PhabricatorAuthController',
+ 'PhabricatorAuthHMACKey' => 'PhabricatorAuthDAO',
'PhabricatorAuthHighSecurityRequiredException' => 'Exception',
'PhabricatorAuthHighSecurityToken' => 'Phobject',
'PhabricatorAuthInvite' => array(
@@ -7998,6 +8001,7 @@
'PhabricatorGuideModule' => 'Phobject',
'PhabricatorGuideModuleController' => 'PhabricatorGuideController',
'PhabricatorGuideQuickStartModule' => 'PhabricatorGuideModule',
+ 'PhabricatorHMACTestCase' => 'PhabricatorTestCase',
'PhabricatorHTTPParameterTypeTableView' => 'AphrontView',
'PhabricatorHandleList' => array(
'Phobject',
diff --git a/src/applications/auth/storage/PhabricatorAuthHMACKey.php b/src/applications/auth/storage/PhabricatorAuthHMACKey.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/storage/PhabricatorAuthHMACKey.php
@@ -0,0 +1,24 @@
+<?php
+
+final class PhabricatorAuthHMACKey
+ extends PhabricatorAuthDAO {
+
+ protected $keyName;
+ protected $keyValue;
+
+ protected function getConfiguration() {
+ return array(
+ self::CONFIG_COLUMN_SCHEMA => array(
+ 'keyName' => 'text64',
+ 'keyValue' => 'text128',
+ ),
+ self::CONFIG_KEY_SCHEMA => array(
+ 'key_name' => array(
+ 'columns' => array('keyName'),
+ 'unique' => true,
+ ),
+ ),
+ ) + parent::getConfiguration();
+ }
+
+}
diff --git a/src/applications/config/option/PhabricatorSecurityConfigOptions.php b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
--- a/src/applications/config/option/PhabricatorSecurityConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
@@ -109,7 +109,8 @@
'Default key for HMAC digests where the key is not important '.
'(i.e., the hash itself is secret). You can change this if you '.
'want (to any other string), but doing so will break existing '.
- 'sessions and CSRF tokens.')),
+ 'sessions and CSRF tokens. This option is deprecated. Newer '.
+ 'code automatically manages HMAC keys.')),
$this->newOption('security.require-https', 'bool', false)
->setLocked(true)
->setSummary(
diff --git a/src/applications/files/engine/PhabricatorFileStorageEngine.php b/src/applications/files/engine/PhabricatorFileStorageEngine.php
--- a/src/applications/files/engine/PhabricatorFileStorageEngine.php
+++ b/src/applications/files/engine/PhabricatorFileStorageEngine.php
@@ -16,6 +16,8 @@
*/
abstract class PhabricatorFileStorageEngine extends Phobject {
+ const HMAC_INTEGRITY = 'file.integrity';
+
/**
* Construct a new storage engine.
*
@@ -367,12 +369,14 @@
$data,
PhabricatorFileStorageFormat $format) {
- $data_hash = PhabricatorHash::digest($data);
+ $hmac_name = self::HMAC_INTEGRITY;
+
+ $data_hash = PhabricatorHash::digestWithNamedKey($data, $hmac_name);
$format_hash = $format->newFormatIntegrityHash();
$full_hash = "{$data_hash}/{$format_hash}";
- return PhabricatorHash::digest($full_hash);
+ return PhabricatorHash::digestWithNamedKey($full_hash, $hmac_name);
}
}
diff --git a/src/applications/files/format/PhabricatorFileAES256StorageFormat.php b/src/applications/files/format/PhabricatorFileAES256StorageFormat.php
--- a/src/applications/files/format/PhabricatorFileAES256StorageFormat.php
+++ b/src/applications/files/format/PhabricatorFileAES256StorageFormat.php
@@ -67,7 +67,9 @@
$input = self::FORMATKEY.'/iv:'.$iv_envelope->openEnvelope();
- return PhabricatorHash::digest($input);
+ return PhabricatorHash::digestWithNamedKey(
+ $input,
+ PhabricatorFileStorageEngine::HMAC_INTEGRITY);
}
public function newStorageProperties() {
diff --git a/src/infrastructure/util/PhabricatorHash.php b/src/infrastructure/util/PhabricatorHash.php
--- a/src/infrastructure/util/PhabricatorHash.php
+++ b/src/infrastructure/util/PhabricatorHash.php
@@ -139,5 +139,89 @@
return $prefix.'-'.$hash;
}
+ public static function digestWithNamedKey($message, $key_name) {
+ $key_bytes = self::getNamedHMACKey($key_name);
+ return self::digestHMACSHA256($message, $key_bytes);
+ }
+
+ public static function digestHMACSHA256($message, $key) {
+ if (!strlen($key)) {
+ throw new Exception(
+ pht('HMAC-SHA256 requires a nonempty key.'));
+ }
+
+ $result = hash_hmac('sha256', $message, $key, $raw_output = false);
+
+ if ($result === false) {
+ throw new Exception(
+ pht('Unable to compute HMAC-SHA256 digest of message.'));
+ }
+
+ return $result;
+ }
+
+
+/* -( HMAC Key Management )------------------------------------------------ */
+
+
+ private static function getNamedHMACKey($hmac_name) {
+ $cache = PhabricatorCaches::getImmutableCache();
+
+ $cache_key = "hmac.key({$hmac_name})";
+
+ $hmac_key = $cache->getKey($cache_key);
+ if (!strlen($hmac_key)) {
+ $hmac_key = self::readHMACKey($hmac_name);
+
+ if ($hmac_key === null) {
+ $hmac_key = self::newHMACKey($hmac_name);
+ self::writeHMACKey($hmac_name, $hmac_key);
+ }
+
+ $cache->setKey($cache_key, $hmac_key);
+ }
+
+ // The "hex2bin()" function doesn't exist until PHP 5.4.0 so just
+ // implement it inline.
+ $result = '';
+ for ($ii = 0; $ii < strlen($hmac_key); $ii += 2) {
+ $result .= pack('H*', substr($hmac_key, $ii, 2));
+ }
+
+ return $result;
+ }
+
+ private static function newHMACKey($hmac_name) {
+ $hmac_key = Filesystem::readRandomBytes(64);
+ return bin2hex($hmac_key);
+ }
+
+ private static function writeHMACKey($hmac_name, $hmac_key) {
+ $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+
+ id(new PhabricatorAuthHMACKey())
+ ->setKeyName($hmac_name)
+ ->setKeyValue($hmac_key)
+ ->save();
+
+ unset($unguarded);
+ }
+
+ private static function readHMACKey($hmac_name) {
+ $table = new PhabricatorAuthHMACKey();
+ $conn = $table->establishConnection('r');
+
+ $row = queryfx_one(
+ $conn,
+ 'SELECT keyValue FROM %T WHERE keyName = %s',
+ $table->getTableName(),
+ $hmac_name);
+ if (!$row) {
+ return null;
+ }
+
+ return $row['keyValue'];
+ }
+
}
diff --git a/src/infrastructure/util/__tests__/PhabricatorHMACTestCase.php b/src/infrastructure/util/__tests__/PhabricatorHMACTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/util/__tests__/PhabricatorHMACTestCase.php
@@ -0,0 +1,40 @@
+<?php
+
+final class PhabricatorHMACTestCase extends PhabricatorTestCase {
+
+ protected function getPhabricatorTestCaseConfiguration() {
+ return array(
+ self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => true,
+ );
+ }
+
+ public function testHMACKeyGeneration() {
+ $input = 'quack';
+
+ $hash_1 = PhabricatorHash::digestWithNamedKey($input, 'test');
+ $hash_2 = PhabricatorHash::digestWithNamedKey($input, 'test');
+
+ $this->assertEqual($hash_1, $hash_2);
+ }
+
+ public function testSHA256Hashing() {
+ $input = 'quack';
+ $key = 'duck';
+ $expect =
+ '5274473dc34fc61bd7a6a5ff258e6505'.
+ '4b26644fb7a272d74f276ab677361b9a';
+
+ $hash = PhabricatorHash::digestHMACSHA256($input, $key);
+ $this->assertEqual($expect, $hash);
+
+ $input = 'The quick brown fox jumps over the lazy dog';
+ $key = 'key';
+ $expect =
+ 'f7bc83f430538424b13298e6aa6fb143'.
+ 'ef4d59a14946175997479dbc2d1a3cd8';
+
+ $hash = PhabricatorHash::digestHMACSHA256($input, $key);
+ $this->assertEqual($expect, $hash);
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Mon, Nov 4, 8:46 AM (1 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6717138
Default Alt Text
D17630.diff (9 KB)

Event Timeline