Re: Richard Barnes' Discuss on draft-ietf-httpbis-http2-16: (with DISCUSS and COMMENT)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 22/01/2015 7:57 p.m., Mark Nottingham wrote:
> Hi Richard,
> 
> My .02 below.
> 
>> On 22 Jan 2015, at 5:14 pm, Richard Barnes wrote:
>> 
>> (1) Section 3.2: "A server MUST ignore a "h2" token in an
>> Upgrade header field." What is the reasoning behind this
>> exclusion?  This seems to unnecessarily rule out the use of TLS,
>> especially given that the server can opt out by choosing "h2c".
> 
> We chose ALPN as the interoperable way to upgrade a TLS connection
> to h2; doing Upgrade inside TLS makes no sense, because ALPN
> support is required, and is much more efficient (since the entire
> connection has to stop for the Upgrade dance). I.e., Upgrade is
> designed for plaintext HTTP only, and use elsewhere is a good sign
> that there's a buggy client.
> 

A quick reading of the draft text does leave one with the impression
that HTTP/1 Upgrade to HTTP/2-over-TLS ("h2") is forbidden.

I understood the intention was to allow HTTP/1 to upgrade to TLS and
TLS in turn ALPN negotiate h2. In this case the HTTP/1 client sends
"Upgrade: TLS/1.2" instead of "h2".


The text could be a bit clearer that Upgrade header in HTTP/1 is still
permitted to contain "TLS/x.y" as per RFC 2817.

Perhapse:
OLD:
   Presence of a token with "h2" implies HTTP/2 over TLS, which is
   instead negotiated as described in Section 3.3.

NEW:
   Presence of a token with "h2" implies HTTP/2 over TLS, which is
   instead upgraded using an appropriate TLS token (see RFC 2817
   section 3.1) then HTTP/2 negotiated as described in Section 3.3.



> 
>> (2) Figure 1 seems really confusing.  If the reader notices the 
>> phrase "9 octets of the frame header", he'll probably come to
>> the right conclusion, but it also seems likely that some readers
>> will infer from the layout that the header is 12 octets long,
>> with the fields aligned to word boundaries.  Just eliminating the
>> header with the bit positions would help a lot.  Likewise for the
>> figures in Section 6.
> 
> Seems reasonable.
> 
> 
>> (3) Section 9.1.1: "For "http" resources..." This seems to imply 
>> that requests for "http" resources can only be carried over bare 
>> TCP, which seems wrong given the presence of the ":scheme" 
>> pseudo-header.  Proposed text:
>> 
>> OLD: "For "http" resources, this depends on the host having 
>> resolved to the same IP address." NEW: "For TCP connections
>> without TLS, this depends on the host having resolved to the same
>> IP address."
> 
> Within the scope of this specification, I think the current text
> is correct; further specifications (including HTTP OppSec, if it
> wants to) can modify this behaviour when they're in effect.
> 
> 
>> (4) Section 9.1.1: "For "https" resources..." The salient 
>> requirement here is that the certificate provided by the server 
>> MUST pass any checks that the client would have done if it were 
>> initiating the connection afresh.  In addition to the name check 
>> here, that would include things like HPKP.  Suggested text:
>> 
>> OLD: "For "https" resources, connection reuse additionally
>> depends on having a certificate that is valid for the host in the
>> URI." NEW: "For "https" resources, connection reuse additionally
>> depends on having a certificate that is valid for the host in the
>> URI. The certificate presented by the server MUST satisfy any
>> checks that the client would perform when forming a new TLS
>> connection for the host in the URI (e.g. HPKP checks [HPKP])."
> 
> Seems reasonable. I'm not sure I'd use HPKP as the reference
> here...
> 
> 
>> (5) Section 10.4: "Pushed responses for which an origin server
>> is not authoritative (see Section 10.1) are never cached or
>> used." This seems like a rather important point, for which I
>> can't find any normative text.  It seems like in Section 8.2.1,
>> the client should be REQUIRED to verify that the ":authority"
>> field in the PUSH_PROMISE contains a value for which the client
>> would have been willing to re-use the connection (as specified in
>> Section 9.1).
> 
> Agreed.
> 
> 
>> ----------------------------------------------------------------------
>>
>>
>> 
COMMENT:
>> ----------------------------------------------------------------------
>>
>>
>>
>> 
Section 2.1:
>> The definitions of "client" and "server" here are a bit lean.
>> For example, one might read them and conclude that the client
>> and server roles are independent of who sends requests and
>> responses. It would be good to clarify these roles, updating the
>> definition in RFC 7230: "An HTTP "client" is a program that
>> establishes a connection to a server for the purpose of sending
>> one or more HTTP requests.  An HTTP "server" is a program that
>> accepts connections in order to service HTTP requests by sending
>> HTTP responses."

Nod, although:

RFC 7230 section 3.1 paragraph 2 states that neither end is restricted
to a particular message type. But highlights that existing
implementations of HTTP contain the assumption of message directionality.

HTTP/2 section 8.2 and 6.6 introduces a mechanism where a server sends
both request and response to the client.

The case of client sending responses to server is left undefined simply
due to lack of need in HTTP. It could be added by a negotiated extension
at any time for either protocol.


>> 
>> Section 2.1: "across a virtual channel" What is a virtual
>> channel? Can this phrase just be deleted?
>> 
>> Section 3.1: "CREF1: RFC Editor's Note:" It seems like it could
>> be useful to leave in a variant of this note, describing the
>> variant identifiers used by pre-RFC versions of the protocol.
>> (And thus removing the RFC 2119 language.)
>> 
>> Section 3.4: "the sever MUST send"
>> 
>> Section 4.1: "R: A reserved 1-bit field." I was mystified by the 
>> purpose of this field until I realized that it's only there
>> because stream IDs are 31 bits (to make room for the Exclusive
>> flag, I guess).  Might help this read more smoothly to note
>> something to that effect here.
>> 
>> Section 5.1: When I initially read Figure 2, I thought that
>> "H/", "ES/", etc. designated types of frames (or frames + flags).
>> If, as they appear, they indicate alternatives, it would be
>> clearer to add a space before the "/".
>> 
>> Section 8.1.2.3: "_:authority_" Remove the underscores?
>> 
>> Section 10.3: "if they are translater verbatim"
>> 
>> 

Amos Jeffries
Squid Software Foundation

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)

iQEcBAEBAgAGBQJUwKz+AAoJELJo5wb/XPRjVgMH/3wRfduKmK2br0hQnnI6RrIk
L5NfH6sAvpAdq35mEK343A0TVLoOjqxN98iVh/HHpBwOxX/Q5T9ilO3mHjem8UK2
+DbdvO+qJ9yyX6kBJYAGlcimCpVVtsxnWaLXUQdi3Exc9w3VKUf7vt+V8u5RdfPO
AYn1EOcPyu5fsraoY/syIDyKvivL1oJCb4vJ4WUArtfU1oAkcNaL8QlD0+xYM7tY
HQ7n7GaxbNjjklnyBTnHgAfeSfT1GIqTtR6XISkgQGxP9dbftNJlUBFum3jp7tch
//YSV/Zoy6zL6I5o5LHyHHSxtc9gZAZW34Z9XL1uklD2IedUldOf9qEiG1/DXQU=
=0tEc
-----END PGP SIGNATURE-----

Received on Thursday, 22 January 2015 07:56:31 UTC