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

From: Mike Bishop <[email-masked]>
Date: Tue, 10 Dec 2019 08:24:48 -0800

MikeBishop 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-329968226

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 10 Dec 2019 21:46:27 -0800

janaiyengar commented on this pull request.

> +## 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 consider the handshake to be confirmed when it receives an
+acknowledgement for a 1-RTT packet.  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.

Sorry about catching this so late, but this is entirely editorial: This para does not belong here; this section is the frame description section. Can you move this para to the {{handshake-confirmed}} section in QUIC-TLS?

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Dec 2019 21:53:09 -0800

martinthomson commented on this pull request.

+

Gah.  I had the same thought before but failed to write it down.  Thanks for catching that.  I'll open a 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/3145#discussion_r356413912

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Dec 2019 21:57:25 -0800

@marten-seemann see https://github.com/marten-seemann/base-drafts/pull/4

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

---

From: Marten Seemann <[email-masked]>
Date: Tue, 10 Dec 2019 21:59:04 -0800

@marten-seemann pushed 2 commits.

d3aabccab43fa412d216cb0d3192c396a4141823  Move alternative confirmation method
d3e4cfaa591d4dc610b90f95bdebc5649bb3c543  Merge pull request #4 from quicwg/move-alt-confirmation

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

---

From: ianswett <[email-masked]>
Date: Thu, 19 Dec 2019 14:20:45 -0800

ianswett approved this pull request.

> @@ -390,13 +390,15 @@ 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 a HANDSHAKE_DONE frame is received.
+
+A client MAY consider the handshake to be confirmed when it receives an
+acknowledgement for a 1-RTT packet.  This can be implemented by recording the
+lowest packet number sent with 1-RTT keys, and the highest value of the Largest

```suggestion
lowest packet number sent with 1-RTT keys, and comparing to the Largest
```

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

---

From: Marten Seemann <[email-masked]>
Date: Mon, 23 Dec 2019 21:10:20 -0800

@marten-seemann pushed 1 commit.

cf39d12c5e0e5421522e95cf2b6543b0060b49ee  apply @ianswett'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/c4041eb0eb6fd2fbf863ca4e7b746cb933986a67..cf39d12c5e0e5421522e95cf2b6543b0060b49ee

---

From: Mike Bishop <[email-masked]>
Date: Fri, 27 Dec 2019 08:13:28 -0800

Re-reviewing, since there have been a lot of commits since I reviewed before.  Still looks good.

 
+lowest packet number sent with 1-RTT keys, and comparing it to the Largest
+Acknowledged field in any received 1-RTT ACK frame: once the latter is higher

```suggestion
Acknowledged field in any received 1-RTT ACK frame: once the latter is greater
```

-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 a HANDSHAKE_DONE frame is received.
+
+A client MAY consider the handshake to be confirmed when it receives an
+acknowledgement for a 1-RTT packet.  This can be implemented by recording the
+lowest packet number sent with 1-RTT keys, and comparing it to 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.

```suggestion
than or equal to the former, the handshake is confirmed.
```
I see the discussion below; I think it's better as "is," since you're describing how it would be implemented.  However, that's certainly not a hard blocker.

> @@ -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.
+
+

Marking this branch resolved because this text has been added.

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

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 31 Dec 2019 20:15:55 -0800

I agree. I think this should be "is".

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