Page MenuHomePhabricator
Differential D14026

Use phutil_hashes_are_identical() when comparing hashes in Phabricator
ClosedPublic
Actions

Authored by epriestley on Sep 1 2015, 2:39 AM.
Tags
None
Referenced Files
F18871349: D14026.diff
Tue, Nov 4, 10:22 PM
F18848407: D14026.id33913.diff
Thu, Oct 30, 4:42 AM
F18824855: D14026.id.diff
Thu, Oct 23, 6:18 PM
F18820769: D14026.diff
Wed, Oct 22, 5:02 PM
F18816246: D14026.id33933.diff
Tue, Oct 21, 5:34 AM
F18815573: D14026.diff
Tue, Oct 21, 2:44 AM
F18812820: D14026.id33913.diff
Mon, Oct 20, 12:35 PM
F18788323: D14026.id33933.diff
Wed, Oct 15, 4:20 AM
View All Files
Subscribers
• chenxiruanhai

Details

Reviewers
chad
Commits
Restricted Diffusion Commit
rP29948eaa5bd2: Use phutil_hashes_are_identical() when comparing hashes in Phabricator
Summary

See D14025. In all cases where we compare hashes, use strict, constant-time comparisons.

Test Plan

Logged in, logged out, added TOTP, ran Conduit, terminated sessions, submitted forms, changed password. Tweaked CSRF token, got rejected.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 33913.Sep 1 2015, 2:39 AM
epriestley retitled this revision from to Use phutil_hashes_are_identical() when comparing hashes in Phabricator.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
epriestley added inline comments.Sep 1 2015, 2:41 AM
src/applications/people/storage/PhabricatorUser.php
411–412

The extra changes here are just following through with this and removing support for the plain CSRF tokens. The BREACH tokens have been in the wild for a little over a year, now.

chad accepted this revision.Sep 1 2015, 2:46 AM
chad edited edge metadata.
This revision is now accepted and ready to land.Sep 1 2015, 2:46 AM
• chenxiruanhai added a subscriber: • chenxiruanhai.Sep 1 2015, 8:58 AM

this is a test.

Closed by commit rP29948eaa5bd2: Use phutil_hashes_are_identical() when comparing hashes in Phabricator (authored by epriestley, committed by epriestley). · Explain WhySep 1 2015, 10:52 PM
This revision was automatically updated to reflect the committed changes.
• cyn0228.ch added a task: T5375: Tokenizers fail in Firefox on Android.Sep 7 2015, 10:42 PM
chad removed a task: T5375: Tokenizers fail in Firefox on Android.Sep 7 2015, 10:45 PM

Revision Contents
Changeset List

PathSize
src/
applications/
auth/
controller/
2 lines
5 lines
engine/
5 lines
factor/
2 lines
provider/
2 lines
conduit/
controller/
3 lines
method/
2 lines
metamta/
controller/
4 lines
people/
storage/
36 lines
settings/
panel/
5 lines
infrastructure/
util/
password/
2 lines

Diff 33933

View Options

src/applications/auth/controller/PhabricatorAuthController.php

Loading...
View Options

src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php

Loading...
View Options

src/applications/auth/engine/PhabricatorAuthSessionEngine.php

Loading...
View Options

src/applications/auth/factor/PhabricatorTOTPAuthFactor.php

Loading...
View Options

src/applications/auth/provider/PhabricatorAuthProvider.php

Loading...
View Options

src/applications/conduit/controller/PhabricatorConduitAPIController.php

Loading...
View Options

src/applications/conduit/method/ConduitConnectConduitAPIMethod.php

Loading...
View Options

src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php

Loading...
View Options

src/applications/people/storage/PhabricatorUser.php

Loading...
View Options

src/applications/settings/panel/PhabricatorSessionsSettingsPanel.php

Loading...
View Options

src/infrastructure/util/password/PhabricatorPasswordHasher.php

Loading...
Phabricator · Built by Phacility