search

bitwarden / server

Bitwarden infrastructure/backend (API, database, Docker, etc).
https://bitwarden.com
Other
15.21k stars 1.27k forks source link

TOTP secrets should be normalised #315

Open clarfonthey opened 6 years ago

clarfonthey commented 6 years ago

Right now, TOTP secrets appear to be stored entirely as unparsed text, which allows for a lot of weirdness like:

  1. Any non-base32 characters are stripped; multiple spaces and garbage characters like / can be left in the input and ignored.
  2. TOTP secrets are usually formatted into 20-bit (four-character) blocks, and no checking is done to see if the size of the secret is actually a multiple of 20 bits. A user could easily accidentally delete a character and mess up the whole secret without noticing.
  3. Because no normalisation is done, no formatting is done, making it harder for a user to copy the secret into another app. Offering formatting into upper or lower case, separating into blocks of four, etc. could help with this. Arguably, a QR code should be provided, but isn't.
  4. In general, it's best to normalise data for database storage anyway.

What would make the most sense to do:

  1. Modify the apps to automatically format the secret as it's entered. It should be separated into blocks of four and lowercased, as it's less likely to have confusables like 1 and I.
  2. Refuse to accept new secrets which have invalid characters or a number of characters which isn't a multiple of four.
  3. Store the secrets in lowercase without the spaces in the database, and when copying the secret, also remove the spaces.
  4. Longer-term, also offer the option of outputting a QR code for the secret.
douglaswinter commented 6 years ago

Not really sure whether this is related, but I have had no success copying the seed into an app - valid tokens are only generated when scanning the QR code. This is no good since I generate my codes either in KeePassXC or in a hardware password manager (OnlyKey).

bitwarden-bot commented 2 years ago

Hi @clarfonthey, We're cleaning up our repositories in preparation for a major reorganization. Issues from last year will be marked as stale and closed after two weeks. If you still need help, comment to let us know and we'll look into it. Thanks!

clarfonthey commented 2 years ago

Bitwarden still doesn't do anything to TOTP secrets so this issue still applies.