Re: [quicwg/base-drafts] Mask packet numbers with a per-connection-ID key (#1043)

[Mike Bishop <[e-mail]>]

MikeBishop commented on this pull request.

Looks reasonable; minor nits.

>  
 ~~~
-    struct {
-        uint16 length = Length;
-        opaque label<6..255> = "QUIC " + Label;
-        uint8 hashLength = 0;
-    } QuicHkdfLabel;
+struct {
+   uint16 length = Length;
+   opaque label<6..255> = "QUIC " + Label;
+   uint8 connectionId<0>;

Is this included just to emphasize that it's zero-length, or does it actually include a byte representing the zero-length array?

> -TLS state machine reports that the ClientHello has been sent, the 0-RTT keys can
-be generated and installed for writing.  When the TLS state machine reports
-completion of the handshake, the 1-RTT keys can be generated and installed for
-writing.
+Note:
+
+: A connection ID might be omitted from the encoding of a packet, as is
+  permitted when the omit_connection_id transport parameter is used, but the
+  connection ID is still used by QHKDF-Expand to derive these values.
+
+The QUIC record protection initially starts with keying material derived from
+handshake keys.  When the TLS state machine reports that the ClientHello has
+been sent, 0-RTT keys can be generated and installed for writing, if 0-RTT is
+available.  Finally, the TLS state machine reports completion of the handshake,
+the 1-RTT keys can be generated and installed for writing.  Additionally, new
+keys are installed each time that the connection ID changes.

Does this need to mention TLS key updates?

> @@ -606,9 +606,12 @@ A Retry packet uses long headers with a type value of 0x7E.  It carries
 cryptographic handshake messages and acknowledgments.  It is used by a server
 that wishes to perform a stateless retry (see {{stateless-retry}}).
 
-The packet number and connection ID fields echo the corresponding fields from
-the triggering client packet.  This allows a client to verify that the server
-received its packet.
+The connection ID field echoes the corresponding fields from the triggering

One field

> @@ -606,9 +606,12 @@ A Retry packet uses long headers with a type value of 0x7E.  It carries
 cryptographic handshake messages and acknowledgments.  It is used by a server
 that wishes to perform a stateless retry (see {{stateless-retry}}).
 
-The packet number and connection ID fields echo the corresponding fields from
-the triggering client packet.  This allows a client to verify that the server
-received its packet.
+The connection ID field echoes the corresponding fields from the triggering
+client packet.  This allows a client to correlate a Retry with the Initial
+packet that it sends that the server received its packet.

This sentence doesn't parse.

> @@ -642,11 +645,11 @@ server and client.
 The connection ID field in a Handshake packet contains a connection ID
 that is chosen by the server (see {{connection-id}}).
 
-The first Handshake packet sent by a server contains a randomized packet number.
-This value is increased for each subsequent packet sent by the server as
-described in {{packet-numbers}}.  The client increments the packet number from
-its previous packet by one for each Handshake packet that it sends (which might
-be an Initial, 0-RTT Protected, or Handshake packet).
+The first Handshake packet sent by a server contains a packet number of 0.  This
+value is increased for each subsequent packet sent by the server as described in
+{{packet-numbers}}.  The client increments the packet number from its previous
+packet by one for each packet that it sends (which might be an
+Initial, 0-RTT Protected, or Handshake packet).

I know this isn't new text, but this statement about the client is redundant -- the client's behavior is already described at line 586.  (In fact, it might be worth consolidating them and simply describing that the first packet sent by each party has a packet number of zero and is incremented with each packet.)

> +packet_number = (masked_packet_number - pn_mask) MOD num_values
+~~~
+
+Packet numbers are not obscured when encoded in frames, such as ACK
+({{frame-ack}}).
+
+These changes are applied before packet protection, so the additional
+authenticated data (AAD) input includes masked values.
+
+These are not true cryptographic confidentiality protections, so entities other
+than endpoints are likely to be able to recover the underlying values by
+observing multiple packets.  For instance, in this version of QUIC, packet
+numbers still increase monotonically as long as the connection ID remains
+constant.  However, these values are different for client and server, and they
+change when a connection ID changes, together ensuring that flows with different
+connection IDs are not linkable based on the value of these fields.

Strong statement; maybe we stick to making it more difficult to link?

-- 
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/1043#pullrequestreview-87606002