Re: [quicwg/base-drafts] Improve KEY_PHASE description (#43)

[ianswett <[e-mail]>]

ianswett commented on this pull request.

Thanks for the huge update.

> @@ -226,24 +259,29 @@ document:
 
 # TLS in Stream 1
 
-QUIC completes its cryptographic handshake on stream 1, which means that the
-negotiation of keying material happens after the QUIC protocol has started.
-This simplifies the use of TLS since QUIC is able to ensure that the TLS
-handshake packets are delivered reliably and in order.
+QUIC reserves stream 1 for a TLS connection.  Stream contains a complete TLS

Stream contains is awkward.  Did you mean Stream 1 contains?

>  
-Prior to the completion of the TLS handshake, QUIC frames can be exchanged.
-However, these frames are not authenticated or confidentiality protected.
-{{pre-handshake}} covers some of the implications of this design and limitations
-on QUIC operation during this phase.
+Initial packets are not authenticated or confidentiality protected.
+{{pre-handshake}} covers some of the implications of this design.  This imposes
+some restrictions on what packets can be sent.  There are also restrictions on

This sentence seems mostly redundant.  Actually, most of this paragraph seems redundant to the one on line 162 and part of the paragraph at 214.

>  
 The server sends TLS handshake messages without protection (@A).  The server
 transitions from no protection (@A) to full 1-RTT protection (@C) after it sends
 the last of its handshake messages.
 
 Some TLS handshake messages are protected by the TLS handshake record
-protection.  However, keys derived at this stage are not exported for use in
-QUIC.  QUIC frames from the server are sent in the clear until the final
+protection.  These keys are not exported from the TLS connection for use in
+QUIC.  QUIC packets from the server are sent in the clear until the final

Why the transition from frames to packets?  I think frames is slightly more accurate, since a server could send a ServerHello, then encrypted data, and then have to resend the ServerHello in a new packet with a larger packet number.

>  
-Every time that a new set of keys is used for protecting outbound packets, the
-KEY_PHASE bit in the public flags is toggled.  The KEY_PHASE bit starts out with
-a value of 0 and is set to 1 when the first encrypted packets are sent.  Once
-the connection is fully enabled, the KEY_PHASE bit can toggle between 0 and 1 as
-keys are updated (see {{key-update}}).
+A QUIC server starts the process by providing TLS with any stream 1 octets that
+might have arrived.  Only in-sequence packets are delivered; packets that arrive

I think you can shorten this section(up to 379) a bit.  The first sentence says most of the important information.  QUIC streams mean in-order delivery, so just saying in-order once should be sufficient.  There's no need to describe how QUIC provides in order delivery.

> +
+As TLS reports the availability of 0-RTT and 1-RTT keys, new keying material can
+be exported from TLS and used for QUIC packet protection.  At each transition
+during the handshake a new secret is exported from TLS and packet protection
+keys are derived from that secret.
+
+Every time that a new set of keys is used for protecting outbound packets, the
+KEY_PHASE bit in the public flags is toggled.
+
+Once the connection is fully enabled, the KEY_PHASE bit allows a recipient to
+detect a change in keying material without necessarily needing to receive the
+first packet that triggered the change.  An endpoint that notices a changed
+KEY_PHASE bit can update keys and decrypt the packet that contains the changed
+bit, see {{key-update}}.
+
+The KEY_PHASE bit on the public flags is the most significant bit (0x80).

I think we should change this to the diversification nonce bit(0x04), since that's not needed with TLS 1.3 crypto.

> +The KEY_PHASE bit on the public flags is the most significant bit (0x80).
+{{key-phase-table}} summarizes the different values for the KEY_PHASE bit.
+
+| Scenario | Client (No 0-RTT) | Client (0-RTT) | Server |
+|:-|-:|-:|-:|
+| Handshake | 0 | 0 |  0 |
+| 0-RTT Data | - | 1 | - |
+| 1-RTT Data | 1 | 0 | 1 |
+| 1st Key Update | 0 | 1 | 0 |
+| 2nd Key Update | 1 | 0 | 1 |
+{: #key-phase-table title="Summary of KEY_PHASE Values"}
+
+{{key-phase-table}} shows that a client marks 1-RTT with a different KEY_PHASE
+bit depending on whether 0-RTT is attempted.  Attempting 0-RTT therefore results
+in fully protected data having different KEY_PHASE values.  This is true even if
+0-RTT data is rejected and ignored by the server.

Does this work?  If the 0RTT packets are never received by the server, wouldn't 0RTT with packet loss and 1RTT be identical from the server's perspective, making it look like 0 for handshake and 0 for 1RTT data, with no 1 in between.

I think the approach of using the version bit, possibly requiring the version bit to be set for handshake packets, then requiring it not be set for 0RTT data, would be a reasonable option?  There are few ways to make it work, but the above doesn't seem like it works, and generally seems unnecessarily complex.

> +keys.  Future packets are therefore protected with 1-RTT keys and marked with a
+KEY_PHASE of 1.
+
+Like the client, a server MUST send retransmissions of its unprotected handshake
+messages or acknowledgments for unprotected handshake messages sent by the
+client in unprotected packets (KEY_PHASE=0).
+
+The second flight of TLS handshake messages from a client is always protected
+with the current packet protection keys.  This includes the TLS
+end_of_early_data alert, which a client sends before its final set of handshake
+messages.
+
+
+### Retransmission and Acknowledgment of Unprotected Packets
+
+The handshake messages the first flight of handshake messages from both client

Not a sentence?

> +* A server MUST NOT retransmit any of its TLS handshake messages with 1-RTT
+  keys.  The client needs these messages in order to determine the 1-RTT keys.
+
+A HelloRetryRequest might be used to reject an initial ClientHello.  A
+HelloRetryRequest handshake message and any second ClientHello that is sent in
+response MUST also be sent without packet protection.  This is natural, because
+no new keying material will be available when these messages need to be sent.
+
+Note:
+
+: An alternative way of identifying handshake data that needs to be sent without
+  protection is to collect all handshake data from before TLS provides the first
+  keys (see {{key-ready-events}}).
+
+Retransmissions of these handshake messages MUST be send in unprotected packets
+(with a KEY_PHASE of 0).  Any ACK frames for these messages MUST also be send in

I think acks can be encrypted.  In particular, one could ack encrypted data and unencrypted data in the same ack.  Otherwise you'd have to clear out all ack data as soon as you transitioned keys.  It's possible, but a bit weird and it seems unnecessary.  As discussed, it's more important we don't ack encrypted data with unencrypted acks.

> +decrypt these packets, which will be unsuccessful.  Though a server that has
+1-RTT keys can safely discard any unprotected handshake messages from the
+client, an unprotected ACK frame could indicate that the server needs to
+retransmit its own handshake messages.  ACK frames therefore cannot be
+discarded.
+
+Until the server has received a positive acknowledgment for all of its
+unprotected handshake messages, either in the form of an explicit acknowledgment
+or implicitly through the use of 1-RTT keys, it MUST NOT discard packets with a
+KEY_PHASE of 0 if they cannot be decrypted successfully.  If these packets are
+unprotected and contain ACK frames, those ACK frames MUST be recovered and used
+to determine whether to retransmit TLS handshake messages.
+
+TBD:
+
+: We could observe/require that all unprotected packets are marked with a

+1, it also means simplifying a bunch of text that says "If you have 0RTT data, then X, otherwise Y"

> @@ -789,9 +1025,9 @@ ISSUE:
   authenticating the initial value, so that peers can be sure that they haven't
   missed an initial message.
 
-In addition to denying endpoints messages, an attacker to generate packets that
-cause no state change in a recipient.  See {{useless}} for a discussion of these
-risks.
+In addition to causing valid packets to be dropped, an attacker to generate

Not a sentence

-- 
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/43#pullrequestreview-10560566