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

[Martin Thomson <[e-mail]>]

martinthomson commented on this pull request.

This seems workable.

> @@ -760,14 +756,12 @@ and ignoring any outstanding Initial packets.
 
 ### Discarding Handshake Keys
 
-An endpoint MUST NOT discard its handshake keys until the TLS handshake is
-confirmed ({{handshake-confirmed}}).  An endpoint SHOULD discard its handshake
-keys as soon as it has confirmed the handshake.  Most application protocols
-will send data after the handshake, resulting in acknowledgements that allow
-both endpoints to discard their handshake keys promptly.  Endpoints that do
-not have reason to send immediately after completing the handshake MAY send
-ack-eliciting frames, such as PING, which will cause the handshake to be
-confirmed when they are acknowledged.
+An endpoint MUST discard its handshake keys when the TLS handshake is confirmed

A "MAY" suffices here, does it not?  Or are you concerned about having to think about Handshake keys after migration?

It could be enough to say that Handshake packets MUST NOT be sent on any path other than the one on which the connection was established.

> @@ -760,14 +756,12 @@ and ignoring any outstanding Initial packets.
 
 ### Discarding Handshake Keys
 
-An endpoint MUST NOT discard its handshake keys until the TLS handshake is
-confirmed ({{handshake-confirmed}}).  An endpoint SHOULD discard its handshake
-keys as soon as it has confirmed the handshake.  Most application protocols
-will send data after the handshake, resulting in acknowledgements that allow
-both endpoints to discard their handshake keys promptly.  Endpoints that do
-not have reason to send immediately after completing the handshake MAY send
-ack-eliciting frames, such as PING, which will cause the handshake to be
-confirmed when they are acknowledged.
+An endpoint MUST discard its handshake keys when the TLS handshake is confirmed
+({{handshake-confirmed}}).  The server MUST send a HANDSHAKE_DONE frame when the
+handshake completes.  Most application protocols will send data after the
+handshake.  A server might want to minimize the number of packets sent by
+bundling the HANDSHAKE_DONE frame with other data.  It MAY delay sending of the

I think that you can drop the bundling sentence.

> @@ -760,14 +756,12 @@ and ignoring any outstanding Initial packets.
 
 ### Discarding Handshake Keys
 
-An endpoint MUST NOT discard its handshake keys until the TLS handshake is
-confirmed ({{handshake-confirmed}}).  An endpoint SHOULD discard its handshake
-keys as soon as it has confirmed the handshake.  Most application protocols
-will send data after the handshake, resulting in acknowledgements that allow
-both endpoints to discard their handshake keys promptly.  Endpoints that do
-not have reason to send immediately after completing the handshake MAY send
-ack-eliciting frames, such as PING, which will cause the handshake to be
-confirmed when they are acknowledged.
+An endpoint MUST discard its handshake keys when the TLS handshake is confirmed
+({{handshake-confirmed}}).  The server MUST send a HANDSHAKE_DONE frame when the
+handshake completes.  Most application protocols will send data after the
+handshake.  A server might want to minimize the number of packets sent by
+bundling the HANDSHAKE_DONE frame with other data.  It MAY delay sending of the
+HANDSHAKE_DONE frame by no more than one round-trip.

This delay sentence is somewhat in conflict with the preceding requirement, so maybe instead say "MUST send within 1 round-trip time of handshake completion."

> @@ -5495,6 +5495,15 @@ 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 MUST be retransmitted until it is acknowledged by the client.

You should move this statement to [Section 13.3](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#retransmission-of-information) for consistency reasons.

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