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

From: "Richard Barnes" <[email-masked]>
Date: Wed, 21 Jan 2015 22:14:22 -0800

Richard Barnes has entered the following ballot position for
draft-ietf-httpbis-http2-16: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to http://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
http://datatracker.ietf.org/doc/draft-ietf-httpbis-http2/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

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

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

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

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

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

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

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"

---

From: Mark Nottingham <[email-masked]>
Date: Thu, 22 Jan 2015 17:57:39 +1100

Hi Richard,

My .02 below.

> On 22 Jan 2015, at 5:14 pm, Richard Barnes <[email-masked]> wrote:
> 

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.

> help a lot.  Likewise for the figures in Section 6.

 Seems reasonable. 

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

> the URI (e.g. HPKP checks [HPKP])." 

Seems reasonable. I'm not sure I'd use HPKP as the reference here...

> the connection (as specified in Section 9.1).

Agreed.

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

---

From: Amos Jeffries <[email-masked]>
Date: Thu, 22 Jan 2015 20:55:43 +1300

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])."
> 
>> (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.
> 
> 
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 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?
>> 

Amos Jeffries
Squid Software Foundation

---

From: Richard Barnes <[email-masked]>
Date: Thu, 22 Jan 2015 08:54:52 -0500

On Thu, Jan 22, 2015 at 1:57 AM, Mark Nottingham <[email-masked]> wrote:

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

Different problem.  You're talking about how you upgrade a TLS connection
once you're making one.  I'm talking about what you do when you start with
an "http" URI.

The path I'm asking about is roughly as follows:
1. Client: Upgrade: h2, h2c
2. Server: Upgrade: TLS/1.2, h2
3. Client: ClientHello with ALPN, ...

(Maybe include a client preamble to help with middleboxes.) It seems like
that provides a path to get from an "http" URI to TLS, which is possible in
h2 by virtue of the :scheme pseudo-header.  You can get to "http" over TLS
via connection reuse; why would you forbid going there directly?

One of my favorite things about this spec is that it disentangles the
concepts of transport and scheme.  The two places I've called out in this
DISCUSS (here and below) are the only ones where there still seems to be
confusion, so it seems worth ironing them out so that the separation can be
clean.

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

I actually think that this spec does a pretty good job of separating
"cleartext/TLS" from "http/https".  The only points of confusion are here
and the point mentioned above.  Regardless of whether we do OppSec, it
seems worth being clear about this distinction.

---

From: Martin Thomson <[email-masked]>
Date: Mon, 26 Jan 2015 15:55:46 -0800

Thanks for the review Richard,

https://github.com/http2/http2-spec/pull/709

> rule out the use of TLS, especially given that the server can opt out by

I think that Mark explained this adequately.

Your suggestion is one of the different startup variants that was
considered and rejected.  The conclusion being that there needs to be
*exactly one* way start any given transport, regardless of scheme.
(We can discuss why the prior knowledge thing remains, if you like.)

Having the startup path vary based on scheme like that seems to invite
the sort of layering confusion you want to avoid though.

> help a lot.  Likewise for the figures in Section 6.

Fixed throughout.

> ":scheme" pseudo-header.  Proposed text:
[...]
> resolved to the same IP address."

Sold.  No harm in anticipating future needs.

> the URI (e.g. HPKP checks [HPKP])."

Yes, though I dropped the HPKP reference.  I considered adding a 2818
or 6125 reference, but decided to avoid both and also avoid implying
too narrow a scope for the checks.

> the connection (as specified in Section 9.1).

MUST-ied.  As in
- <xref target="authority"/>) are never cached or used.
+ <xref target="authority"/>) MUST NOT be used or cached.

> program that accepts connections in order to service HTTP requests by

Check the PR here.

> What is a virtual channel?  Can this phrase just be deleted?

Baleeted.

> protocol.  (And thus removing the RFC 2119 language.)

I think that I might just rely on tools.ietf.org to provide that
little bit of history.  If that's OK with you.

I mean, technically, we should be registering all these identifiers.

> flag, I guess).  Might help this read more smoothly to note something to

The exclusive thing was a relatively recent addition.  An explanation
would be retconning.

The actual and original reason is what I shall term "MSB-phobia": the
fear associated with 32- or 64-bit numbers that use the most
significant bit.  Languages without unsigned variants of the fixed
sized integers, combined with an unwillingness to deal with the
occasional rollover into negative values (or a fear that doing so
might trigger a bug), causes some people to avoid using the bit
entirely.

