1455219 - Accept FxA OAuth tokens for authorization in tokenserver

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Token, enhancement)

Description

[Ryan Kelly [:rfkelly]]

In order to simplify the authorization process for new applications such as Lockbox, we'd like to allow them to gain access to Firefox Sync through a simple OAuth-style login flow rather than the current convoluted webchannels-and-browserid-assertions dance.

The details of the OAuth flow itself are specified in this document:

  https://docs.google.com/document/d/1IvQJFEBFz0PnL4uVlIvt8fBS_IPwSK-avK0BRIHucxQ

And the necessary support on the FxA side of things is being worked on in this PR:

  https://github.com/mozilla/fxa-content-server/pull/6017

From the perspective of the Sync service, we can assume that the application has successfully completed an OAuth-style flow and has obtained two things:

  * An FxA OAuth token with scope "https://identity.mozilla.com/apps/oldsync"
  * A JWK containing the derived key material necessary for decrypting sync data (as defined in Bug 1426304)

It now needs to use those to authorize itself via the Sync Tokenserver and get access to the Sync Storage service.  That's where this bug comes in :-)

Currently, the tokenserver expects clients to prove authorization by providing a BrowserID identity assertion signed by FxA, and an "X-Client-State" header with a hash of the current encryption keys:

    GET /1.0/sync/1.5
    Authorization: BrowserID <gigantic base64-encoded assertion>
    X-Client-State:  <hash of kB>

I propose we add an additional authorization mechanism to tokenserver, allowing it to accept an FxA OAuth bearer token along with the key-id from its JWK:

    GET /1.0/sync/1.5
    Authorization: Bearer <short hex oauth token here>
    X-KeyID: <"kid" field from JWK here>

To verify authorization on such a request, tokenserver will need to:

  * POST the bearer token to https://oauth.accounts.firefox.com/verify
  * Extract the "uid" and "scope" fields from the response
  * Check that the returned "scope" field includes "https://identity.mozilla.com/apps/oldsync", indicating access to sync
  * Convert the JWK key-id into the same form as X-Client-State header
  * Proceed as it does currently from BrowserID assertions

I have a prototype implementation of this underway here:

  https://github.com/mozilla-services/tokenserver/pull/115

Along with a small proof-of-concept client that can exercise it in a dev environment:

  https://github.com/rfk/boxlocker

:jrgm and :bobm, let's use this bug to chat about any operational concerns with such a setup.

Comment 1

[Ryan Kelly [:rfkelly]]

+Interested folks who I forgot to add to CC when filing the bug.

:ulfr, I expect you may have some questions/concerns from a security point of view, I'm working on a broader doc and email to help drive that discussion...

Comment 2

[Ryan Kelly [:rfkelly]]

>   * POST the bearer token to https://oauth.accounts.firefox.com/verify

So, this is the biggest potential issue I see from an operational standpoint.  For BrowerID, we're able to run a local verifier service and avoid making a bunch of outgoing network requests.  For FxA, there's currently no way to verify them locally, you *must* POST them to the hosted OAuth verifier.

I think this should be OK in the short-term, because the vast majority of sync clients will still be using the existing BrowserID mechanism.  We would only expect a small number of clients to use the OAuth mechanism, as Lockbox works its way through test pilot.

If and when Lockbox becomes super popular and/or we switch some other clients to use OAuth rather than BrowserID as well, then I think we have a clear path towards making the tokens locally-verifiable, by switching from opaque database-backed tokens to signed JWTs.  But I don't want to block on that work, and I don't want to do that work before we're confident we're going to need it.

So I think it should be fine to do remote verification of these tokens to start with.

From the perspective of fxa-oauth-server, I would expect the load from this to be less than the load from kintowe, at least in the early stages.

Thoughts?

Comment 3

[Ryan Kelly [:rfkelly]]

>  GET /1.0/sync/1.5
>  Authorization: Bearer <short hex oauth token here>
>  X-KeyID: <"kid" field from JWK here>

I'm also thinking about whether we can avoid having the client send its kid in a header here.  An alternative could be to remember the kid as part of the OAuth token data, so that the server can learn it when verifying the token without having the client send it explicitly.  But I haven't thought through the engineering details of that on the FxA side yet.

Comment 4

[Ryan Kelly [:rfkelly]]

> * Check that the returned "scope" field includes "https://identity.mozilla.com/apps/oldsync", indicating access to sync

It's also interesting to think about using OAuth scopes to do finer-grained access control for sync.  For example you could imagine a scope "https://identity.mozilla.com/apps/oldsync/bookmarks" that only grants access to the bookmarks collection, or a scope like "https://identity.mozilla.com/apps/oldsync/passwords#read" which lets you read passwords but not write them.

Unfortunately some of the details of the crypto in sync make this harder to implement than it sounds, so I'd like to explicitly avoid grappling with that problem for now.  There will be a simple scope "https://identity.mozilla.com/apps/oldsync" that grants full read/write access to the user's (encrypted) sync data.

Comment 5

[Bob Micheletto [:bobm]]

