Subject: [quicwg/base-drafts] Define transport parameters (#122)

From: Martin Thomson <[email-masked]>
Date: Wed, 04 Jan 2017 02:43:45 -0800

This builds on @MikeBishop's work in #99 in a big way.  I have stolen a lot of text from that.

Again, it's the tippy top of a bunch of changes.  #113 comes before this.

Closes #99.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/122

-- Commit Summary --

  * Big endian
  * Define version negotiation more thoroughly
  * Reserve space
  * not off
  * Add definition of transport parameters to -transport
  * Remove extra whitespace
  * Define the extension in -tls
  * Add a stub iana section, fix dead references

-- File Changes --

    M draft-ietf-quic-tls.md (54)
    M draft-ietf-quic-transport.md (377)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/122.patch
https://github.com/quicwg/base-drafts/pull/122.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/122

---

From: Mike Bishop <[email-masked]>
Date: Wed, 04 Jan 2017 10:35:12 -0800

MikeBishop commented on this pull request.

This takes a big step toward requiring the use of TLS (and therefore requiring a new full version of the QUIC spec to use anything else) rather than just defining a new version identifier to be QUIC + other-crypto-protocol and a document describing how the integration would work.

I think either path is reasonable, but I had understood us to be on the latter path, and this PR changes that.  That should probably be something the WG as a whole discusses in depth.

> @@ -372,6 +404,33 @@ Check the flags in the common header
 ~~~
 {: #packet-types title="Types of QUIC Packets"}
 
+
+### Handling Packets from Different Versions
+
+Version negotiation ({{version-negotiation}}) is performed using packets that
+have the VERSION bit set.  This bit is always set on packets that are sent prior
+to connection establishment.  When receiving a packet that is not associated
+with an existing connection, packets without a VERSION bit MUST be discarded.
+
+While there might be similarities between different versions of this protocol,
+implementations have to assume that a version that it does not support uses a
+different packet format.
+
+Between different versions the following things are guaranteed to remain
+constant are:

Too many "are"s.

 ## Crypto and Transport Handshake {#handshake}
 
 QUIC relies on a combined crypto and transport handshake to minimize connection
-establishment latency.  QUIC provides a dedicated stream (Stream ID 1) to be
-used for performing a combined connection and security handshake (streams are
-described in detail in {{streams}}).  The crypto handshake protocol encapsulates
-and delivers QUIC's transport handshake to the peer on the crypto stream.  The
-first QUIC packet from the client to the server MUST carry handshake information
-as data on Stream ID 1.
+establishment latency.  QUIC allocates stream 1 for the cryptographic handshake,
+which uses TLS.

This seems to be explicitly rejecting the separation of a modular crypto protocol and a definition of how TLS is used as that module.  We should discuss that change in the WG more generally.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/122#pullrequestreview-15167957

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Jan 2017 17:04:21 -0800

martinthomson commented on this pull request.

Sure, the principle I'm operating on here is that *this version* of quic uses TLS.  That doesn't preclude the use of other cryptographic handshake protocols for other versions.  Given that no other alternative exists, building in the ability to change out this piece of the protocol doesn't seem worth the additional effort.

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Jan 2017 17:15:34 -0800

@martinthomson pushed 1 commit.

0d940af  are are are

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/122/files/78d3a4ab146ac24ceb971a6e3d2db84dc9f2bc68..0d940afa31f30a62a972e1476e7e4be5f11ad718

---

From: janaiyengar <[email-masked]>
Date: Thu, 05 Jan 2017 17:40:49 -0800

janaiyengar commented on this pull request.

Thanks much for the work! I agree with Mike's comment. I think the intended goal for this document was to keep it independent of TLS, though the wg was clearly chartered to only work on TLS. 

> @@ -298,6 +298,28 @@ significant bit is referred to as bit 0.  Hexadecimal notation is used for
 describing the value of fields.

+## Versions

I think this belongs in its own section, outside "Packet Types and Formats," since it applies to the entire document, and is independent of Packet Types.

> @@ -376,6 +401,33 @@ Check the flags in the common header
 ~~~
 
+

Suggest removing the clause "While there might be similarities between different versions of this protocol"

+* the location and size of the Flags field,
+
+* the location and value of the VERSION bit in the Flags field,

Not sure what it means to say that "the value of the VERSION bit" is "guaranteed to remain constant". I think location is adequate.

+to connection establishment.  When receiving a packet that is not associated
+with an existing connection, packets without a VERSION bit MUST be discarded.
+
+While there might be similarities between different versions of this protocol,
+implementations have to assume that a version that it does not support uses a
+different packet format.
+
+Between different versions the following things are guaranteed to remain
+constant are:
+
+* the location and size of the Flags field,
+
+* the location and value of the VERSION bit in the Flags field,
+
+* the existence of a 64-bit field following the Flags field, and

Connection ID is an optional field, so its existence is not guaranteed. Suggestion: "the location and size of the Connection ID field" seems adequate.

> @@ -575,47 +627,41 @@ QUIC's connection establishment begins with version negotiation, since all
 communication between the endpoints, including packet and frame formats, relies
 on the two endpoints agreeing on a version.
 
-A QUIC connection begins with a client sending a handshake packet. The details
-of the handshake mechanisms are described in {{handshake}}, but all of the
-initial packets sent from the client to the server MUST have the VERSION flag
-set, and MUST specify the version of the protocol being used.
+A QUIC connection begins with a client sending a handshake packet. Until the
+cryptographic handshake ({{handshake}}) produces 1-RTT keys, packets sent by a
+client MUST include the version in the packet header.  This allows the server to
+identify early packets and to enable version negotiation.

By "early packets", do you mean 0-RTT data? This would be a significant change. This bit is not intended to be conflated with 0-RTT data. The version bit is only set until the client hears back from the server with the version bit off, so generally, the first RTT, but it does not directly have anything to do with 1-RTT or 0-RTT keys. (In Google QUIC without Stateless Rejects, the first RTT of a cold connect would have no server config, and the second RTT would carry a fully formed CHLO with 0-RTT data, encrypted using 1-RTT keys. Version negotiation however would have completed during the first RTT.)

-* If the client's version is not acceptable to the server, the server MUST send
-  a Version Negotiation packet to the client.  This packet will have the VERSION
-  flag set and will include the server's set of supported versions.  On
-  subsequently received packets for the same connection ID with the unacceptable
-  version, the server MUST continue responding with a Version Negotiation
-  packet.
+If the packet contains a version that is acceptable to the server, the server

For parity with para above, use "If the version selected by the client is acceptable to the server". (It's also confusing because "the packet" reads as though it's referring to the version negotiation packet.)

 When the client receives a Version Negotiation packet from the server, it should
 select an acceptable protocol version.  If such a version is found, the client
 MUST resend all packets using the new version, and the resent packets MUST use
 new packet numbers.  These packets MUST continue to have the VERSION flag set
 and MUST include the new negotiated protocol version.
 
-The client MUST send its version on all packets until it receives a packet from
-the server with the VERSION flag off.  If version negotiation is successful, the
-client should receive a packet from the server with the VERSION flag off
-indicating the end of version negotiation.  All subsequent packets the client
-sends MUST have the version flag off.
-
-Once the server receives a packet from the client with the VERSION flag off, it
-MUST ignore the VERSION flag in subsequently received packets.
+The client MUST include its selected version on all packets until it starts
+protecting packets with 1-RTT keys.  Only unprotected packets and 0-RTT
+protected packets can include a version.  A client MUST NOT change the version
+it uses unless it is in response to a version negotiation packet from the
+server.

As noted above, this is a function change, and I'm not sure that it's warranted. It's reasonable for TLS 1.3 (I think), but it's not required at all for version negotiation.

I don't believe that the goal was to have TLS be baked into this version of QUIC. There's no reason this version of QUIC cannot work with a future version of TLS and/or a different crypto mechanism. The reason that no alternative exists at the moment is because the wg is scoped to be only working on TLS, similar to the wg working only on HTTP as an application.

-#### Encoding
+: The initial stream level flow control parameter is encoded as an unsigned
+  32-bit integer.  The sender of this parameter indicates that the flow control
+  offset for all stream data sent toward it is this value.

nit: "... initial flow control offset ..."

-#### Encoding
+: The initial stream level flow control parameter is encoded as an unsigned

nit: "The stream-level flow control ..." (remove "initial". I think stream-level and connection-level should be hyphenated)

-QUIC encodes the transport parameters and options as tag-value pairs, all as
-7-bit ASCII strings.  QUIC parameter tags are listed below.
+: The connection level flow control parameter contains the initial connection

For readability, frame this and the stream flow control sentence (para above) similarly.

-QUIC encodes the transport parameters and options as tag-value pairs, all as
-7-bit ASCII strings.  QUIC parameter tags are listed below.
+: The connection level flow control parameter contains the initial connection
+  flow control window encoded as an unsigned 32-bit integer.  The sender of this
+  parameter sets the byte offset for connection level flow control to this

This isn't an "offset"... suggested mod: "The sender of this parameter indicates that the total data on all streams sent toward it must not exceed this initial value."

-#### Encoding
+: The initial stream level flow control parameter is encoded as an unsigned
+  32-bit integer.  The sender of this parameter indicates that the flow control
+  offset for all stream data sent toward it is this value.

This is per-stream. Perhaps something like: "The sender of this parameter indicates that the initial flow control offset for data sent toward it on any stream is this value."

-* TCID: Indicates support for truncated Connection IDs.  If sent by a peer,
-  indicates that connection IDs sent to the peer should be truncated to 0 bytes.
-  This is expected to commonly be used by an endpoint where the 5-tuple is
-  sufficient to identify a connection.  For instance, if the 5-tuple is unique
-  at the client, the client MAY send a TCID parameter to the server.  When a
-  TCID parameter is received, an endpoint MAY choose to not send the connection
-  ID on subsequent packets.
+: The truncated connection identifier parameter indicates that packets sent to
+  the peer can omit the connection ID.  This can be used by an endpoint where

"sent to the peer" --> "sent to the endpoint" (since line 781 says "An endpoint MAY use the following transport parameters"

-* TCID: Indicates support for truncated Connection IDs.  If sent by a peer,
-  indicates that connection IDs sent to the peer should be truncated to 0 bytes.
-  This is expected to commonly be used by an endpoint where the 5-tuple is
-  sufficient to identify a connection.  For instance, if the 5-tuple is unique
-  at the client, the client MAY send a TCID parameter to the server.  When a
-  TCID parameter is received, an endpoint MAY choose to not send the connection
-  ID on subsequent packets.
+: The truncated connection identifier parameter indicates that packets sent to
+  the peer can omit the connection ID.  This can be used by an endpoint where

"can omit" --> "MAY omit"

> +problems, particularly with respect to transport parameters that establish
+limits:
+
+* A client might exceed a newly declared connection or stream flow control limit
+  with 0-RTT data.  If this occurs, the client ceases transmission as though the
+  flow control limit was reached.  Once WINDOW_UPDATE frames indicating an
+  increase to the affected flow control offsets is received, the client can
+  recommence sending.
+
+* Similarly, a client might exceed the concurrent stream limit declared by the
+  server.  A client MUST reset any streams that exceed this connection.  A
+  server MAY reset these streams.
+
+A client cannot use or rely upon any other transport parameter that was enabled
+in a previous connection.  This includes those defined in this document.  A
+client MUST assume that the transport parameter was absent, unless the

Are you saying that it's ok for the client to cache and use flow control limits and stream limits from a previous connection, but not other params? That seems reasonable to me, but I'd make that more explicit.

> +* Similarly, a client might exceed the concurrent stream limit declared by the
+  server.  A client MUST reset any streams that exceed this connection.  A
+  server MAY reset these streams.
+
+A client cannot use or rely upon any other transport parameter that was enabled
+in a previous connection.  This includes those defined in this document.  A
+client MUST assume that the transport parameter was absent, unless the
+definition of a transport parameter provides explicit rules for use of the
+option in 0-RTT.
+
+
+### New Transport Parameters
+
+An endpoint MUST ignore transport parameters that it does not support.
+
+The definition of new transport parameters can be used to negotiate new protocol

Nit: "The definition of new transport parameters can be used..." --> "New transport parameters can be used..."

> +A client cannot use or rely upon any other transport parameter that was enabled
+in a previous connection.  This includes those defined in this document.  A
+client MUST assume that the transport parameter was absent, unless the
+definition of a transport parameter provides explicit rules for use of the
+option in 0-RTT.
+
+
+### New Transport Parameters
+
+An endpoint MUST ignore transport parameters that it does not support.
+
+The definition of new transport parameters can be used to negotiate new protocol
+behavior.  Endpoints that support features that deviate from this specification
+MUST assume that a peer will operate as described in this document unless it
+provides an explicit signal of support.  A new transport parameter could be used
+to provide this signal.

Suggestion: I don't think the last two sentences add any value. I would remove them and combine this para with the previous one: "New transport parameters can be used to negotiate new protocol behavior. An endpoint MUST ignore transport parameters that it does not support."

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/122#pullrequestreview-15423312

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Jan 2017 18:23:58 -0800

It's only optional AFTER version negotiation is possible.

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Jan 2017 18:57:24 -0800

We had a long discussion about that in relation to the TLS draft.  Most relevant is this comment from Dragana: https://github.com/quicwg/base-drafts/pull/43#discussion_r91596541  If I don't make this change, it isn't possible for the packet protection to work out which keys to use on a given packet.

Yes, this is different from Google QUIC, but that's not a downside.  Google QUIC relied very heavily on trial decryption.  This change would mean no need for trial decryption at all.

This isn't specific to TLS 1.3, it's a property that is needed by any crypto protocol that needs to distinguish between the different types of keying material that is in use during the handshake.  There are always unencrypted, replayable-protected, and fully-protected packets with a 0-RTT-capable protocol.  If you accept that it is valuable to distinguish between those three types, you end up with this design.

+limits:
+

Yeah, I need to rewrite this a little.  Actually, there is an alternative we should discuss.  I will raise a separate issue on this point.

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Jan 2017 19:08:58 -0800

I'm following the principles that I thought we had agreed: this version uses TLS (and only TLS) and a new version would be needed to change that.  We have a clear description of the interface between the two protocols that makes it a relatively easy process to forklift TLS.  What we don't need is some abstract definition of a "crypto handshake protocol" and then a specific embodiment of that.  That makes the protocol much harder to read and reason about.

My plan is to remove the entire "requirements on a crypto handshake protocol" section.  It's redundant with other more concrete text.  Any replacement crypto can then just provide the same services that TLS does (or not, because those services are not relevant in that context).

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Jan 2017 19:32:08 -0800

ee4739d  Jana's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/122/files/0d940afa31f30a62a972e1476e7e4be5f11ad718..ee4739dfc5ac893493d54733acc69ba338381608

---

From: janaiyengar <[email-masked]>
Date: Thu, 05 Jan 2017 21:15:08 -0800

> @@ -633,7 +632,7 @@ on the two endpoints agreeing on a version.
 A QUIC connection begins with a client sending a handshake packet. Until the
 cryptographic handshake ({{handshake}}) produces 1-RTT keys, packets sent by a
 client MUST include the version in the packet header.  This allows the server to
-identify early packets and to enable version negotiation.
+identify 0-RTT packets and to enable version negotiation.

I would simply remove the last sentence. I don't think that the server should be using the version bit to infer anything about what packets are being sent.

>  ### Values of Transport Parameters for 0-RTT
 
-Transport parameters from the server are remembered by the client for 0-RTT
-connections.  If values change as a result of completing the handshake, the
-client is expected to respect the new values.  This introduces some potential
-problems, particularly with respect to transport parameters that establish
-limits:
+Optional transport parameters from the server MUST be reset to default values by
+the client for 0-RTT connections.
+
+The mandatory transport parameters for stream and connection flow control and
+concurrent stream limits MUST be remembered by the client.  The idle timeout value

I think MAY is adequate here -- I don't see the need for requiring a client to remember these parameters (which cannot be enforced anyways.)

> -connections.  If values change as a result of completing the handshake, the
-client is expected to respect the new values.  This introduces some potential
-problems, particularly with respect to transport parameters that establish
-limits:
+Optional transport parameters from the server MUST be reset to default values by
+the client for 0-RTT connections.
+
+The mandatory transport parameters for stream and connection flow control and
+concurrent stream limits MUST be remembered by the client.  The idle timeout value
+is not expected to be relevant for the short time that it takes to obtain 1-RTT
+keys.
+
+
+Transport parameter values could change as a result of completing the handshake.
+The client MUST respect the new values when the handshake completes.  This
+introduces some potential problems:
 
 * A client might exceed a newly declared connection or stream flow control limit
   with 0-RTT data.  If this occurs, the client ceases transmission as though the

perhaps "the client MUST cease"

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/122#pullrequestreview-15442518

---

From: Martin Thomson <[email-masked]>
Date: Sun, 08 Jan 2017 14:21:55 -0800

I guess that we still disagree about the necessity for marking 0-RTT packets then.  I will remove the sentence in the interests of not blocking the entire PR.  Expect this to come up again.

---

From: Martin Thomson <[email-masked]>
Date: Sun, 08 Jan 2017 14:23:35 -0800

The problem with "MAY" is that we don't establish defaults.  If the client decides not to remember it has no idea how much flow control window it can use or how many streams it can open.  So we either need to define defaults (maybe 0-RTT defaults) or we do this as a MUST.

---

From: Martin Thomson <[email-masked]>
Date: Sun, 08 Jan 2017 14:23:45 -0800

Sure.

---

From: Mike Bishop <[email-masked]>
Date: Sun, 08 Jan 2017 16:31:20 -0800

My thought was something like this:

- RFC QQQQ:  Uses a crypto protocol on Stream 1, gets keys from it, uses the keys to provide an encrypted transport protocol like so....
- RFC QQQT:  If your crypto protocol is TLS 1.3, here's what happens on Stream 1 and when the keys are handed to QUIC.  RFC QQQQ using TLS 1.3 is defined as QUIC version T.
- RFC QQQF in the far future:  If your crypto protocol is Foo, here's what happens on Stream 1 and when the keys are handed to QUIC.  RFC QQQQ using Foo is defined as QUIC version F.

---

From: Martin Thomson <[email-masked]>
Date: Sun, 08 Jan 2017 17:00:48 -0800

Oh, I see how you are doing that.  That's awkward: that sort of means that the normative version of quic is defined in QUIC-TLS, which includes QUIC-TRANSPORT.

I had imagined this as follows:

RFC QQQQ depends normatively on RFC QQQT, it defines v1.

Some later RFC QQQF defines a new crypto protocol mapping, describes the necessary deviations from RFC QQQQ, and would largely just be a complete replacement of RFC QQQT as well as vN.

The problem with abstraction is that it is abstract.  You have to define the shape of the abstract and THEN the actual concrete embodiment.  That's a lot more effort in the short term.  Even if there are N future versions with different crypto protocols, the work ends up being more to define the abstraction (assuming that the interface between the two is kept clean, which I think we can manage).

---

From: Mike Bishop <[email-masked]>
Date: Mon, 09 Jan 2017 13:50:18 -0800

Well, QQQQ could also say that "by default, the crypto protocol is TLS, as defined in QQQT" and register the version for that combination itself.

But yes, it is somewhat more work to do an abstraction.  But then, it's already mostly that way, and I think there's value there.  Either way, I think this may be a decision point we need to toss to the mailing list.

---

From: Martin Thomson <[email-masked]>
Date: Mon, 09 Jan 2017 14:09:46 -0800

What is that value?  Because all I'm seeing right now is a section that is badly out of date and in need of maintenance.  The easiest maintenance would be deleting it.

I will open a thread on the list.

---

From: Martin Thomson <[email-masked]>
Date: Mon, 09 Jan 2017 16:22:23 -0800

@martinthomson pushed 3 commits.

723ccd7  Reflow
9e9a7a9  Add transport parameters to the interface definition
2b6271d  Use better anchor names

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/122/files/9a008f09623c207239c7f39838b83ad18fa7bf6d..2b6271d19873ebf32c8e6d7d868dd9913c6b6626

---

From: mirjak <[email-masked]>
Date: Tue, 10 Jan 2017 09:12:09 -0800

mirjak commented on this pull request.

So my understanding actually was that it should be possible to change the crypto protocol without changing the version. However, that would mean you probably need another negotiation mechanism for the crypto protocol...? I guess that doesn't make sense. If we define this version to always use TLS (it doesn't have to be 1.3. though, right? should probably also be possible to support higher versions...), then we could probably move the text on requirement for a crypto protocol in the applicability statement (which we just started to write today; expect to see a -00 draft latest by next week).

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Jan 2017 13:33:19 -0800

Yes, @MikeBishop already observed that since we don't have a negotiation for crypto protocol, this version necessarily uses TLS.  And you are right to observe that any version of TLS will do, though you don't get all the properties you need from versions earlier than 1.3.

I wonder if we need explicit language saying that 1.2 isn't good enough, some stacks could negotiate 1.2, which could actually work.

---

From: Patrick McManus <[email-masked]>
Date: Tue, 10 Jan 2017 14:24:34 -0800

mcmanus commented on this pull request.

definitely a +1 to being explicit about >= 1.3.. being overly clever for the reader is not a good thing here.

also +1 on binding a version of the protocol to TLS with text that acknowledges security versioning is done as protocol versioning.

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Jan 2017 16:34:10 -0800

dbea96b  Don't require storing of 0-RTT parameters

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/122/files/f4577328707a7babfd68116e6507117879878a7b..dbea96b2bad8710dce44b1145465a14619e56f13

---

From: janaiyengar <[email-masked]>
Date: Wed, 11 Jan 2017 22:08:47 -0800

+1 on >= 1.3, but I don't think this document should be tied to TLS.

---

From: janaiyengar <[email-masked]>
Date: Wed, 11 Jan 2017 22:49:59 -0800

janaiyengar requested changes on this pull request.

There's plenty of useful stuff here that can go into an uncontroversial PR on Transport Params, which I've tried to mark out. I've suggested pieces to separate into PRs for discussion. 

> @@ -1106,31 +1168,35 @@ protection for the QUIC negotiation.  This does not prevent version downgrade
 during the handshake, though it means that such a downgrade causes a handshake
 failure.
 
-Protocols that use the QUIC transport MUST use Application Layer Protocol
-Negotiation (ALPN) {{!RFC7301}}.  The ALPN identifier for the protocol MUST be
-specific to the QUIC version that it operates over.  When constructing a
-ClientHello, clients MUST include a list of all the ALPN identifiers that they
-support, regardless of whether the QUIC version that they have currently
-selected supports that protocol.
+TLS uses Application Layer Protocol Negotiation (ALPN) {{!RFC7301}} to select an
+application protocol.  The application-layer protocol MAY restrict the QUIC
+versions that it can operate over.  Servers MUST select an application protocol

I would leave out what the application protocol MAY do from this doc

-Servers SHOULD select an application protocol based solely on the information in
-the ClientHello, not using the QUIC version that the client has selected.  If
-the protocol that is selected is not supported with the QUIC version that is in
-use, the server MAY send a QUIC version negotiation packet to select a
-compatible version.
+If the server cannot select a compatible combination of application protocol and
+QUIC version, it MUST abort the connection. A client MUST abort a connection if
+the server picks an incompatible combination of QUIC version and ALPN
+identifier.

I would leave this para out, since it's not for TLS to do. I think this belongs in the HTTP mapping doc.

> @@ -629,10 +628,9 @@ QUIC's connection establishment begins with version negotiation, since all
 
-A QUIC connection begins with a client sending a handshake packet.  Until
-packets are protected by 1-RTT keys (see {{handshake}}), packets sent by a

This threw me off, since I thought we had agreed that this text change was to be in a separate PR that would get discussed, since it changes text from simple version negotiation to combining it with crypto keys. Especially since we have no agreement on making TLS part of the transport doc, I think this text ought to be reverted (I've posted on the commit of this PR as well.)

> +      opaque value<0..2^16-1>;
+   } TransportParameter;
+
+   struct {
+      select (Handshake.msg_type) {
+         case client_hello:
+            QuicVersion negotiated_version;
+            QuicVersion initial_version;
+
+         case encrypted_extensions:
+            QuicVersion supported_versions<2..2^8-4>;
+      };
+      TransportParameter parameters<30..2^16-1>;
+   } TransportParameters;
+~~~
+{: #figure-transport-parameters title="Definition of TransportParameters"}

I would separate this description of transport params out into a non-controversial separate PR. (starting from line 697)

> +New transport parameters can be registered according to the rules in
+{{iana-transport-parameters}}.
+
+
+### Version Negotiation Validation {#version-validation}
+
+The transport parameters include three fields that encode version information.
+These retroactively authenticate the version negotiation (see
+{{version-negotiation}}) that is performed prior to the cryptographic handshake.
+
+Inclusion of these values in an extension provides integrity protection for
+these values.  As a result, modification of version negotiation packets by an
+attacker can be detected.
+
+The client includes two fields in the transport parameters that it includes in
+the TLS ClientHello:

remove mention of TLS ClientHello

> +New transport parameters can be used to by endpoints to advertise new
+capabilities.  This can be used to negotiate new protocol behavior.  An endpoint
+MUST ignore transport parameters that it does not support.
+
+New transport parameters can be registered according to the rules in
+{{iana-transport-parameters}}.
+
+
+### Version Negotiation Validation {#version-validation}
+
+The transport parameters include three fields that encode version information.
+These retroactively authenticate the version negotiation (see
+{{version-negotiation}}) that is performed prior to the cryptographic handshake.
+
+Inclusion of these values in an extension provides integrity protection for
+these values.  As a result, modification of version negotiation packets by an

nit: "Inclusion of these values in an extension" --> "The handshake protocol"

> +* The negotiated_version is the version that was finally selected for use.  This
+  MUST be identical to the value that is on the packet that carries the
+  ClientHello.  A server that receives a negotiated_version that does not match
+  the version of QUIC that is in use MUST terminate the connection with a TBD
+  error code.
+
+* The initial_version is the version that the client initially attempted to use.
+  If the server did not send a version negotiation packet
+  {{version-negotiation-packet}}, this will be identical to the
+  negotiated_version.
+
+A stateful server can remember how version negotiation was performed and
+validate the initial_version value.
+
+A server that does not maintain state for early packets uses a different
+process. If the initial and negotiated versions are the same, a stateless server

Why are we discussing stateless servers? Version negotiation is part of the QUIC connection,  and it is not expected that the server would shoot a version negotiation packet and just drop the connection. At any rate, this is again a function change from what is, and I think belongs in a separate PR.

>  ### Crypto Handshake Protocol Features
 
+(TODO: Remove or migrate this section.)

Remove this from this PR.

-#### Encoding
+Definitions for each of the defined transport parameters are included in

I would remove the para above from the transport params PR, and modulo comments below, add the rest of the text into a non-controversial transport params PR. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/122#pullrequestreview-16294516

---

From: Mike Bishop <[email-masked]>
Date: Thu, 12 Jan 2017 16:05:33 -0800

I actually think this is appropriate, because it's describing how app-protocol selection works.  TLS already enables you to advertise lists of things you support, not all of which are rational combinations.  This is describing how TLS should handle ALPN in QUIC.

However, if you wanted to move part of this text to the -transport document and describe app-protocol selection independent of the crypto protocol in use, I wouldn't object.

---

From: Mike Bishop <[email-masked]>
Date: Thu, 12 Jan 2017 16:07:07 -0800

As before.  It *is* for TLS to do, since the server will run into this while trying to process ALPN, but you're correct that it's going to be an issue for any crypto protocol.

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Jan 2017 18:44:29 -0800

+   } TransportParameter;
+
+   struct {

Why would it be non-controversial - this is the entire point of this PR?

---

From: janaiyengar <[email-masked]>
Date: Fri, 13 Jan 2017 16:17:08 -0800

+   } TransportParameter;
+
+   struct {

Defining a structure to hold transport params, describing the various params, what to do at 0-RTT, are all basically non-controversial. This is mostly what is described from here and below, and I think that can be separated.

Having considered this, I think you're right. This makes sense in the context of describing how ALPN ought to work for QUIC. SGTM.

Yes, you're right. I hadn't fully processed ALPN and the new world order, but I think it makes sense when considering how TLS would use ALPN with QUIC.

---

From: Juan Benet <[email-masked]>
Date: Thu, 19 Jan 2017 00:44:31 -0800

jbenet commented on this pull request.

(a bit late to the discussion, but here are some thoughts from a large p2p project)

- It is pretty important to preserve the ability to _swap to a different crypto handshake protocol_, in case problems are found, or for some extremely sensitive applications. Specs tend to bake and solidify, so doors you close now may remain shut forever.
- Be careful with crypto agility-- negotiating handshake protocols can be a big security problem. Crypto agility without the ruthless removal of older protocols (and necessary rejection of old peers) has been known to cause massive problems (yes, hence everyone here asking for TLS >= 1.3, 👍  ). 
- Differentiate "crypto extensibility" from "crypto agility", as agl puts it here (https://www.imperialviolet.org/2016/05/16/agility.html):
    > Firstly I'd like to separate extensibility from agility. A protocol is extensible if you can add features to it without having to update every implementation at the same time—which is generally impossible. Cryptographic agility depends on having extensibility, at least if you ever want to use something that wasn't designed into a protocol at the beginning.
    >
    > Protocols should be extensible: the world keeps changing and no design is going to be perfect for all time. But extensibility is much harder in practice than it sounds.
- I personally think QUIC should have _extensibility_ regarding its crypto encryption protocol (and thus keep the "what is needed from a crypto protocol" section of the spec), but i also think crypto agility maybe should not be encouraged (i.e. negotiation is not necessary at all, only extensibility (a code specifying which protocol is being used may be sufficient). Negotiation/agility is not required because a meta-protocol that "negotiates which crypto protocol to use" could just as easily be the one used by those who require it/are fine to saddle on the security implications of crypto agility).
- In practice, it's ok to have two peers/a client+server pair _know_ what crypto protocol they will use to connect with each other (eg latest browsers using TLS 1.3, or even a "TLS >= 1.3 negotiation protocol", all ipfs peers using TLS 1.3 or CurveCP).
- But again, baking in TLS as _the_ only option at this point is the sort of thing that can "protocol ossify" and be quite difficult to upgrade from. I dont think you lose much (anything?) by keeping a way to specify which protocol the connection is using, which this version says SHOULD be TLS 1.3, but which leaves the door open to implementors / users / future protocol designers to upgrade smoothly to something else, without requiring a full new QUIC version (harder to do).
- > also +1 on binding a version of the protocol to TLS with text that acknowledges security versioning is done as protocol versioning.
  I don't think this solves the problem. It's a good idea when _everyone agrees_ which protocol is the right one. But consider that there has been a lot of suspicious activity even in standards bodies, and we may see a lot more in the coming years. Already it is the case that many well-known cryptographers/security people prefer other crypto primitives (eg djb crypto) or different crypto handshake protocols.
- i don't know if this is still relevant (or whether TLS 1.3 has absorbed DTLS 1.3 as far as QUIC is concerned), but i think it may be relevant to keep the door open to use DTLS 1.3 for unreliable "channels" (stream, but it's not really a stream). I dont think most standard users of QUIC will need unreliable streams, and you may have dropped it from the spec by this point, but i know users like WebRTC and IPFS would greatly benefit from muxing both unreliable channels and reliable streams, and use TLS for reliable and DTLS (or something else) for unreliable.

Sorry, there's a lot of feedback here, much that isn't up to date with everything going on in the WG. If you'd like me to separate any of these comments into other issues, please lmk.