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.
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth
