Use 160-bit TOTP keys rather than 80-bit TOTP keys
ClosedPublic
Actions

Authored by epriestley on Nov 7 2018, 5:13 PM.

Details

Reviewers

amckinley

Commits

rP8a4bf386552f: Use 160-bit TOTP keys rather than 80-bit TOTP keys

Summary

See https://hackerone.com/reports/435648. We currently use 80-bit TOTP keys. The RFC suggests 128 as a minimum and recommends 160.

The math suggests that doing the hashing for an 80-bit key is hard (slightly beyond the reach of a highly motivated state actor, today) but there's no reason not to use 160 bits instead to put this completely out of reach.

See some additional discussion on the HackerOne report about enormous key sizes, number of required observations, etc.

Test Plan

Added a new 160-bit TOTP factor to Google Authenticator without issue.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Nov 7 2018, 5:13 PM

Harbormaster completed remote builds in B21108: Diff 47260.Nov 7 2018, 5:14 PM

epriestley requested review of this revision.Nov 7 2018, 5:14 PM

amckinley accepted this revision.Nov 7 2018, 7:02 PM

amckinley added inline comments.

src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
188	Just for giggles, I checked that `auth_factorconfig.factorSecret` was compatible with this change, and yeah, it's a `LONGTEXT`. I also don't think it's worth migrating existing secrets or throwing a setup warning for installs with the old "less secure" TOTP secrets, but maybe we should call out this change in particular in the changelog?

This revision is now accepted and ready to land.Nov 7 2018, 7:02 PM

Yeah, I'll note this in the changelog. I think migrating is probably not worth the effort since even 80 bit secrets are not really within reach of any practical attacker today, but might add a UI hint when MFA next gets a round of updates.

Closed by commit rP8a4bf386552f: Use 160-bit TOTP keys rather than 80-bit TOTP keys (authored by epriestley). · Explain WhyNov 7 2018, 11:44 PM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in T13222: 2018 Week 48-51 Bonus Content.Nov 26 2018, 5:07 PM