In any case, we've avoided providing such explanations in the past,
preferring to stick with facts.

> indicate alternatives, it would be clearer to add a space before the

Done.

> Section 8.1.2.3: "_:authority_"

Stephen noticed it too.

Did you know that <spanx> defaults to <spanx style="emph">.  Well, you
do now.  And you will never get those brain cells back either.

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 28 Jan 2015 11:36:04 +1100

Hi Richard,

On 23 Jan 2015, at 12:54 am, Richard Barnes <[email-masked]> wrote:

> Different problem.  You're talking about how you upgrade a TLS connection once you're making one.  I'm talking about what you do when you start with an "http" URI.
> 
> (Maybe include a client preamble to help with middleboxes.) It seems like that provides a path to get from an "http" URI to TLS, which is possible in h2 by virtue of the :scheme pseudo-header.  You can get to "http" over TLS via connection reuse; why would you forbid going there directly?

This implies support for RFC2817, which is pretty much unused. We haven't had any implementers express interest in that upgrade path, IIRC, and we'd need to assess it for security properties.

OTOH it's currently perfectly possible to do the following without changing the h2 spec:

1. Client: Upgrade: TLS/1.2
2. Server: Upgrade: TLS/1.2
3. Client: ClientHello with ALPN

What's the qualitative difference? 

It's true that if the server doesn't support h2, the client won't be able to multiplex http:// and https:// URLs over it, but that's going to be the case no matter what we do here. While some implementations might want to know whether their peer supports h2 before negotiating TLS, I'm not sure enabling that is a good outcome -- after all, the IAB wants to encourage use of encryption...

> One of my favorite things about this spec is that it disentangles the concepts of transport and scheme.  The two places I've called out in this DISCUSS (here and below) are the only ones where there still seems to be confusion, so it seems worth ironing them out so that the separation can be clean.

I understand, but we've prioritised interoperability and security here.

> I actually think that this spec does a pretty good job of separating "cleartext/TLS" from "http/https".  The only points of confusion are here and the point mentioned above.  Regardless of whether we do OppSec, it seems worth being clear about this distinction.

Your proposed language creates ambiguity; whereas before we divided the world into http and https resources, now there would be several states.

Cheers,

---

From: Richard Barnes <[email-masked]>
Date: Wed, 28 Jan 2015 03:24:06 -0500

> https://github.com/http2/http2-spec/pull/709

I <3 PRs for DISCUSS resolution.

> considered and rejected.

OK, thanks.  That's what I needed.

> The conclusion being that there needs to be

This still seems like burying the lede.  How about the proposed change in
8.2.1?  It seems like the point that the server must be authoritative is
important enough to put up front.  This seems like it's pretty parallel to
the checks on ":method", so borrowing text from that:

"The server MUST include a value in the ":authority" header field for which
the server is authoritative (see Section 10.1).  If a client receives
a PUSH_PROMISE for a resource for which the server is not authoritative, it
MUST respond with a stream error (Section 5.4.2) of type PROTOCOL_ERROR."

(Or maybe STREAM_REFUSED?  Since these requests should not be processed at
all by the client.)

---

From: Martin Thomson <[email-masked]>
Date: Wed, 28 Jan 2015 10:03:22 -0800

On 28 January 2015 at 00:24, Richard Barnes <[email-masked]> wrote:
> the server is authoritative (see Section 10.1).  If a client receives a
> PUSH_PROMISE for a resource for which the server is not authoritative, it

WFM.  (STREAM_REFUSED doesn't add value in this context: we don't need
to worry about retries, because retries are allowed anyway.)

I'll keep my proposed MUST NOT; no harm in having extra coverage for
something important like this.

---

From: Mark Nottingham <[email-masked]>
Date: Fri, 30 Jan 2015 16:05:53 +1100

Hi Richard,

Was there anything else that we need to talk about before you release the DISCUSS?

Cheers,

> On 29 Jan 2015, at 5:03 am, Martin Thomson <[email-masked]> wrote:
>

---

From: Richard Barnes <[email-masked]>
Date: Fri, 30 Jan 2015 09:34:54 -0500

Just waiting on Martin to update the PR with the below text on authority.
--Richard

---

From: Martin Thomson <[email-masked]>
Date: Fri, 30 Jan 2015 11:20:27 -0800

> Just waiting on Martin to update the PR with the below text on authority.

Bah, that's not discuss worthy.  Updated.

---

From: Richard Barnes <[email-masked]>
Date: Fri, 30 Jan 2015 14:30:55 -0500

Cleared.