Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly track the key-rotation timestamp for the user's sync key #124

Closed
rfk opened this issue Oct 23, 2018 · 5 comments
Closed

Explicitly track the key-rotation timestamp for the user's sync key #124

rfk opened this issue Oct 23, 2018 · 5 comments
Assignees
Labels
5 Estimate - l - Moderately complex, will require some effort but clearly defined.

Comments

@rfk
Copy link
Contributor

rfk commented Oct 23, 2018

Over in https://github.com/mozilla/fxa-auth-server/issues/2674 we're going to add a new field to the metadata returned in FxA identity assertions telling us the timestamp at which the user's key material last changed. This is similar to, but not the same as, the "generation number" that we currently track.

Let's update tokenserver to track both "fxa-generation" and "fxa-keysChangedAt" for each user, so that it can accurately construct the kid for the user's sync key. This will in turn allow the new durable sync storage server to more accurately purge data that was encrypted with an older key.

Concretely, we currently communicate fxa_uid and fxa_kid fields to the storage nodes like this:

{
    'fxa_uid': user['fxa_uid'],        // e.g. "00001111222233334444555566667788"
    'fxa_kid': user['client-state'],   // e.g. "AABBCCDDEEFF11223344556677889900"
}

Given two fxa_kid values, it's not possible to tell by inspection which is the "current" key-id and which is out-of-date. When we hand out scoped keys for OAuth clients, we identify the keys by a combination of timestamp and hash, which means they sort lexicographically in a useful fashion. I think we should do the same here, so we end up sending:

{
    'fxa_uid': user['fxa_uid'],                                        // e.g. "00001111222233334444555566667788"
    'fxa_kid': user['keys-changed-at'] + "-" + user['client-state'],   // e.g. "1569299946203-AABBCCDDEEFF11223344556677889900"
}

This will allow the storage nodes to easily detect and purge data stored under old keys.

The tricky part will be doing this accurately in the presence of incomplete information, because we haven't been tracking the key-rotation timestamp for existing users. I believe we need to do the following, in order:

  1. For OAuth sync clients, stop treating the key rotation timestamp as being the same as the generation number. Otherwise we risk locking OAuth clients out of sync once we start tracking the key-rotation timestamp properly. I'm working on a PR for this.
  2. Get the FxA changes to track key-rotation timestamp into production on the FxA side, which will make a new keysChangedAt field available to the tokenserver.
  3. Update tokenserver to start recording keysChangedAt alongside the generation number in the db, and reporting it out in the fxa_kid field as above.
@rfk
Copy link
Contributor Author

rfk commented Nov 28, 2018

@bbangert not sure if this is linked into your "python spanner prototype" tree anywhere, but I wanted to flag that it should be. It won't block you from pushing ahead with implementing the storage backend, but I think it'll be needed before we can put any production users in a spanner node. (Otherwise you're going to have a hard time purging a user's old data after a password reset, because you won't have the timestamps necessary to tell which data is "old").

@rfk
Copy link
Contributor Author

rfk commented Sep 23, 2019

I worked through the required FxA changes over in mozilla/fxa#2570; before I head out on PTO I will take a fresh look at what corresponding changes will be needed here in tokenserver. Broadly I think it's going to look like:

  • Adding a keysChangedAt column in the node-assignment table, alongside generation.
  • Extracting the value for keysChangedAt from either the assertion or the OAuth Key-ID timestamp, like we currently do for generation
  • Fiddling with the way we handle generation for OAuth clients, because it will no longer be possible to find out a generation number for those clients.
  • Ensuring that we use keyChangedAt rather than generation when reconstructing the fxa_kid field to embed in the Hawk token.

I will also work on some docs that explain more clearly why each of those things are necessary; I realized that the README etc in this repo is substantially out of date :-(

@rfk
Copy link
Contributor Author

rfk commented Sep 24, 2019

I've updated the issue description here to say more about the motivation and next steps involved.

@tublitzed tublitzed added the 5 Estimate - l - Moderately complex, will require some effort but clearly defined. label Sep 27, 2019
@rfk rfk mentioned this issue Oct 18, 2019
@rfk
Copy link
Contributor Author

rfk commented Nov 6, 2019

A brief status update here, referencing the steps in the issue description:

@rfk
Copy link
Contributor Author

rfk commented Nov 13, 2019

This is ready to go!

@rfk rfk closed this as completed Nov 13, 2019
@rfk rfk mentioned this issue Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 Estimate - l - Moderately complex, will require some effort but clearly defined.
Projects
None yet
Development

No branches or pull requests

3 participants