Page MenuHomePhabricator
Files F13182407

D9217.diff
No OneTemporary
Actions

D9217.diff
View Options

diff --git a/resources/sql/autopatches/20140520.authtemptoken.sql b/resources/sql/autopatches/20140520.authtemptoken.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20140520.authtemptoken.sql
@@ -0,0 +1,11 @@
+CREATE TABLE {$NAMESPACE}_auth.auth_temporarytoken (
+ id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+ objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+ tokenType VARCHAR(64) NOT NULL COLLATE utf8_bin,
+ tokenExpires INT UNSIGNED NOT NULL,
+ tokenCode VARCHAR(64) NOT NULL COLLATE utf8_bin,
+
+ UNIQUE KEY `key_token` (objectPHID, tokenType, tokenCode),
+ KEY `key_expires` (tokenExpires)
+
+) ENGINE=InnoDB, COLLATE utf8_general_ci;
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
@@ -1279,6 +1279,9 @@
'PhabricatorAuthSessionGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthSessionGarbageCollector.php',
'PhabricatorAuthSessionQuery' => 'applications/auth/query/PhabricatorAuthSessionQuery.php',
'PhabricatorAuthStartController' => 'applications/auth/controller/PhabricatorAuthStartController.php',
+ 'PhabricatorAuthTemporaryToken' => 'applications/auth/storage/PhabricatorAuthTemporaryToken.php',
+ 'PhabricatorAuthTemporaryTokenGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php',
+ 'PhabricatorAuthTemporaryTokenQuery' => 'applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php',
'PhabricatorAuthTerminateSessionController' => 'applications/auth/controller/PhabricatorAuthTerminateSessionController.php',
'PhabricatorAuthTryFactorAction' => 'applications/auth/action/PhabricatorAuthTryFactorAction.php',
'PhabricatorAuthUnlinkController' => 'applications/auth/controller/PhabricatorAuthUnlinkController.php',
@@ -4045,6 +4048,13 @@
'PhabricatorAuthSessionGarbageCollector' => 'PhabricatorGarbageCollector',
'PhabricatorAuthSessionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorAuthStartController' => 'PhabricatorAuthController',
+ 'PhabricatorAuthTemporaryToken' =>
+ array(
+ 0 => 'PhabricatorAuthDAO',
+ 1 => 'PhabricatorPolicyInterface',
+ ),
+ 'PhabricatorAuthTemporaryTokenGarbageCollector' => 'PhabricatorGarbageCollector',
+ 'PhabricatorAuthTemporaryTokenQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorAuthTerminateSessionController' => 'PhabricatorAuthController',
'PhabricatorAuthTryFactorAction' => 'PhabricatorSystemAction',
'PhabricatorAuthUnlinkController' => 'PhabricatorAuthController',
diff --git a/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php b/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php
--- a/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php
+++ b/src/applications/auth/factor/PhabricatorAuthFactorTOTP.php
@@ -2,6 +2,8 @@
final class PhabricatorAuthFactorTOTP extends PhabricatorAuthFactor {
+ const TEMPORARY_TOKEN_TYPE = 'mfa:totp:key';
+
public function getFactorKey() {
return 'totp';
}
@@ -23,13 +25,42 @@
PhabricatorUser $user) {
$key = $request->getStr('totpkey');
+ if (strlen($key)) {
+ // If the user is providing a key, make sure it's a key we generated.
+ // This raises the barrier to theoretical attacks where an attacker might
+ // provide a known key (such attacks are already prevented by CSRF, but
+ // this is a second barrier to overcome).
+
+ // (We store and verify the hash of the key, not the key itself, to limit
+ // how useful the data in the table is to an attacker.)
+
+ $temporary_token = id(new PhabricatorAuthTemporaryTokenQuery())
+ ->setViewer($user)
+ ->withObjectPHIDs(array($user->getPHID()))
+ ->withTokenTypes(array(self::TEMPORARY_TOKEN_TYPE))
+ ->withExpired(false)
+ ->withTokenCodes(array(PhabricatorHash::digest($key)))
+ ->executeOne();
+ if (!$temporary_token) {
+ // If we don't have a matching token, regenerate the key below.
+ $key = null;
+ }
+ }
+
if (!strlen($key)) {
- // TODO: When the user submits a key, we should require that it be
- // one we generated for them, so there's no way an attacker can ever
- // force a key they control onto an account. However, it's clumsy to
- // do this right now. Once we have one-time tokens for SMS and email,
- // we should be able to put it on that infrastructure.
$key = self::generateNewTOTPKey();
+
+ // Mark this key as one we generated, so the user is allowed to submit
+ // a response for it.
+
+ $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+ id(new PhabricatorAuthTemporaryToken())
+ ->setObjectPHID($user->getPHID())
+ ->setTokenType(self::TEMPORARY_TOKEN_TYPE)
+ ->setTokenExpires(time() + phutil_units('1 hour in seconds'))
+ ->setTokenCode(PhabricatorHash::digest($key))
+ ->save();
+ unset($unguarded);
}
$code = $request->getStr('totpcode');
diff --git a/src/applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php b/src/applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php
@@ -0,0 +1,18 @@
+<?php
+
+final class PhabricatorAuthTemporaryTokenGarbageCollector
+ extends PhabricatorGarbageCollector {
+
+ public function collectGarbage() {
+ $session_table = new PhabricatorAuthTemporaryToken();
+ $conn_w = $session_table->establishConnection('w');
+
+ queryfx(
+ $conn_w,
+ 'DELETE FROM %T WHERE tokenExpires <= UNIX_TIMESTAMP() LIMIT 100',
+ $session_table->getTableName());
+
+ return ($conn_w->getAffectedRows() == 100);
+ }
+
+}
diff --git a/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php b/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
@@ -0,0 +1,106 @@
+<?php
+
+final class PhabricatorAuthTemporaryTokenQuery
+ extends PhabricatorCursorPagedPolicyAwareQuery {
+
+ private $ids;
+ private $objectPHIDs;
+ private $tokenTypes;
+ private $expired;
+ private $tokenCodes;
+
+ public function withIDs(array $ids) {
+ $this->ids = $ids;
+ return $this;
+ }
+
+ public function withObjectPHIDs(array $object_phids) {
+ $this->objectPHIDs = $object_phids;
+ return $this;
+ }
+
+ public function withTokenTypes(array $types) {
+ $this->tokenTypes = $types;
+ return $this;
+ }
+
+ public function withExpired($expired) {
+ $this->expired = $expired;
+ return $this;
+ }
+
+ public function withTokenCodes(array $codes) {
+ $this->tokenCodes = $codes;
+ return $this;
+ }
+
+ protected function loadPage() {
+ $table = new PhabricatorAuthTemporaryToken();
+ $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 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,
+ 'objectPHID IN (%Ls)',
+ $this->objectPHIDs);
+ }
+
+ if ($this->tokenTypes !== null) {
+ $where[] = qsprintf(
+ $conn_r,
+ 'tokenType IN (%Ls)',
+ $this->tokenTypes);
+ }
+
+ if ($this->expired !== null) {
+ if ($this->expired) {
+ $where[] = qsprintf(
+ $conn_r,
+ 'tokenExpires <= %d',
+ time());
+ } else {
+ $where[] = qsprintf(
+ $conn_r,
+ 'tokenExpires > %d',
+ time());
+ }
+ }
+
+ if ($this->tokenCodes !== null) {
+ $where[] = qsprintf(
+ $conn_r,
+ 'tokenCode IN (%Ls)',
+ $this->tokenCodes);
+ }
+
+ $where[] = $this->buildPagingClause($conn_r);
+
+ return $this->formatWhereClause($where);
+ }
+
+ public function getQueryApplicationClass() {
+ return 'PhabricatorApplicationAuth';
+ }
+
+}
diff --git a/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php b/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php
@@ -0,0 +1,41 @@
+<?php
+
+final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO
+ implements PhabricatorPolicyInterface {
+
+ protected $objectPHID;
+ protected $tokenType;
+ protected $tokenExpires;
+ protected $tokenCode;
+
+ public function getConfiguration() {
+ return array(
+ self::CONFIG_TIMESTAMPS => false,
+ ) + parent::getConfiguration();
+ }
+
+/* -( PhabricatorPolicyInterface )----------------------------------------- */
+
+
+ public function getCapabilities() {
+ return array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ );
+ }
+
+ public function getPolicy($capability) {
+ // We're just implement this interface to get access to the standard
+ // query infrastructure.
+ return PhabricatorPolicies::getMostOpenPolicy();
+ }
+
+ public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+ return false;
+ }
+
+ public function describeAutomaticCapability($capability) {
+ return null;
+ }
+
+
+}

File Metadata

Mime Type
text/plain
Expires
Fri, May 10, 11:28 PM (3 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6284309
Default Alt Text
D9217.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility