Subject: [Gen-art] Gen-ART review of draft-ietf-httpbis-tunnel-protocol-04

From: Christer Holmberg <[email-masked]>
Date: Fri, 22 May 2015 08:26:36 +0000

I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
Document:                                     draft-ietf-httpbis-tunnel-protocol-04.txt
Reviewer:                                        Christer Holmberg
Review Date:                                  22 May 2015
IETF LC End Date:                          3 June 2015
IETF Telechat Date:                       6 June 2015
Summary:                                     The document is well written, and almost ready for publication. However, I have a few editorial comments, and one technical question/issue.
Major Issues:
Q1:
As the ALPN header field can contain multiple, comma separated, header field values, I don't think the ABNF is correct. It should be something like:
ALPN = "ALPN":" protocol-id *(COMMA protocol-id)
Minor Issues: None
Editorial Issues:

Section 1:
-------------

Q1_1:

The text says:

"Proxies do not implement the tunneled protocol"

Are proxies prevented from implementing any tunneled protocol? If not, should the text say "Proxies might not implement the tunneled protocol"?

Q1_2:

The 2nd paragraph says:

   "The HTTP ALPN header field identifies the protocol that will be used
   within the tunnel, using the Application Layer Protocol Negotiation
   identifier (ALPN, [RFC7301])."

...and the 3rd paragraph says:

   "When the CONNECT method is used to establish a tunnel, the ALPN
   header field can be used to identify the protocol that the client
   intends to use with that tunnel."

Do you need both sentences, or could they be combined into a single sentence?

Q1_3:

The text says:

"For a tunnel that is then secured using TLS [RFC5246], the header field carries the same application
protocol label as will be carried within the TLS handshake."

I think it would be useful to add a reference to RFC 7301 after TLS handshake:

              "...be carried within the TLS handshake [RFC7301]."

(The draft does reference 7301 earlier, but that is related to the definition of ALPN.)

Q1_4:

The text says:

"The ALPN header field carries an indication of client intent only.
              An ALPN identifier is used here only to identify the application
              protocol or suite of protocols that the client intends to use in the
              tunnel.  No negotiation takes place using this header field.  In TLS,
              the final choice of application protocol is made by the server from
              the set of choices presented by the client.  Other substrates could
              negotiate the application protocol differently."

What if TLS is NOT used? Who makes the choice of application protocol then? What if the recipient does not support, or does not want to use, the protocol(s) indicated by the client?

Section 2:
-------------

Q2_1:

The text says that the ALPN header field will contain the protocol that will be used within the tunnel.

I think "will" is wrong wording, as the recipient has the final saying on what will be used. Later in the document the text says "intended to be used", and I think that would fit here too.

Section 2.3:
----------------

Q2-3_1:

The text says:

"For a CONNECT tunnel that conveys a TLS session that in turn
              encapsulates another protocol,..."

The text is confusing. Shouldn't it simply say "A tunnel that is secured using TLS", or something?

Q2-3_2:

The text says:

"When used in the ALPN header field, the ALPN identifier and registry
              are used..."

What is meant by "registry" here?

---

From: Paul Kyzivat <[email-masked]>
Date: Fri, 22 May 2015 11:21:09 -0400

On 5/22/15 4:26 AM, Christer Holmberg wrote:

> Major Issues:
>
> Q1:
>
> As the ALPN header field can contain multiple, comma separated, header
> field values, I don’t think the ABNF is correct. It should be something
> like:
>

Yeah. The existing notation (1#protocol-id) is just wrong.

But COMMA isn't defined in this draft or in 7230. So maybe it should be:

   ALPN = "ALPN":" protocol-id *("," protocol-id)

	Thanks,
	Paul