(In reply to Ryan Kelly [:rfkelly] from comment #2)

> So, this is the biggest potential issue I see from an operational
> standpoint.  For BrowerID, we're able to run a local verifier service and
> avoid making a bunch of outgoing network requests.  For FxA, there's
> currently no way to verify them locally, you *must* POST them to the hosted
> OAuth verifier.

Should I file a feature request to add some percentage of OAuth backed traffic to the load tests?  At the predicted scale you are talking about, as long as there aren't any first run massive sign up surprises, I wouldn't expect this to be an issue.  I would like to have this load tested  in several different traffic ratio scenarios to make sure.  Maybe: 1% OAuth, 50% Oauth, and All Oauth all the time?  

Would you use something like a connection pool to do the verification back to the verifier?  Or one new connection for every Oauth request?

Comment 6

[Ryan Kelly [:rfkelly]]

> Should I file a feature request to add some percentage of OAuth backed traffic to the load tests?

I will take a look at adding this in the linked PR, at first glance it it shouldn't be too difficult.  This would only apply to the tokenserver loadtests, from the perspective of the individual storage nodes nothing changes here.

Comment 7

[Ryan Kelly [:rfkelly]]

> OAuth backed traffic to the load tests?

I guess one wrinkle with this is, we can't mock out OAuth tokens in same manner as we can mock out BrowserID tokens.  I see two ways to approach it.

One is, we include an actual live fxa-oauth-server in the loadtest.  This means the tests have to do a real FxA sign-in, generate an OAuth token, and use that token to talk to the tokenserver API.  That's a *lot* of overhead to do inline in the loadtest script, but would be the most authentic test.

Another option is to run a live mock of the fxa-oauth-server verification API, which would accept POSTed OAuth tokens and return API results formatted the same way as the real fxa-oauth-server, but it wouldn't involve any live accounts.  This would keep overheads down and ensure the bottleneck in the loadtests is actually the tokenserver, rather than the OAuth token-generation step.

I lean towards the second option being more useful on balance, but am interested in your thoughts :rbillings :bobm :jrgm.

Comment 8

[Julien Vehent]

As I understand the new flow, it does not change the security assumptions around tokenserver/sync. The only difference is that token verification relies on the FxA oauth verifier rather than one that's private to tokenserver (which is fundamentally the same thing since we run both).

Overall I'm in favor of consolidating to oauth and gradually removing browserid (even though we'll still support it for a while because clients don't update quickly). f+ on the plan, will need a secreview.

Comment 9

[Ryan Kelly [:rfkelly]]

> it does not change the security assumptions around tokenserver/sync

That's my take as well, it doesn't change who we trust, just the mechanism by which we verify it.

Comment 10

[Ryan Kelly [:rfkelly]]

> f+ on the plan, will need a secreview.

The code for this is ready to go, and I'll prep a tag for some loadtesting shortly.  :ulfr, what can I provide/do to help secreview of this thing?  Do you want to have a synchronous meeting to discuss?

Comment 11

[Ryan Kelly [:rfkelly]]

The code for this is ready to loadtest in stage: Bug 1459113

Let's not send it to production without some review from :ulfr's team though!

Comment 12

[Ryan Kelly [:rfkelly]]

> Let's not send it to production without some review from :ulfr's team though!

In private email, :ulfr reports that his team is pretty backed up and unlikely to get a change to look at it this quarter.  We agreed that it seems sufficiently low-risk (and easy to revert) that we can proceed without blocking on that review.

Comment 13

[Ryan Kelly [:rfkelly]]

If possible, I'd like to get this enabled on tokenserver in production before the all-hands, to unblock some joint hacking we want to do with the lockbox team.  :jrgm, does that sound doable in amongst other commitments?

Comment 14

[John Morrison [:jrgm]]

> :jrgm, does that sound doable in amongst other commitments?

Yes, I can work on that tomorrow (Tuesday, 5th, PDT)

Comment 15

[Ryan Kelly [:rfkelly]]

>We agreed that it seems sufficiently low-risk (and easy to revert) that we can proceed without blocking on that review.

As usual, typing that sort of thing into a bug comment is a great way to flush out last-minute edge-cases.  Sorry John, can we please pref the OAuth support off in production until we get a handle on Bug 1467364 (and also possibly a related edge-case that's specific to the combination of OAuth and Sync)?

Comment 16

[Ryan Kelly [:rfkelly]]

This has been rolled back in prod.  For reference, here's the edge-case I was talking about that we should fix before re-enabling:

  https://github.com/mozilla/fxa-oauth-server/pull/561

I went ahead and made it a public bug for simplicity, given the above rollback.

Comment 17

[Ryan Kelly [:rfkelly]]

Looks like the above fixes have been rolled out to production, :jrgm can we please re-enable the OAuth support in tokenserver?

Comment 18

[John Morrison [:jrgm]]

Okay, v1.3.1 tokenserver stack is deployed in production with 
[oauth]
backend = tokenserver.verifiers.RemoteOAuthVerifier
server_url = https://oauth.accounts.firefox.com