Editorial comments (was: WGLC)

Thanks for the review.  You caught a few things here.  Most of these
were pretty straightforward to fix.  You can find the commit tags in
brackets.

On 3 August 2014 07:03, Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> 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?

If you are expecting to do HTTP/2 - for any reason - and you don't get
a connection preface, then that's bad. [1f8d95b]

>> 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").

[f36089a]

>> 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.

Note that text *very carefully* says "does not decompress".  Which
covers your first option.  As Mike notes, a receiver can decompress
and discard if it wants to retain the connection.

The receiver can't reset HPACK state without dealing with the speed of
light.  It can send a SETTINGS frame to bounce the value to zero and
back, but that has a delayed effect, during which time the receiver is
forced to reset any stream that contains header-bearing frames.  So
yes, there is technically an option available that doesn't require a
connection tear-down.

There is a difference between something being technically possible and
being permissible. As a general rule we've decided not to hide errors
like this; I seem to recall discussing this specific case on several
occasions.

We do have an advisory limit on header list size in settings, so I
don't think we need anything harder here.

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

[1726995]

> 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.

[046648f]

>> 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.

[5085478]

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

Technically, it's a connection error of type FRAME_SIZE_ERROR.  Now,
whether it's worth actually sending is up to you.

>> 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.

The handling of unknown schemes is handled at the HTTP layer, with
HTTP status codes.  We're not changing anything in this regard.

> 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).

See above.

>> 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.

The way to interpret this is that HTTP/2 explicitly favours the
authority form. [95ebf48]

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

Nothing.  Is that somehow unclear?

Note that we need this for some perverse 1.1 cases that were discussed
at length.  The following form is apparently common enough to support:

GET http://example.com/x HTTP/1.1
Host: example.org:9322

>> 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.

We discussed this at length.  The conclusion was that reason phrases
were not useful.

>> 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).

Yep.  That's why we favour the authority form.  If you are translating
mechanically, that sucks, but we can't really fix it without total
ordering.

>> 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.

See above comment and resolution.  [046648f]

>> 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).

Would a reference to SP 800-57 help here?  (This isn't my text, so I'm
relying on the internet to help me.)

>> 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)).

Yes.  Bad servers will be bad.  That's why we have the GOAWAY code: so
we don't perpetuate such badness.

> 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.

Yes.  See above.

> 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.

Yes, but this is a consequence of a choice that an encoder makes.  The
blacklist is on keys, not values, but that can be escalated by
blacklisting problem origins, or the entire header table.

>> 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).

The most insecure schemes are what this is specifically addressing.
Good schemes don't suffer from any weakness to guessing of the sort we
are concerned with here.

Received on Monday, 4 August 2014 19:51:26 UTC