Subject: Re: [quicwg/base-drafts] use a HANDSHAKE_DONE frame to drive the handshake to confirmation (#3145)

From: David Schinazi <[email-masked]>
Date: Sat, 16 Nov 2019 19:13:37 -0800

DavidSchinazi requested changes on this pull request.

I support this PR, modulo minor changes.

> @@ -385,13 +385,9 @@ perspective of the endpoint in question.
 
 ### Handshake Confirmed {#handshake-confirmed}
 
-In this document, the TLS handshake is considered confirmed at an endpoint when
-the following two conditions are met: the handshake is complete, and the
-endpoint has received an acknowledgment for a packet sent with 1-RTT keys.
-This second condition can be implemented by recording the lowest packet number
-sent with 1-RTT keys, and the highest value of the Largest Acknowledged field
-in any received 1-RTT ACK frame: once the latter is higher than or equal to the
-former, the handshake is confirmed.
+In this document, the TLS handshake is considered confirmed at the server when
+the handshake completes. At the client, the handshake is considered confirmed
+when the HANDSHAKE_DONE frame is received.

nit: could we say "when *a* HANDSHAKE_DONE frame is received."

> @@ -5495,6 +5497,16 @@ Reason Phrase:
   This SHOULD be a UTF-8 encoded string {{!RFC3629}}.

+## HANDSHAKE_DONE frame {#handshake-done-frame}
+
+The server uses the HANDSHAKE_DONE frame (type=0x1e) to signal confirmation of
+the handshake to the client.  The HANDSHAKE_DONE frame contains no additional
+fields.
+
+This frame can only be sent by the server. A server MUST treat receipt of a
+HANDSHAKE_DONE frame as a connection error of type PROTOCOL_VIOLATION.
+
+

Can we add the following paragraph to this section:

```
Clients MAY treat receipt of an acknowledgment for a packet it sent with 1-RTT
keys as receipt of a HANDSHAKE_DONE frame.  This can be implemented by
recording the lowest packet number sent with 1-RTT keys, and the highest
value of the Largest Acknowledged field in any received 1-RTT ACK frame:
once the latter is higher than or equal to the former, the handshake can be
confirmed.
```

> @@ -5495,6 +5497,16 @@ Reason Phrase:
   This SHOULD be a UTF-8 encoded string {{!RFC3629}}.

+## HANDSHAKE_DONE frame {#handshake-done-frame}
+
+The server uses the HANDSHAKE_DONE frame (type=0x1e) to signal confirmation of
+the handshake to the client.  The HANDSHAKE_DONE frame contains no additional
+fields.
+
+This frame can only be sent by the server. A server MUST treat receipt of a

Can we add the following sentence of the start of this paragraph:

```
Servers MUST NOT send a HANDSHAKE_DONE frame before confirming
the handshake.
```

-- 
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/3145#pullrequestreview-317998542

---

From: Marten Seemann <[email-masked]>
Date: Sat, 16 Nov 2019 19:25:33 -0800

marten-seemann commented on this pull request.

Before **completing** the handshake.

-- 
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/3145#discussion_r347116283

---

From: Marten Seemann <[email-masked]>
Date: Sat, 16 Nov 2019 19:27:40 -0800

@marten-seemann pushed 1 commit.

969c862258c8bd8ee5be355889a379078f61fcc6  apply David's suggestions, with minor modifications

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3145/files/d1a4dcee66481e203df7909d8499dccb330fd2a3..969c862258c8bd8ee5be355889a379078f61fcc6

---

From: Jana Iyengar <[email-masked]>
Date: Sat, 16 Nov 2019 19:28:23 -0800

janaiyengar commented on this pull request.

s / the handshake can be confirmed / the handshake is confirmed

-- 
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/3145#discussion_r347116358

---

From: David Schinazi <[email-masked]>
Date: Sat, 16 Nov 2019 19:32:53 -0800

DavidSchinazi commented on this pull request.

I intentionally made it "can be" because this is a MAY

-- 
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/3145#discussion_r347116466

---

From: David Schinazi <[email-masked]>
Date: Sat, 16 Nov 2019 19:35:37 -0800

DavidSchinazi approved this pull request.

Thanks for making changes, I support this.

-- 
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/3145#pullrequestreview-317999190

---

From: ianswett <[email-masked]>
Date: Sat, 16 Nov 2019 19:48:21 -0800

ianswett approved this pull request.

Thanks for adding the text about a 1-RTT ACK being equivalent to a client receiving HANDSHAKE_DONE.

-- 
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/3145#pullrequestreview-317999416

---

From: martinduke <[email-masked]>
Date: Sat, 16 Nov 2019 19:55:08 -0800

martinduke approved this pull request.

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/3145#pullrequestreview-317999544

---

From: Marten Seemann <[email-masked]>
Date: Sat, 16 Nov 2019 19:55:15 -0800

a1df2ca28bfaefc1ec22ca81efe5bb17615b22fe  send HANDSHAKE_DONE immediately

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3145/files/969c862258c8bd8ee5be355889a379078f61fcc6..a1df2ca28bfaefc1ec22ca81efe5bb17615b22fe

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 16 Nov 2019 23:16:51 -0800

kazuho 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/3145#pullrequestreview-318004286

---

From: Christian Huitema <[email-masked]>
Date: Sun, 17 Nov 2019 00:36:47 -0800

huitema approved this pull request.

Ship 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/3145#pullrequestreview-318007302

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 17 Nov 2019 05:05:15 -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/3145#pullrequestreview-318019194

---

From: Ryan Hamilton <[email-masked]>
Date: Sun, 17 Nov 2019 13:00:51 -0800

Looks great!

-- 
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/3145#issuecomment-554786594

---

From: Ryan Hamilton <[email-masked]>
Date: Sun, 17 Nov 2019 13:01:17 -0800

RyanAtGoogle 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/3145#pullrequestreview-318043876

---

From: Martin Thomson <[email-masked]>
Date: Sun, 17 Nov 2019 16:15:28 -0800

martinthomson approved this pull request.

One comment, but this is good.

> @@ -5495,6 +5497,24 @@ Reason Phrase:

+## HANDSHAKE_DONE frame {#handshake-done-frame}
+
+The server uses the HANDSHAKE_DONE frame (type=0x1e) to signal confirmation of
+the handshake to the client.  The HANDSHAKE_DONE frame contains no additional
+fields.
+
+This frame can only be sent by the server. Servers MUST NOT send a
+HANDSHAKE_DONE frame before completing the handshake.  A server MUST treat
+receipt of a HANDSHAKE_DONE frame as a connection error of type
+PROTOCOL_VIOLATION.
+
+A client MAY treat receipt of an acknowledgment for a packet it sent with 1-RTT
+keys as receipt of a HANDSHAKE_DONE frame.  This can be implemented by recording

I misread this first sentence at first and got the wrong impression.  I think that this belongs in the other document, but it should probably instead say that:

> A client MAY consider the handshake to be confirmed when it receives an acknowledgement for a 1-RTT packet.  ...[the how to implement bit]

-- 
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/3145#pullrequestreview-318056964

---

From: Marten Seemann <[email-masked]>
Date: Mon, 18 Nov 2019 00:04:06 -0800

e163adc74a30cf73bdcca5dfd238a10b137b2a4b  apply mt's suggestion

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

---

From: Marten Seemann <[email-masked]>
Date: Mon, 18 Nov 2019 00:04:24 -0800

Thanks. I applied your suggestion.

-- 
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/3145#discussion_r347240088