RE: Working Group Last Call: draft-ietf-httpbis-http2-14 and draft-ietf-httpbis-header-compression-09

3.5 - I don't think it's so much that you *can* infer they're HTTP/2....  You can choose to throw the GOAWAY at them in hopes they see and understand it, or you can omit it because you think it's likely they won't.  Since the GOAWAY frame will likely be racing with a TCP RST, it effectively doesn't matter -- the frame will get lost when the RST gets there.

4.3 - I think this is an implementation decision.  You SHOULD always gracefully handle decompression so your context remains valid, even if you choose to reject the request because the output would be too large for you to process.  However, if for some reason you're not able to do so, this error code gives you a way to signal why you're closing the connection.  Of course, again, it races with the TCP RST and may well never be seen by the remote endpoint.

-----Original Message-----
From: Ilari Liusvaara [mailto:ilari.liusvaara@elisanet.fi] 
Sent: Sunday, August 3, 2014 7:04 AM
To: Mark Nottingham
Cc: HTTP Working Group
Subject: Re: Working Group Last Call: draft-ietf-httpbis-http2-14 and draft-ietf-httpbis-header-compression-09

On Fri, Aug 01, 2014 at 10:54:32AM +1000, Mark Nottingham wrote:
>
> The editors have published new drafts of HTTP/2 and HPACK:
>   http://tools.ietf.org/html/draft-ietf-httpbis-http2-14

>   http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-09

> 
> ... and our issues list is currently empty. As such, this message 
> begins Working Group Last Call on these documents.

Some misc. comments from a readthrough.

Main spec:

> 3.5.  HTTP/2 Connection Preface

> Clients and servers MUST terminate the TCP connection if either peer 
> does not begin with a valid connection preface.  A GOAWAY frame 
> (Section 6.8) can be omitted if it is clear that the peer is not using 
> HTTP/2.

Under what cicrumstances one can deduce from invalid preface that the peer is using HTTP/2? If ALPN says so?

Also, what GOAWAY code (not that it matters much)? PROTOCOL_ERROR?


> 4.3.  Header Compression and Decompression

> Header compression is stateful, using a single compression context for 
> the entire connection.

This seems bit misleading to me. There are two contexts per connection, one per direction (but perhaps the other would be "decompression context").

> A receiver MUST terminate the connection with a connection error 
> (Section 5.4.1) of type COMPRESSION_ERROR if it does not decompress a 
> header block.

I think this is problematic. Hidden limit (no requirement to handle HPACK blocks up to very large size, no way to signal actual limits) that is connection-killer if exceeded.

Since it is was decided that signaling HPACK block limits was a bad idea, either:
- Require being able to decompressing large HPACK blocks for effects
  on header table (so one can gracefully deal with large headers).
- Reset incoming HPACK state (can be done using SETTINGS) on error
  and continue with connection.

> 5.1.  Stream States

Idle state description seems to be missing text about receiving anything except HEADERS or PUSH_PROMISE being a protocol error.

State "reserved (local):"

> The endpoint can send a HEADERS frame.  This causes the stream to open 
> in a "half closed (remote)" state.

Except if HEADERS sets END_STREAM (e.g. pushing cache updates via HEAD), then the transition is directly to closed state.

State "reserved (remote):"

> Receiving a HEADERS frame causes the stream to transition to "half 
> closed (local)".

Same as above, END_STREAM being set causes transition to closed.

> 6.6.  PUSH_PROMISE

> Since PUSH_PROMISE reserves a stream, ignoring a PUSH_PROMISE frame 
> causes the stream state to become indeterminate.  A receiver MUST 
> treat the receipt of a PUSH_PROMISE on a stream that is neither "open" 
> nor "half closed (local)" as a connection error (Section 5.4.1) of 
> type PROTOCOL_ERROR.

There is also the RST_STREAM race (as noted in state description) that can cause PUSH_PROMISE to be recevied in closed state.

> 6.8.  GOAWAY

How to handle invalid GOAWAY frames (plen < 8)? GOAWAY(PROTOCOL_ERROR)?

> 8.1.2.3.  Request Header Fields

