search

heroku / heroku-two-factor

Deprecated - see our updated docs.
https://devcenter.heroku.com/articles/two-factor-authentication
0 stars 0 forks source link

Re-generating 2FA codes (i.e. on a new device) doesn't invalidate older tokens #2

Closed jacobian closed 10 years ago

jacobian commented 11 years ago

When you set up 2FA on a new device, it shouldn't generate the same tokens as the old device. heroku-two-factor gets this wrong, which means that if you set up a new device and forget to wipe the old one, you've got an old authenticator app floating around generating valid codes.

Steps to reproduce:

  1. Have 2FA already enabled and set up on Device A.
  2. Have a clean install (no codes) of an authenticator app (e.g. Google Authenticator) on Device B.
  3. Run heroku twofactor:enable, generating a barcode, and scan this barcode on Device B.
  4. Notice that the codes generated by Device B match those on Device A.

There's an argument to be made that this is by design -- there's nothing particularly wrong with having multiple devices capable of generating valid TOTP codes -- but there does need to be a way to "switch" devices, getting new codes on Device B, and rendering Device A's codes invalid.

[Scumbag security developer, brags about getting a new phone by opening an issue against our 2FA implementation...]

jacobian commented 11 years ago

BTW, we shouldn't feel too bad about this, most other TOTP implementations are getting this wrong, too. Only Google and GitHub seem to have gotten it right :tongue:.

pedro commented 10 years ago

Great note! I was slightly surprised to find this is the case, that's why I was able to ask people to just run twofactor:enable again today.

Lets definitely make that blow up the previous secret. Maybe the CLI can check if it's already enabled and show a warning?

jacobian commented 10 years ago

Well, we don't want it to just blow up the existing code and replace it with a new one -- if we did that, I could disable 2fa by stealing someone's password and running twofactor:enable. I think we probably need to force people to the website (where they'll have to log in using the old 2FA token or a backup code) and then have a way to generate a new token there.

pedro commented 10 years ago

Right! Or basically you need to pass a 2fa check before you can replace the old secret, right?

jacobian commented 10 years ago

So actually, this is a matter of some urgency: turns out this makes 2FA pretty trivial to disable if you have someone's API token. Assume I own your laptop, now check this out:

$ HKHEADER='Accept: application/json' hk api GET /account/two-factor
{"enabled":true,"url":"otpauth://totp/Heroku:jacob@heroku.com?secret=XXXXXXXXXXXXX"}

Yup, there's the 2FA secret there, each time. So I think we need to do one or more of the following:

Again this is pretty urgent, needs to fixed fairly soon; without this fix, 2FA is relatively easy to override if you steal someone's API token.

pedro commented 10 years ago

Started working on this here: https://github.com/heroku/api/pull/1969

Good job bringing up the options – ended up going for the second (more consistent, leave GET without side effects, etc).

Also noticed we were not validating for the token to allow users to disable 2fa, so that will change too. Will make sure this plugin still works fine with this.

pedro commented 10 years ago

Talked to JKM – we can't render the totp secret again or it will open the way to replay attacks.

Proposing some api tweaks and a new endpoint to address this:

pedro commented 10 years ago

Will do another review/deploy monday.

jacobian commented 10 years ago

:+1: looks great to me.

pedro commented 10 years ago

@jacobian all deployed.

People will need to update the plugin in order to run 2fa:enable again. Do you want me to email people or are you on it?