[OAUTH-WG] Review of draft-ietf-oauth-signed-http-request-03

[Mike Jones <[e-mail]>]

There are some deployments that I'm aware of that are considering using draft-ietf-oauth-signed-http-request.  Because of that, I've done a detailed review of https://tools.ietf.org/html/draft-ietf-oauth-signed-http-request-03, which follows.  Only substantive issues are discussed.  If the draft is checked into GitHub, I'd be glad to create a pull request for a set of accompanying editorial suggestions.  If it isn't, I can send the editorial suggestions in a subsequent message.

SUBSTANTIVE ISSUES

1. Introduction - The draft says "it is desirable to bind the signature to the HTTP request".  It would be great to say why this is desirable and what attributes one might want to have bound.  (The "what attributes" discussion could go in the Security Considerations section, as requested below.)

3.  Generating a JSON Object from an HTTP Request - Why is "at" required?  The spec would be far more general if this field was OPTIONAL like the others.  Is there any reason not to do this, while still staying that it's required in the PoP-protected access token use case?

3.  "ts" parameter - State that this value is a NumericDate as defined in RFC 7519.  (That way you'll inherit the POSIX.1 language about leap seconds, etc.)

3.  "Note to WG" - I suspect that this wouldn't get past the IESG without crypto agility.  A parameter probably needs to be introduced to specify the hash algorithm used - with the default being SHA-256 if omitted.

3.1.  Calculating the query parameter list and hash - The draft says that "The client iterates through all query parameters in whatever order it chooses...".  I suspect this would be a lot less error prone if instead, it was specified that the query parameters are to be processed in the order that they will appear in the query string.

3.2.  Bullet 3:  At a minimum, this clause needs to describe what the canonical encoding is if an input value is preceded by or followed by space or tab characters.  Are those stripped before being used in the "name: value" construct?

3.2.  Bullet 3:  Is a newline terminator present for all "name: value" lines or is it only used between lines, with there being none at the end of the value to be encoded?  This should be explicitly stated.

4.2.  HTTP Form Body - The draft defines the name "pop_access_token", which seems like a misnomer, because the PoP access token is actually passed in the "at" parameter as part of this value.  I would suggest renaming this to either "pop_http_message" or "signed_http_message".

4.3.  HTTP Query Parameter - Ditto to my naming comment in 4.2.

6.2.  "typ":"pop" registration - The draft doesn't say when this type can or must be used.  For instance, is this intended to be used in the access token representation the case where the PoP access token is represented as a JWT?  Or is it intended to be used for the signed HTTP message JWT header?  Or both?  And is its use optional or required in which cases?

7.  Security Considerations, Guidance to Applications - The draft provides almost no guidance to application designers about which parameters should be signed and when.  For instance, I'd think that many applications would want to sign URL components (the "u", "p", and "q" parameters).  If so, please provide guidance on when these should be included.  Likewise, if there's a set of headers that one would normally expect to include in the signature, please name them and provide justification for their inclusion.  Finally, if there are headers that one should normally *not* include, please also list those and say why.  Left to their own devices, application designers are almost certain to choose many distinct and sub-optional sets of choices.

7.5.  Validating the integrity of the HTTP message - The third paragraph says that "If a header or query parameter is repeated on either the outgoing request from the client or the incoming request to the protected resource, that query parameter or header name MUST NOT be covered by the hash and signature."  This doesn't seem to be consistent with the creation and validation procedures described earlier in the draft, which essentially said that parameters are processed in the chosen order and appended to the arrays.  It's perfectly meaningful to have multiple instances of some parameters and so it's counterproductive to prohibit them being signed.  Please remove this prohibition (and if you like, replace it with a warning that order must be preserved for the signature to validate - which is true of all parameters - not just parameters that occur multiple times).

                                                          Thanks,
                                                          -- Mike

P.S.  Yes, I realize that some other PoP options are also in the works based on discussions at the OAuth Security Workshop.  But I wanted to get these issues on record for those who are considering using the existing signed-http-request draft.