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 Sunday, 3 August 2014 14:04:27 UTC