Re: Review Request for third draft of "Signing HTTP Messages"

Hi Manu,

This looks really good. A couple of comments below.

On 21 Jul 2014, at 10:45 am, Manu Sporny <msporny@digitalbazaar.com> wrote:
>> 
>> * Section 2.3. - How is whitespace around field-values handled — is 
>> it included in the value or not? Note that as per 
>> <http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-26#section-3.2>,
>> it isn’t (but it would be good if you made this explicit, both in
>> spec and with examples)
> 
> Whitespace around field values is not included. The spec now refers to
> the appropriate section in draft-ietf-httpbis-p1-messaging-26:
> 
> https://github.com/web-payments/web-payments.org/commit/2e00f1a30dc99e80b18deebb30f4d4b03d4b4a75

Is this straight concatenation, or with a comma?

E.g., if you have

Cache-Control: max-age=60
Cache-Control: must-revalidate

Would the canonicalised string be:

Cache-Control: max-age=60must-revalidate

or

Cache-Control: max-age=60,must-revalidate

or...?

While your text isn't ambiguous really, people are very used to concatenate-delimit-with-comma (plus some whitespace, usually), and it might be good to point out what you mean (even if with an example).


>> * Section 2.3 - Request-line is HTTP/1 specific, and — again — the 
>> HTTP version is a hop-by-hop attribute of the message, so you’re 
>> going to get signature failures through intermediaries. Suggestion: 
>> use HTTP/2’s :method, :authority and :path pseudo-headers.
> 
> Are you asking me to:
> 
> 1) Change the name `(request-line)` to something else like
>   `(request-target)`?
> 2) Specify that the header field value should be constructed by
>   concatenating the lowercased :method, an ASCII space,
>   the :authority, an ASCII space, and :path pseudo-headers?
> 
> I'm assuming that you think that :scheme shouldn't be included because
> it'll break when proxied internally between https->http? Speaking about
> proxies, isn't using :authority going to break the signature from
> hop-to-hop? Is it worth specifying that servers sitting behind a proxy
> should use the X-Forwarded-Host or Forwarded-Host headers as a
> replacement for :authority when recreating the signature? Now I'm
> wondering if there is an attack that allows an attacker to inject both
> X-Forwarded-Host and Forwarded-Host that would result in a compromised
> signature.
> 
> I made both changes above, let me know if that's not what you wanted.
> 
> https://github.com/web-payments/web-payments.org/commit/495584b09f88acd439734fcfa9f9826bf40173c3

I meant mostly (2). 

What I'd suggest is to actually use the :-prefix that http/2 specifies, because it can't be part of a valid http header field-name. The method isn't part of the request-target, so that would leave you with these "special" header field names:

:method
:uri

For :uri, I'd refer to the Effective Request URI in RFC7230 Section 5.5; that's the concept you're looking to hook onto, I think, and it should be stable between HTTP versions.


>> * Can trailers be included in a signature?
> 
> In theory, yes. We've got another spec in the works about how to do this
> (although it's horribly out of date and we don't know if we're going to
> finish it any time soon... not much demand for it as far as we can tell):
> 
> https://web-payments.org/specs/source/http-signature-trailers/
> 
> I imagine that if/when we do, we'll require something like this header:
> 
> Signature: location=trailer
> 
> and then have the actual signature in the trailers.

Cool. Note that the Trailer header <http://tools.ietf.org/html/rfc7230#section-4.4> already allows this; e.g. you'd have:

Trailer: Signature

to indicate that Signature is coming in the trailers.


>> * Can a Signature header be included in the message trailers?
> 
> In theory, yes. We need to spec exactly how this would work. The feature
> was deemed low-priority and out of scope for the base Signing HTTP
> Messages spec.

OK. 


>> * You talk (often implicitly) about how a signature is created, but 
>> not what constitutes a valid signature. This is going to be 
>> especially important for the authentication scheme.
> 
> I tried to clarify by adding two sections. The first specifies how a
> signature is created, and the second specifies how a signature is verified:
> 
> https://github.com/web-payments/web-payments.org/commit/d535b92c05f676c040c3e4efd417bd2e5b68ef68
> 
> ... but don't know if that's what you were looking for.

That looks like a good start.

Cheers,

--
Mark Nottingham   https://www.mnot.net/

Received on Monday, 28 July 2014 01:39:30 UTC