Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Totp MFA Database Design #297

Closed
vbudhram opened this issue Jan 22, 2018 · 8 comments
Closed

Totp MFA Database Design #297

vbudhram opened this issue Jan 22, 2018 · 8 comments
Assignees
Milestone

Comments

@vbudhram
Copy link
Contributor

fxa-auth-db-mysql

This issue outlines specific engineering database tasks needed to support TOPT based multifactor authentication.

At a high level, TOTP data is returned in sessionTokenWithVerificationStatus and actual token verification is done in the auth-server via Speakeasy module.

New Tables

Totp Table

Column Description Options Datatype
id unique key for entry PRIMARY KEY NOT NULL BIGINT UNSIGNED
uid account's uid NOT NULL BINARY(16)
shared_secret secret used to calulate hash NOT NULL VARCHAR(80)
epoch initial time since epoch used to calulate hash NOT NULL BIGINT UNSIGNED

Totp Recovery Code Table

Column Description Options Datatype
id unique key for entry PRIMARY KEY NOT NULL BIGINT UNSIGNED
uid account's uid NOT NULL BINARY(16)
codeHash hashed recovery code NOT NULL BINARY(32)

Alter Tables

Sessions Table

Column Description Options Datatype
verificationMethod method used to verify session. ex, email-2fa, totp-2fa NULL BIGINT UNSIGNED
verificationDate date session was verified NULL BIGINT UNSIGNED

New stored procedures

.createTotpToken(uid, shared_secret, epoch)

Parameters:

  • uid - (Buffer16) the uid of the account
  • shared_secret - (string) the shared secret used to caluate hash
  • epoch - (integer) epoch used to calulate hash, defaults to 0

Returns:

  • resolves with:
    • an empty object {}
  • rejects: with one of:
    • any error from the underlying storage engine

.generateTotpRecoveryCodes(uid)

Deletes all current recovery codes for uid and generates now ones.

Parameters:

  • uid - (Buffer16) the uid of the account

Returns:

  • resolves with:
    • an array of recovery code objects
  • rejects: with one of:
    • any error from the underlying storage engine

.consumeTotpRecoveryCode(uid, codeHash)

Consumes the recovery code and deletes from table if successful.

Parameters:

  • uid - (Buffer16) the uid of the account
  • codeHash - (Buffer32)

Returns:

  • resolves with:
    • an empty object {}
  • rejects: with one of:
    • error.notFound()
    • any error from the underlying storage engine

Alter stored procedures

.sessionTokenWithVerificationStatus(id)

Parameters:

  • tokenId - (Buffer32) the id of the token to retrieve

Returns:

  • resolves with:
    • an sessionTokenWithVerificationStatus object { ... } and
      • shared_secret
      • epoch
  • rejects with:
    • error.notFound() if this token does not exist
    • any error from the underlying storage system (wrapped in error.wrap()

New endpoints

Create TOTP Code

  • Method : PUT
  • Path : /totp/<uid>
    • uid : hex128
  • Params
    • shared_secret : string
    • epoch : epoch

Generate recovery codes

  • Method : POST
  • Path : /totp/<uid>/recoveryCodes/generate
    • uid : string

Consume recovery codes

  • Method : PUT
  • Path : /totp/<uid>/recoveryCodes
    • uid : hex128
  • Params
    • code : string
@vbudhram vbudhram self-assigned this Jan 22, 2018
@rfk rfk added this to the FxA-143: MFA milestone Jan 22, 2018
@rfk
Copy link
Contributor

rfk commented Jan 22, 2018

(@vbudhram by the way, I made an "MFA" milestone to link all the breakdown issues here together)

@vbudhram
Copy link
Contributor Author

I made an "MFA" milestone

Thank you! When you get a chance mind taking a look through?

@vbudhram vbudhram changed the title Totp MFA Design Totp MFA Database Design Jan 22, 2018
@rfk rfk added the train-105 label Jan 22, 2018
@rfk rfk self-assigned this Jan 22, 2018
@rfk
Copy link
Contributor

rfk commented Jan 23, 2018

Hrm, turns out it's not as easy to review things in an issue as it is in a PR :-)

This mostly looks good to me, some random thoughts from my initial read-through:

  • I don't understand the bigint primary keys on these tables, are they necessary? It seems like we will only look up by uid or by (uid, codeHash) and could just use that for the primary key.
  • Are the recovery codes specific to TOTP, or could we use the same recovery codes for any future MFA mechanism as well?
  • The name verificationDate could be misleading since it's a full timestamp, verificationTimestamp or verifiedAt could be a better fit here.
  • It's not clear to me whether deleting all backup codes before generating new ones is the right thing, but I guess that'll depend on how these eventually show up in the UX.
  • Given that we'll eventually have several different ways to do verification, I'm not entirely sure about slurping in the TOTP state as part of sessionTokenWithVerificationStatus, versus having the auth-server do a second lookup if and when it decides to do a TOTP prompt.
  • What happens if we try to add a totp code for a user who already has one? I assume we should error out, but worth sanity-checking this
  • "Consume recovery codes" doesn't feel like a PUT to me; it could be a POST to /totp/<uid>/recoveryCodes/consume perhaps? The existing API for unblock codes may provide some design fodder.
  • I expect we'll also need to ability to remove a TOTP enrollment from an account.

Ultimately I think we'll have to iterate both this and mozilla/fxa-auth-server#2262 a bit concurrently during implementation, but 👍 to the general shape of this so far.

@vbudhram
Copy link
Contributor Author

Hrm, turns out it's not as easy to review things in an issue as it is in a PR

I'll put up a PR to reflex change so that we could iterate a little quicker.

I don't understand the bigint primary keys on these tables, are they necessary?

Agreed, we can use the (uid, codeHash) combo.

Are the recovery codes specific to TOTP

Didn't really think of that, but no they are not. I would say that recovery code is its own type of verification method. I will make table generic to reflect that.

The name verificationDate could be misleading since it's a full timestamp

I like verifiedAt since more concise.

I'm not entirely sure about slurping in the TOTP state as part of sessionTokenWithVerificationStatus

My line of thought here was that by always pulling back the totp information we would always know whether or not to display screen. One solution might be to add a user_verification_types table or column to let server know what verification to query?

What happens if we try to add a totp code for a user who already has one?

Yea, think we should only support one totp per user.

"Consume recovery codes" doesn't feel like a PUT to me; it could be a POST

👍🏽

I expect we'll also need to ability to remove a TOTP enrollment from an account.

Will add!

@vladikoff
Copy link
Contributor

Are there other modules we can take a look at? I'm concerned that the SpeakEasy module doesn't work with Authy ( speakeasyjs/speakeasy#95 )

@vladikoff
Copy link
Contributor

ref: https://github.com/yeojz/otplib

@vbudhram
Copy link
Contributor Author

vbudhram commented Jan 30, 2018

@vladikoff That is interesting, I originally planned to prototype with speakeasy, but can build using the otplib. I am also not opposed to writing our own TOTP library.

@vbudhram
Copy link
Contributor Author

vbudhram commented Feb 5, 2018

After working through this, opting to break this issue up into three separate and independent ones.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants