976098 - Call-me URLs should be opaque to users

Status: VERIFIED FIXED

(Hello (Loop) :: Server, defect)

Description

[Adam Roach [:abr]]

The temporary session URIs described in Bug 971994 shouldn't contain any user-extractable information. For the state-token approach, this implies that the information needs to be encrypted prior to having the HMAC applied.

See http://tools.ietf.org/html/draft-rescorla-stateless-tokens-01 for a detailed treatment of encrypted state tokens.

Comment 1

[Alexis Metaireau (:alexis)]

Attachment: link to github PR

Comment 2

[Eric Rescorla (:ekr)]

Comment on attachment 8384170 [details] [review]
link to github PR

Re-assigning to this rbarnes

Comment 3

[Alexis Metaireau (:alexis)]

This commit (on node) introduces support for GCM, but haven't been released in node yet.
https://github.com/joyent/node/commit/bf5bbe4512706c1d8f59850576e5abae8d3455e1

The current plan is to stick for now with token = IV || CBC(payload) || HMAC(CBC(payload)), and we could change that later on to support GCM.

Comment 4

[Alexis Metaireau (:alexis)]

Comment on attachment 8384170 [details] [review]
link to github PR

We've updated the pull request addressing the comments.

Comment 5

[Alexis Metaireau (:alexis)]

And yet another update there with your last comments, kind you get another look?

Comment 6

[Alexis Metaireau (:alexis)]

Reassigning the reviewer since this blocks the rest of the stuff on the server.

Comment 7

[Richard Barnes [:rbarnes]]

One small change noted in PR.  Need to check token length in decoding.

Comment 8

[Alexis Metaireau (:alexis)]

https://github.com/mozilla/loop-server/commit/f4bbda1aa08a98dfbbbf940d97e6f4f8b0bd3d8e

Comment 9

[Alexis Metaireau (:alexis)]

Eric, Richard, we seem to be doing something that already exists (e.g. ciphering / mac JSON tokens) described at http://self-issued.info/docs/draft-ietf-oauth-json-web-token.html#EncryptedJWTExample. 

Can you point at the key differences with what we're currently doing, and why this is considered a better approach?

Comment 10

[Richard Barnes [:rbarnes]]

You could in principle re-use JWT here.  The main benefit would be re-using the code.  Note that we don't care in this case about interoperability.  

There are a few draw-backs:  

(1) JWT tokens would be appreciably longer than what's in there now, since a full JWT encodes the algorithms/key IDs (which are hard-coded here).  You could mitigate this by trimming the header on encode() and re-adding it on decode(), at the cost of more complication in the set-up (need to generate the static header).

(2) JWT doesn't have any pre-existing claim types for the information that's in loop tokens.  We would need to define one, and probably in a "Collision Resistant Namespace" (i.e., URL format) in order to avoid risks of replay/name collision.  More token bloat.

Net of all that, it doesn't really seem worth the effort to me.

Comment 11

[James Bonacci [:jbonacci]]

:alexis given the more recent comments, is this still considered Resolved/Fixed?

Comment 12

[Alexis Metaireau (:alexis)]

It is, yes!

Comment 13

[James Bonacci [:jbonacci]]

OK.