> ":scheme" is not restricted to "http" and "https" schemed URIs.  A 
> proxy or gateway can translate requests for non-HTTP schemes, enabling 
> the use of HTTP to interact with non-HTTP services.

I think that there should be RST_STREAM error code for unknown :scheme, so that endpoints receiving unknown values can gracefully reject those requests.

With known schemes, the endpoint is in much better position to respond to potentially malformed requests (or unknown :authority).

Receiving indication of "scheme unknown" is much cleaner to deal with than receiving confused HTTP reply (or worse, connection error).

Also, I think that if :scheme is "https" and the underlying connection is not secure, then the server MUST reply with RST_STREAM(INADEQUATE_SECURITY).

> To ensure that the HTTP/1.1 request line can be reproduced accurately, 
> this header field MUST be omitted when translating from an HTTP/1.1 
> request that has a request target in origin or asterisk form (see 
> [RFC7230], Section 5.3).  Clients that generate
> HTTP/2 requests directly SHOULD instead omit the "Host" header field.  
> An intermediary that converts an HTTP/2 request to
> HTTP/1.1 MUST create a "Host" header field if one is not present in a 
> request by copying the value of the ":authority" header field.

I can't parse this properly. But I note that Host: header in HTTP/2 requests is troublesome if the server has to do anything with it, due to the fact that it can be anywhere in the header block.

Also, what happens if both Host and :authority are present?

> 8.1.2.4.  Response Header Fields

> HTTP/2 does not define a way to carry the version or reason phrase 
> that is included in an HTTP/1.1 status line.

Reason phrases are sometimes useful with 4XX and 5XX responses.
Could be added by including OPTIONAL :reason response header.


> 8.1.3.  Examples

>     GET /resource HTTP/1.1           HEADERS
>     Host: example.org          ==>     + END_STREAM
>     Accept: image/jpeg                 + END_HEADERS
>                                          :method = GET
>                                          :scheme = https
>                                          :path = /resource
>                                          host = example.org
>                                          accept = image/jpeg

This hits the ordering issue I noted above (host can appear anywhere, whereas :authority is very near the start).

> 8.2.2.  Push Responses

> This stream becomes
> "half closed" to the client (Section 5.1) after the initial HEADERS 
> frame is sent.

Unless HEADERS has END_STREAM, then it is closed.

> 9.2.2.  TLS Cipher Suites

> Ephemeral
> key exchange MUST have a minimum size of 2048 bits for DHE or security 
> level of 128 bits for ECDHE.

How is "security level" defined? I can come up with multiple defintions, and those are quite different (some fail NIST P-256).

> Clients MAY advertise support of cipher suites that are prohibited by 
> the above restrictions in order to allow for connection to servers 
> that do not support HTTP/2.  This enables a fallback to protocols 
> without these constraints without the additional latency imposed by 
> using a separate connection for fallback.

If connecting to typical TLS server without special HTTP/2 support at TLS level(!), then one better put all the HTTP/2 ciphersuites at beginning of the list, because many TLS servers will pick the first one off of the list, and if it isn't compatible with HTTP/2, the connection will fail (GOAWAY(INADEQUATE_SECURITY)).

Worse, some servers have ciphersuite perferences. If server prefers some non-HTTP/2-compatible cipher, does not treat HTTP/2 specially and clent sends that ciphersuite, again, the connection is going to fail.


Hpack spec:

> 8.1.2.  Mitigation

> An encoder without good knowledge of the provenance of header fields 
> might instead introduce a penalty for bad guesses, such that attempts 
> to guess a header field value results in all values being removed from 
> consideration in all future requests, effectively preventing further 
> guesses.

If one has to consider re-installation, this can potentially take lots of memory, since "blacklisted" values must be kept even beyond flushing out of compression state.

> Implementations might also choose to protect certain header fields 
> that are known to be highly valued, such as the Authorization or 
> Cookie header fields, by disabling or further limiting compression.

For most authorization schemes (all but the most insecure ones), indexing Authorization values is useless anyway, as those will never be repeated (and the Authorization header name is in static table).


-Ilari

Received on Monday, 4 August 2014 18:26:21 UTC