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

From: Martin Thomson <[email-masked]>
Date: Mon, 06 Feb 2017 16:12:39 -0800

@martinthomson pushed 1 commit.

1da4ede  Remove extra sentence on ignoring unknown 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/ca264eb533a4b598d6c2a16619736ee228ff1d20..1da4ede787b1d955fcce4f09b87ccea6f72d36a1

---

From: Martin Thomson <[email-masked]>
Date: Mon, 06 Feb 2017 16:12:59 -0800

54b749a  editorial

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 06 Feb 2017 18:12:22 -0800

@martinthomson pushed 2 commits.

70a28cb  Fix formatting
9c2f2c1  Proofread version negotiation and correct errors

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 06 Feb 2017 21:04:31 -0800

72920a7  Use hex, add initial registry contents

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 07 Feb 2017 00:58:10 -0800

c3d7e70  Transport parameters during 0-RTT are either remembered or defaults

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 07 Feb 2017 01:04:41 -0800

@janaiyengar, I think that this is basically there.  I had to pick some defaults for transport parameters in order to deal with the conclusion to #126.  I think that's a terrible decision, but I'm clearly in the rough.  I picked very conservative defaults (gmail-level concurrency, yay) so that constrained servers don't get too badly screwed by this decision.

I moved the transport-requirements up and scrubbed it a little.  As a result this also fixes #9.

This is now getting too big - at the point that this starts fixing multiple issues, I think that we have to either commit to fixing niggles in follow-up changes, or abandon all the work.  I'd rather go with the former if at all possible.

-- 
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#issuecomment-277939915

---

From: janaiyengar <[email-masked]>
Date: Fri, 17 Feb 2017 09:51:36 -0800

Martin -- not sure where this PR stands; is it waiting on me? I would at least not remove the Crypto Requirements. I'd like to see that remain in the text (or some form of it.)

-- 
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#issuecomment-280719746

---

From: Martin Thomson <[email-masked]>
Date: Fri, 17 Feb 2017 15:00:06 -0800

@janaiyengar, the crypto requirements are simply moved (and reworded).  And yes, I'm still waiting on you.

-- 
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#issuecomment-280789919

---

From: janaiyengar <[email-masked]>
Date: Fri, 17 Feb 2017 18:09:20 -0800

janaiyengar commented on this pull request.

A few comments. The most important one being the shift in where source address spoofing prevention (STK) lies.

>  ## 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 sent by client to the server MUST carry handshake information
-as data on Stream ID 1.
+establishment latency.  QUIC allocates stream 1 for the cryptographic handshake.
+This version of QUIC uses TLS.
+
+QUIC provides this stream with reliable, ordered delivery of data.  In return,
+TLS provides QUIC with:

Can you change this to "the cryptographic handshake provides QUIC with"?

> +     packets, and
+
+   * 1-RTT keys have forward secrecy
+
+* authenticated values for the transport parameters of the peer (see
+  {{transport-parameters}})
+
+* authenticated confirmation of version negotiation (see {{version-validation}})
+
+* authenticated negotiation of an application protocol (TLS uses ALPN
+  {{?RFC7301}} for this purpose)
+
+* for the server, the ability to carry data that provides assurance that the
+  client can receive packets that are addressed with the transport address that
+  is claimed by the client (see {{source-address-token}})
+

This changes what the draft is saying in a fundamental way. The draft currently says that address proofing is handled by QUIC and need not be done by TLS. This modification says exactly the opposite.

It's also missing any mention of cert compression, which causes measurable perforamance difference. Granted that is detail that doesn't belong in this document, so I'm fine with not mentioning that.

> +         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"}
+
+The `extension_data` field of the quic_transport_parameters extension defined in
+{{QUIC-TLS}} contains a TransportParameters value.  TLS encoding rules are
+therefore used to encode the transport parameters.
+

What are "TLS encoding rules"?

-### Proof of Source Address Ownership
+The client includes two fields in the transport parameters:
+
+* 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.

"terminate the connection with a QUIC_VERSION_NEGOTIATION_MISMATCH error code."

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

---

From: Martin Thomson <[email-masked]>
Date: Sat, 18 Feb 2017 01:52:51 -0800

110effc  Fix long line

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

---

From: Martin Thomson <[email-masked]>
Date: Sun, 19 Feb 2017 19:39:03 -0800

martinthomson commented on this pull request.

Thanks for reviewing @janaiyengar, I have made the changes you suggest.  Please check out #275 when it comes to address validation.  It builds on top of this PR, I'll rebase it shortly.

TER?  It's an X.690-series ITU recommendation?  Not really.

If you follow the reference to [Section 3 of TLS 1.3](https://tlswg.github.io/tls13-spec/#rfc.section.3), you can see those rules laid out for you.

+

This is the first part of the handling we agreed to.  The cryptographic protocol carries the data (though the cryptographic protocol itself is carried by QUIC).  It does this .... well, I won't reiterate why when I have a PR that goes through and implements the full client address validation we agreed to in #275.  I encourage you to read that and not get too excited about this particular PR coming up short.

Done.

Thanks.

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

---

From: Martin Thomson <[email-masked]>
Date: Sun, 19 Feb 2017 22:46:12 -0800

686aad7  Correct line length

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

---

From: janaiyengar <[email-masked]>
Date: Tue, 21 Feb 2017 14:52:35 -0800

+

Gotcha. Cool -- thanks for pointing me to #275.

-- 
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#discussion_r102340256

---

From: janaiyengar <[email-masked]>
Date: Tue, 21 Feb 2017 14:55:43 -0800

Ping on this comment -- since you're probably going to have to rebase anyways.

-- 
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#discussion_r102340872

---

From: janaiyengar <[email-masked]>
Date: Tue, 21 Feb 2017 14:55:53 -0800

janaiyengar approved this pull request.

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

---

From: janaiyengar <[email-masked]>
Date: Tue, 21 Feb 2017 14:56:16 -0800

One minor change requested, but LGTM.

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 21 Feb 2017 15:17:48 -0800

1ffe4e9  Fix case

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 21 Feb 2017 15:17:55 -0800

Merged #122.

-- 
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#event-971664573