@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Add "temporary tokens" to auth, for SMS codes, TOTP codes, reset codes, etc
Summary:
Ref T4398. We have several auth-related systems which require (or are improved by) the ability to hand out one-time codes which expire after a short period of time.
In particular, these are:
- SMS multi-factor: we need to be able to hand out one-time codes for this in order to prove the user has the phone.
- Password reset emails: we use a time-based rotating token right now, but we could improve this with a one-time token, so once you reset your password the link is dead.
- TOTP auth: we don't need to verify/invalidate keys, but can improve security by doing so.
This adds a generic one-time code storage table, and strengthens the TOTP enrollment process by using it. Specifically, you can no longer edit the enrollment form (the one with a QR code) to force your own key as the TOTP key: only keys Phabricator generated are accepted. This has no practical security impact, but generally helps raise the barrier potential attackers face.
Followup changes will use this for reset emails, then implement SMS multi-factor.
Test Plan:
- Enrolled in TOTP multi-factor auth.
- Submitted an error in the form, saw the same key presented.
- Edited the form with web tools to provide a different key, saw it reject and the server generate an alternate.
- Change the expiration to 5 seconds instead of 1 hour, submitted the form over and over again, saw it cycle the key after 5 seconds.
- Looked at the database and saw the tokens I expected.
- Ran the GC and saw all the 5-second expiry tokens get cleaned up.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4398
Differential Revision: https://secure.phabricator.com/D9217
+222
-5
+11
resources/sql/autopatches/20140520.authtemptoken.sql
+11
resources/sql/autopatches/20140520.authtemptoken.sql
···
1
+
CREATE TABLE {$NAMESPACE}_auth.auth_temporarytoken (
2
+
id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
3
+
objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
4
+
tokenType VARCHAR(64) NOT NULL COLLATE utf8_bin,
5
+
tokenExpires INT UNSIGNED NOT NULL,
6
+
tokenCode VARCHAR(64) NOT NULL COLLATE utf8_bin,
7
+
8
+
UNIQUE KEY `key_token` (objectPHID, tokenType, tokenCode),
9
+
KEY `key_expires` (tokenExpires)
10
+
11
+
) ENGINE=InnoDB, COLLATE utf8_general_ci;
+10
src/__phutil_library_map__.php
+10
src/__phutil_library_map__.php
···
1279
1279
'PhabricatorAuthSessionGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthSessionGarbageCollector.php',
1280
1280
'PhabricatorAuthSessionQuery' => 'applications/auth/query/PhabricatorAuthSessionQuery.php',
1281
1281
'PhabricatorAuthStartController' => 'applications/auth/controller/PhabricatorAuthStartController.php',
1282
+
'PhabricatorAuthTemporaryToken' => 'applications/auth/storage/PhabricatorAuthTemporaryToken.php',
1283
+
'PhabricatorAuthTemporaryTokenGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php',
1284
+
'PhabricatorAuthTemporaryTokenQuery' => 'applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php',
1282
1285
'PhabricatorAuthTerminateSessionController' => 'applications/auth/controller/PhabricatorAuthTerminateSessionController.php',
1283
1286
'PhabricatorAuthTryFactorAction' => 'applications/auth/action/PhabricatorAuthTryFactorAction.php',
1284
1287
'PhabricatorAuthUnlinkController' => 'applications/auth/controller/PhabricatorAuthUnlinkController.php',
···
4045
4048
'PhabricatorAuthSessionGarbageCollector' => 'PhabricatorGarbageCollector',
4046
4049
'PhabricatorAuthSessionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
4047
4050
'PhabricatorAuthStartController' => 'PhabricatorAuthController',
4051
+
'PhabricatorAuthTemporaryToken' =>
4052
+
array(
4053
+
0 => 'PhabricatorAuthDAO',
4054
+
1 => 'PhabricatorPolicyInterface',
4055
+
),
4056
+
'PhabricatorAuthTemporaryTokenGarbageCollector' => 'PhabricatorGarbageCollector',
4057
+
'PhabricatorAuthTemporaryTokenQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
4048
4058
'PhabricatorAuthTerminateSessionController' => 'PhabricatorAuthController',
4049
4059
'PhabricatorAuthTryFactorAction' => 'PhabricatorSystemAction',
4050
4060
'PhabricatorAuthUnlinkController' => 'PhabricatorAuthController',
+36
-5
src/applications/auth/factor/PhabricatorAuthFactorTOTP.php
+36
-5
src/applications/auth/factor/PhabricatorAuthFactorTOTP.php
···
2
2
3
3
final class PhabricatorAuthFactorTOTP extends PhabricatorAuthFactor {
4
4
5
+
const TEMPORARY_TOKEN_TYPE = 'mfa:totp:key';
6
+
5
7
public function getFactorKey() {
6
8
return 'totp';
7
9
}
···
23
25
PhabricatorUser $user) {
24
26
25
27
$key = $request->getStr('totpkey');
28
+
if (strlen($key)) {
29
+
// If the user is providing a key, make sure it's a key we generated.
30
+
// This raises the barrier to theoretical attacks where an attacker might
31
+
// provide a known key (such attacks are already prevented by CSRF, but
32
+
// this is a second barrier to overcome).
33
+
34
+
// (We store and verify the hash of the key, not the key itself, to limit
35
+
// how useful the data in the table is to an attacker.)
36
+
37
+
$temporary_token = id(new PhabricatorAuthTemporaryTokenQuery())
38
+
->setViewer($user)
39
+
->withObjectPHIDs(array($user->getPHID()))
40
+
->withTokenTypes(array(self::TEMPORARY_TOKEN_TYPE))
41
+
->withExpired(false)
42
+
->withTokenCodes(array(PhabricatorHash::digest($key)))
43
+
->executeOne();
44
+
if (!$temporary_token) {
45
+
// If we don't have a matching token, regenerate the key below.
46
+
$key = null;
47
+
}
48
+
}
49
+
26
50
if (!strlen($key)) {
27
-
// TODO: When the user submits a key, we should require that it be
28
-
// one we generated for them, so there's no way an attacker can ever
29
-
// force a key they control onto an account. However, it's clumsy to
30
-
// do this right now. Once we have one-time tokens for SMS and email,
31
-
// we should be able to put it on that infrastructure.
32
51
$key = self::generateNewTOTPKey();
52
+
53
+
// Mark this key as one we generated, so the user is allowed to submit
54
+
// a response for it.
55
+
56
+
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
57
+
id(new PhabricatorAuthTemporaryToken())
58
+
->setObjectPHID($user->getPHID())
59
+
->setTokenType(self::TEMPORARY_TOKEN_TYPE)
60
+
->setTokenExpires(time() + phutil_units('1 hour in seconds'))
61
+
->setTokenCode(PhabricatorHash::digest($key))
62
+
->save();
63
+
unset($unguarded);
33
64
}
34
65
35
66
$code = $request->getStr('totpcode');
+18
src/applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php
+18
src/applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php
···
1
+
<?php
2
+
3
+
final class PhabricatorAuthTemporaryTokenGarbageCollector
4
+
extends PhabricatorGarbageCollector {
5
+
6
+
public function collectGarbage() {
7
+
$session_table = new PhabricatorAuthTemporaryToken();
8
+
$conn_w = $session_table->establishConnection('w');
9
+
10
+
queryfx(
11
+
$conn_w,
12
+
'DELETE FROM %T WHERE tokenExpires <= UNIX_TIMESTAMP() LIMIT 100',
13
+
$session_table->getTableName());
14
+
15
+
return ($conn_w->getAffectedRows() == 100);
16
+
}
17
+
18
+
}
+106
src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
+106
src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
···
1
+
<?php
2
+
3
+
final class PhabricatorAuthTemporaryTokenQuery
4
+
extends PhabricatorCursorPagedPolicyAwareQuery {
5
+
6
+
private $ids;
7
+
private $objectPHIDs;
8
+
private $tokenTypes;
9
+
private $expired;
10
+
private $tokenCodes;
11
+
12
+
public function withIDs(array $ids) {
13
+
$this->ids = $ids;
14
+
return $this;
15
+
}
16
+
17
+
public function withObjectPHIDs(array $object_phids) {
18
+
$this->objectPHIDs = $object_phids;
19
+
return $this;
20
+
}
21
+
22
+
public function withTokenTypes(array $types) {
23
+
$this->tokenTypes = $types;
24
+
return $this;
25
+
}
26
+
27
+
public function withExpired($expired) {
28
+
$this->expired = $expired;
29
+
return $this;
30
+
}
31
+
32
+
public function withTokenCodes(array $codes) {
33
+
$this->tokenCodes = $codes;
34
+
return $this;
35
+
}
36
+
37
+
protected function loadPage() {
38
+
$table = new PhabricatorAuthTemporaryToken();
39
+
$conn_r = $table->establishConnection('r');
40
+
41
+
$data = queryfx_all(
42
+
$conn_r,
43
+
'SELECT * FROM %T %Q %Q %Q',
44
+
$table->getTableName(),
45
+
$this->buildWhereClause($conn_r),
46
+
$this->buildOrderClause($conn_r),
47
+
$this->buildLimitClause($conn_r));
48
+
49
+
return $table->loadAllFromArray($data);
50
+
}
51
+
52
+
protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
53
+
$where = array();
54
+
55
+
if ($this->ids !== null) {
56
+
$where[] = qsprintf(
57
+
$conn_r,
58
+
'id IN (%Ld)',
59
+
$this->ids);
60
+
}
61
+
62
+
if ($this->objectPHIDs !== null) {
63
+
$where[] = qsprintf(
64
+
$conn_r,
65
+
'objectPHID IN (%Ls)',
66
+
$this->objectPHIDs);
67
+
}
68
+
69
+
if ($this->tokenTypes !== null) {
70
+
$where[] = qsprintf(
71
+
$conn_r,
72
+
'tokenType IN (%Ls)',
73
+
$this->tokenTypes);
74
+
}
75
+
76
+
if ($this->expired !== null) {
77
+
if ($this->expired) {
78
+
$where[] = qsprintf(
79
+
$conn_r,
80
+
'tokenExpires <= %d',
81
+
time());
82
+
} else {
83
+
$where[] = qsprintf(
84
+
$conn_r,
85
+
'tokenExpires > %d',
86
+
time());
87
+
}
88
+
}
89
+
90
+
if ($this->tokenCodes !== null) {
91
+
$where[] = qsprintf(
92
+
$conn_r,
93
+
'tokenCode IN (%Ls)',
94
+
$this->tokenCodes);
95
+
}
96
+
97
+
$where[] = $this->buildPagingClause($conn_r);
98
+
99
+
return $this->formatWhereClause($where);
100
+
}
101
+
102
+
public function getQueryApplicationClass() {
103
+
return 'PhabricatorApplicationAuth';
104
+
}
105
+
106
+
}
+41
src/applications/auth/storage/PhabricatorAuthTemporaryToken.php
+41
src/applications/auth/storage/PhabricatorAuthTemporaryToken.php
···
1
+
<?php
2
+
3
+
final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO
4
+
implements PhabricatorPolicyInterface {
5
+
6
+
protected $objectPHID;
7
+
protected $tokenType;
8
+
protected $tokenExpires;
9
+
protected $tokenCode;
10
+
11
+
public function getConfiguration() {
12
+
return array(
13
+
self::CONFIG_TIMESTAMPS => false,
14
+
) + parent::getConfiguration();
15
+
}
16
+
17
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
18
+
19
+
20
+
public function getCapabilities() {
21
+
return array(
22
+
PhabricatorPolicyCapability::CAN_VIEW,
23
+
);
24
+
}
25
+
26
+
public function getPolicy($capability) {
27
+
// We're just implement this interface to get access to the standard
28
+
// query infrastructure.
29
+
return PhabricatorPolicies::getMostOpenPolicy();
30
+
}
31
+
32
+
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
33
+
return false;
34
+
}
35
+
36
+
public function describeAutomaticCapability($capability) {
37
+
return null;
38
+
}
39
+
40
+
41
+
}