Brian, thanks for reading through the document and setting fire to the strawman within.

Very good call on the hash inputs, I think you’re definitely right. I’m not sure how best to handle that apart from some kind of out-of-band delimiter. Maybe we should hash a dot-separated base64 encoded list? (I’m only half joking.)

The repeated/ordering problem is definitely one that would need to be sorted out (no pun intended). I think there’s some language in there that says that the ordering has to be the same, or that the server needs to be aware of it.

I wouldn’t be surprised if I fat-fingered in “client” in the wrong places a few times, too.

Also missing from the document: a means to communicate the JWT from the client to the RS. I envisioned it being in parallel to RFC6750, with the preferred method of being a header:


Authorization: Signed-Http eyj…. SIGNED JWT GOES HERE …. uyweWEafe23


With form and query parameters as other options. Note that adding this as a query parameter doesn’t screw up the signature calculation, since you have to specify which query parameters you signed.


 — Justin


> On Jul 21, 2015, at 4:19 PM, Brian Campbell <bcampbell@pingidentity.com> wrote:
> 
> I think I said, at the last meeting, that I would review draft-ietf-oauth-signed-http-request, which was maybe foolish of me, but I thought I should be timely and send something before the meeting tomorrow. Even though the document isn't on the agenda. 
> 
> Let me first say that I honestly don't know if this HTTP signing is the right approach or not for presenting some kind of 'better than bearer' access tokens to the RS. As such, I don't intend my reading/review of the document as either an endorsement of it or opposition to it. 
> 
> That said, I did notice a couple of potential security or interoperability issues that I wanted to raise.
> 
> Following the description of calculating the query parameter list and hash <https://tools.ietf.org/html/draft-ietf-oauth-signed-http-request-01#section-3.2> and validating the query parameter list and hash <https://tools.ietf.org/html/draft-ietf-oauth-signed-http-request-01#section-4.1> (text of both copied below) it seems like different query strings could result in the same hash value. For example,
> 
> The query string ?foo=bar%3D&bar= processed in order shown there would have a names list of ["foo", "bar"] and parameters of
>  foo=bar=
>  bar=
> and the hash would be of
>  foo=bar=bar=
> 
> While the different query string ?foo=&bar=bar%3D processed in the same order, left to right, would have the same names list of ["foo", "bar"] and parameters of 
>  foo=
>  bar=bar=
> and the hash would be of
>  foo=bar=bar=
> 
> It's a made up example but seems to show that different content in the query string can sometimes be manipulated to produce the same hash. I don't have an exploit in mind but the bad guys are smarter than me. And it's probably just generally the kind of thing a security related protocol shouldn't allow for. 
> 
> Or am I misunderstanding something?
> 
> Seems like encoding and delimitation need to be explicitly handled in whatever way the query parameters are dealt with. There's already a [[]] in there that hints at the possibility. 
> 
> The text also says "repeated parameter names are processed separately with no special handling" but, for that to work, doesn't it require that the client and server process repeated parameters in the same order?
> 
> I think the header list and hash likely has similar issues as it's basically the same approach.
> 
> As I look at the text again, shouldn't the 4.x sections talk about the server or resource server rather than the client?
>  
>  
> 3.2 <https://tools.ietf.org/html/draft-ietf-oauth-signed-http-request-01#section-3.2>.  Calculating the query parameter list and hash
> 
> 
> 
>    To generate the query parameter list and hash, the client creates two
>    data objects: an ordered list of strings to hold the query parameter
>    names and a string buffer to hold the data to be hashed.
> 
>    The client iterates through all query parameters in whatever order it
>    chooses and for each query parameter it does the following:
> 
> 
>    1.  Adds the name of the query parameter to the end of the list.
> 
>    2.  Encodes the name and value of the query parameter as "name=value"
>        and appends it to the string buffer.  [[Separated by an
>        ampersand?  Alternatively we could have this also pulled into an
>        ordered list and post-process the concatenation, but that might
>        be too deep into the weeds. ]]
> 
>    Repeated parameter names are processed separately with no special
>    handling.  Parameters MAY be skipped by the client if they are not
>    required (or desired) to be covered by the signature.
> 
>    The client then calculates the HMAC hash over the resulting string
>    buffer.  The list and the hash result are added as the value of the
>    "p" member.
> 
> 
> 
> 4.1 <https://tools.ietf.org/html/draft-ietf-oauth-signed-http-request-01#section-4.1>.  Validating the query parameter list and hash
> 
> 
> 
>    The client has at its disposal a map that indexes the query parameter
>    names to the values given.  The client creates a string buffer for
>    calculating the hash.  The client then iterates through the "list"
>    portion of the "p" parameter.  For each item in the list (in the
>    order of the list) it does the following:
> 
>    1.  Fetch the value of the parameter from the HTTP request parameter
>        map.  If a parameter is found in the list of signed parameters
>        but not in the map, the validation fails.
> 
>    2.  Encode the parameter as "name=value" and concatenate it to the
>        end of the string buffer. [[same separator issue as above.]]
> 
>    The client calculates the hash of the string buffer and base64url
>    encodes it.  The client compares that string to the string passed in
>    as the hash.  If the two match, the hash validates, and all named
>    parameters and their values are considered covered by the signature.
> 
>    There MAY be additional query parameters that are not listed in the
>    list and are therefore not covered by the signature.  The client MUST
>    decide whether or not to accept a request with these uncovered
>    parameters.
> 
> _______________________________________________
> OAuth mailing list
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth