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
Unknown Object (File)
Fri, May 3, 2:27 AM
Unknown Object (File)
Mon, Apr 29, 4:27 PM
Unknown Object (File)
Mon, Apr 29, 1:57 PM
Unknown Object (File)
Wed, Apr 24, 10:03 PM
Unknown Object (File)
Mon, Apr 22, 4:30 AM
Unknown Object (File)
Sun, Apr 21, 4:09 PM
Unknown Object (File)
Wed, Apr 17, 3:12 PM
Unknown Object (File)
Thu, Apr 11, 7: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
Branch
hashcomp
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 7868
Build 8767: [Placeholder Plan] Wait for 30 Seconds
Build 8766: arc lint + arc unit

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
resources/
celerity/
20 lines
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 33913

View Options

resources/celerity/map.php

Loading...
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