Subject: [quicwg/base-drafts] Stronger migration handshake (#2370)

From: Christian Huitema <[email-masked]>
Date: Sat, 26 Jan 2019 19:25:54 -0800

State requirement that clients must wait until the server considers the handshake finished before starting migration.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Merge pull request #11 from quicwg/master
  * State requirement that clients must wait until ACK of 1-RTT

-- File Changes --

    M draft-ietf-quic-transport.md (5)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2370.patch
https://github.com/quicwg/base-drafts/pull/2370.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/2370

---

From: MikkelFJ <[email-masked]>
Date: Sat, 26 Jan 2019 22:19:15 -0800

mikkelfj commented on this pull request.

> @@ -1847,6 +1847,11 @@ endpoint migrates to a new address.
 An endpoint MUST NOT initiate connection migration before the handshake is
 finished and the endpoint has 1-RTT keys.  The design of QUIC relies on
 endpoints retaining a stable address for the duration of the handshake.
+Clients MUST NOT initiate connection migration before they are
+certain that their peer also considers the handshake finished. This
+means that in addition to waiting for availability of 1-RTT keys,
+clients MUST wait acknowledgement by the server of one of their
+1-RTT messages before initiating connection migration.

Suggest singular form: A client...

> A client MUST NOT initiate connection migration before it is certain
that it's peer also considers the handshake finished. This means that
in addition to waiting for availability of 1-RTT keys, a client MUST
also await acknowledgement of at least one of it's 1-RTT transmitted
packets before initiating connection migration.

-- 
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/2370#pullrequestreview-196813558

---

From: MikkelFJ <[email-masked]>
Date: Sat, 26 Jan 2019 22:22:36 -0800

An endpoint also MUST NOT initiate connection migration if the peer sent the
 `disable_migration` transport parameter during the handshake.  An endpoint which

Not in this PR, but why do we use both client, server, peer, and endpoint, when migration can only happen from the client in V1 (or did I miss something?)

-- 
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/2370#pullrequestreview-196813615

---

From: erickinnear <[email-masked]>
Date: Sun, 27 Jan 2019 04:03:24 -0800

erickinnear commented on this pull request.

Some of that's just a mix of different text coming in at different times, I think. 
At one point, the idea was to deliberately keep the text agnostic (for the most part, any text restricting who can initiate is clearly going to be one-sided) to endpoint, since that allowed reasoning about it as initiator and responder rather than client vs. server. 

-- 
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/2370#discussion_r251233066

---

From: erickinnear <[email-masked]>
Date: Sun, 27 Jan 2019 04:05:01 -0800

Just a nit, but this appears to put stricter requirements on the statements made in the previous two sentences, is there any reason not to merge them together? 

-- 
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/2370#pullrequestreview-196824474

---

From: Christian Huitema <[email-masked]>
Date: Sun, 27 Jan 2019 11:41:40 -0800

huitema commented on this pull request.

The part about 1-rtt ack is actually client specific. I think I was influenced by the picotls implementation that only provides the 1-rtt read key after CFIN has been received, and did not spell out the parallel server requirement that some 1rtt packet should have been received. Will do.

-- 
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/2370#discussion_r251251777

---

From: Christian Huitema <[email-masked]>
Date: Sun, 27 Jan 2019 12:13:06 -0800

@huitema pushed 1 commit.

e449a442086ecaf478c7a2b002a3f43c1a4b17b7  Update the proposed migration text based on comments.

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

---

From: Christian Huitema <[email-masked]>
Date: Sun, 27 Jan 2019 12:17:34 -0800

I rewrote this to be generally neutral, while leaving the specific requirements for server and client. Where did we specify that only the client initiates migrations in V1? I know of at least one implementation (not mine) that migrates the connection to a new CID as soon as the handshake is done and it has received "NEW CID" frames from the client.

-- 
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/2370#discussion_r251253257

---

From: MikkelFJ <[email-masked]>
Date: Sun, 27 Jan 2019 14:16:51 -0800

@huitema  > Where did we specify that only the client initiates migrations in V1?

I'm not sure, but from memory it was dropped at some point because symmetric ping pong got hairy during updates or some defence became difficult.

Maybe it didn't land? Found this:

> Caution:
If both endpoints change connection ID in response to seeing a change in connection ID from their peer, then this can trigger an infinite sequence of changes.

But there is this, which seems to be preferred address only - maybe I got it mixed up_

9.6, transport:
> Migrating a connection to a new server address mid-connection is left for future work. 

-- 
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/2370#discussion_r251257955

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 27 Jan 2019 14:29:08 -0800

kazuho commented on this pull request.

>  endpoints retaining a stable address for the duration of the handshake.
+An endpoint MUST NOT initiate connection migration before the handshake is
+finished for it and its peer and the endpoint has 1-RTT keys. This means
+that the server MUST NOT initiate connection migration before it has
+received at least one 1-RTT data from the client, and the client 
+MUST NOT initiate connection migration before receiving 
+acknowledgement by the server of at least one of its
+1-RTT messages.

How about reusing the event that triggers the 3 PTO timer to drop the handshake keys, instead of defining a different event?

Activation of the 3 PTO timer to drop the handshake key happens when an endpoint "has both received and acknowledged all CRYPTO frames for that encryption level and when all CRYPTO frames for that encryption level have been acknowledged by its peer." (https://quicwg.org/base-drafts/draft-ietf-quic-tls.html#rfc.section.4.9).

-- 
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/2370#pullrequestreview-196848476

---

From: Christian Huitema <[email-masked]>
Date: Sun, 27 Jan 2019 17:40:09 -0800

I think the updated version solves your issue.

-- 
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/2370#discussion_r251265791

---

From: Christian Huitema <[email-masked]>
Date: Sun, 27 Jan 2019 17:43:24 -0800

I really don't like the CRYPTO frames reference. There is no FIN mark on the crypto stream, and the QUIC engine proper does not know when "all crypto frames" have been received. I was careful to use signals that the transport stack understand: obtaining keys, receiving packets, etc.

-- 
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/2370#discussion_r251266038

---

From: ianswett <[email-masked]>
Date: Sun, 27 Jan 2019 18:04:48 -0800

ianswett commented on this pull request.

I'm happy with this approach, but I'd replace "before receiving acknowledgement by the server of at least one of its 1-RTT messages" with "before receiving a 1RTT ACK frame." for simplicity.

-- 
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/2370#discussion_r251267637

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 27 Jan 2019 19:19:31 -0800

@huitema 
> I really don't like the CRYPTO frames reference. There is no FIN mark on the crypto stream, and the QUIC engine proper does not know when "all crypto frames" have been received.  I was careful to u…

I am not sure if I agree with the observation. The requirement (i.e. "has both received and acknowledged all CRYPTO frames for that encryption level and when all CRYPTO frames for that encryption level have been acknowledged by its peer") can be implement as checking that 1-RTT read key is available and that the send buffer of CRYPTO frames in Handshake epoch is empty.

Anyways, I think using different events for starting the handshake key drop timer and permitting migration is an unnecessary complexity.

-- 
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/2370#discussion_r251274234

---

From: Christian Huitema <[email-masked]>
Date: Sun, 27 Jan 2019 19:34:08 -0800

@kazuho _"Anyways, I think using different events for starting the handshake key drop timer and permitting migration is an unnecessary complexity."_ I agree. I think there should be a "when is handshake complete" section specifying that, and we can reference it in places like dropping key, starting migration, or starting key rotations.

-- 
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/2370#discussion_r251275533

---

From: Martin Thomson <[email-masked]>
Date: Sun, 27 Jan 2019 21:41:22 -0800

If people want to talk about this this week, then this needs an issue.

-- 
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/2370#issuecomment-458004872

---

From: Christian Huitema <[email-masked]>
Date: Mon, 28 Jan 2019 01:56:02 -0800

cdaf4ff621a0a6f2ed841c758828ec3610f0cb6c  Add handshake completion section so we can have a single reference.

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

---

From: Christian Huitema <[email-masked]>
Date: Mon, 28 Jan 2019 13:09:28 -0800

@martinthomson that would be issue #2309 

-- 
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/2370#issuecomment-458302106

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 30 Jan 2019 18:22:34 -0800

janaiyengar commented on this pull request.

A few comments

> +### Handshake Completion {#handshake-completion}
+
+The connection establishment handshake concludes when both parties have
+successfully agreed on the protocol version and on encryption keys, and
+have obtained insurance that all necessary Initial and Handshake messages
+have been properly received by their peers, as defined in {{QUIC-TLS}}.
+
+For the server, this is achieved when the TLS stack informs the
+transport that the session establishment is complete, and that the
+1-RTT receive keys can now be used, as indicated in section 4.1.3 of
+{{QUIC-TLS}}.
+
+For the client, this is achieved after the 1-RTT keys become available 
+when the server acknowledges either the client's Handshake message carrying
+the last element of the Crypto Stream or an 1-RTT message sent by the client.
+Waiting for either of these events allows the client to verify the

```suggestion
Waiting for either of these events allows the client to verify that
```

> +
+The connection establishment handshake concludes when both parties have
+successfully agreed on the protocol version and on encryption keys, and
+have obtained insurance that all necessary Initial and Handshake messages
+have been properly received by their peers, as defined in {{QUIC-TLS}}.
+
+For the server, this is achieved when the TLS stack informs the
+transport that the session establishment is complete, and that the
+1-RTT receive keys can now be used, as indicated in section 4.1.3 of
+{{QUIC-TLS}}.
+
+For the client, this is achieved after the 1-RTT keys become available 
+when the server acknowledges either the client's Handshake message carrying
+the last element of the Crypto Stream or an 1-RTT message sent by the client.
+Waiting for either of these events allows the client to verify the
+completion of the handshake even in cases where the Handshake

```suggestion
the handshake is complete, even in cases where acknowledgements for
```

> +The connection establishment handshake concludes when both parties have
+successfully agreed on the protocol version and on encryption keys, and
+have obtained insurance that all necessary Initial and Handshake messages
+have been properly received by their peers, as defined in {{QUIC-TLS}}.
+
+For the server, this is achieved when the TLS stack informs the
+transport that the session establishment is complete, and that the
+1-RTT receive keys can now be used, as indicated in section 4.1.3 of
+{{QUIC-TLS}}.
+
+For the client, this is achieved after the 1-RTT keys become available 
+when the server acknowledges either the client's Handshake message carrying
+the last element of the Crypto Stream or an 1-RTT message sent by the client.
+Waiting for either of these events allows the client to verify the
+completion of the handshake even in cases where the Handshake
+acknowledgement is lost.

```suggestion
Handshake packets are lost.
```

> @@ -1532,6 +1532,24 @@ be unambiguously different to ensure no confusion about their interpretation.
 One way that a new format could be introduced is to define a TLS extension with
 a different codepoint.
 
+### Handshake Completion {#handshake-completion}
+
+The connection establishment handshake concludes when both parties have

I don't agree with some of this text, but importantly, it's more than is needed. Suggested rephrase of this paragraph, and combine this with the next paragraph: "Connection handshake is complete when version negotiation is complete and 1-RTT keys are available at both endpoints."

> @@ -1532,6 +1532,24 @@ be unambiguously different to ensure no confusion about their interpretation.
 One way that a new format could be introduced is to define a TLS extension with
 a different codepoint.
 
+### Handshake Completion {#handshake-completion}
+
+The connection establishment handshake concludes when both parties have
+successfully agreed on the protocol version and on encryption keys, and
+have obtained insurance that all necessary Initial and Handshake messages
+have been properly received by their peers, as defined in {{QUIC-TLS}}.
+
+For the server, this is achieved when the TLS stack informs the
+transport that the session establishment is complete, and that the
+1-RTT receive keys can now be used, as indicated in section 4.1.3 of
+{{QUIC-TLS}}.
+
+For the client, this is achieved after the 1-RTT keys become available 
+when the server acknowledges either the client's Handshake message carrying

Replace "the server acknowledges ... by the client" with "either all Handshake packets or a 1-RTT packet sent by the client are acknowledged"

-- 
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/2370#pullrequestreview-198395293

---

From: Christian Huitema <[email-masked]>
Date: Thu, 31 Jan 2019 16:49:32 -0800

4b096a19558f17aa9539868cd4767f6dbb0b0203  Per Tokyo interop: editors will complete the section "handshake completion", and the migration conditions will then be explained.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2370/files/cdaf4ff621a0a6f2ed841c758828ec3610f0cb6c..4b096a19558f17aa9539868cd4767f6dbb0b0203

---

From: Christian Huitema <[email-masked]>
Date: Thu, 31 Jan 2019 16:54:16 -0800

Jana, I temporarily removed the content of that section, since MT has taken the action to write the "handshake is complete" text, introducing the new "previous epoch done" frame, etc.

-- 
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/2370#discussion_r252900924

---

From: Christian Huitema <[email-masked]>
Date: Thu, 31 Jan 2019 16:54:48 -0800

Removed that text, waiting for MT.

-- 
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/2370#discussion_r252901038