Re: [quicwg/base-drafts] Add retry integrity tag (#3120)

[Martin Thomson <[e-mail]>]

martinthomson approved this pull request.

Some editorial suggestions and one question about a length field.

I'm ambivalent with the implicit ODCID thing.  I like implicit things generally because you can't succeed if you don't have the right information, but this seems like it might still fail if the client doesn't check the tag.  We could completely avoid that being an issue if we enciphered the token, but that might change the optimization options enough that it isn't worthwhile.

> @@ -1197,6 +1197,64 @@ TLS ClientHello.  The server MAY retain these packets for later decryption in
 anticipation of receiving a ClientHello.
 
 
+## Retry Packet Integrity {#retry-integrity}
+
+Retry packets (see the Retry Packet section of {{QUIC-TRANSPORT}}) carry a
+Retry Integrity Tag that provides two properties: it allows discarding
+packets that have accidentally been corrupted by the network, and it mitigates
+off-path attackers' ability to send valid Retry packets.
+
+The Retry Integrity Tag is a 128-bit field that is computed as the output of
+AEAD_AES_128_GCM used with the following inputs:

```suggestion
AEAD_AES_128_GCM {{!AEAD=RFC5116}} used with the following inputs:
```

> @@ -2736,7 +2736,7 @@ available.
 
 ## Protected Packets {#packet-protected}
 
-All QUIC packets except Version Negotiation and Retry packets use authenticated
+All QUIC packets except Version Negotiation packets use authenticated
 encryption with additional data (AEAD) {{!RFC5116}} to provide confidentiality

The confidentiality bit means that this statement still applies to Retry.

> @@ -4111,37 +4111,31 @@ wishes to perform a retry (see {{validate-handshake}}).
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                 Source Connection ID (0..160)               ...
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-| ODCID Len (8) |
-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-|          Original Destination Connection ID (0..160)        ...
-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                        Retry Token (*)                      ...

In the interests of cleaner parsing, should we add a length for the token?  That would increase the similarity of this header with the Initial header.

> @@ -1197,6 +1197,64 @@ TLS ClientHello.  The server MAY retain these packets for later decryption in
 anticipation of receiving a ClientHello.
 
 
+## Retry Packet Integrity {#retry-integrity}
+
+Retry packets (see the Retry Packet section of {{QUIC-TRANSPORT}}) carry a
+Retry Integrity Tag that provides two properties: it allows discarding
+packets that have accidentally been corrupted by the network, and it mitigates
+off-path attackers' ability to send valid Retry packets.
+
+The Retry Integrity Tag is a 128-bit field that is computed as the output of
+AEAD_AES_128_GCM used with the following inputs:
+
+- The key is 128 bits all set to zero.

```suggestion
- The secret key, K, is 128 bits all set to zero.
```

> @@ -1197,6 +1197,64 @@ TLS ClientHello.  The server MAY retain these packets for later decryption in
 anticipation of receiving a ClientHello.
 
 
+## Retry Packet Integrity {#retry-integrity}
+
+Retry packets (see the Retry Packet section of {{QUIC-TRANSPORT}}) carry a
+Retry Integrity Tag that provides two properties: it allows discarding
+packets that have accidentally been corrupted by the network, and it mitigates
+off-path attackers' ability to send valid Retry packets.
+
+The Retry Integrity Tag is a 128-bit field that is computed as the output of
+AEAD_AES_128_GCM used with the following inputs:
+
+- The key is 128 bits all set to zero.
+- The nonce is 96 bits all set to zero.

```suggestion
- The nonce, N, is 96 bits all set to zero.
```

and continue.

>  |                        Retry Token (*)                      ...
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                                                               |
++                                                               +
+|                                                               |
++                   Retry Integrity Tag (128)                   +

I wonder if we shouldn't just label this AEAD Ciphertext rather than just a tag.

> @@ -4163,10 +4157,9 @@ A client MUST accept and process at most one Retry packet for each connection
 attempt.  After the client has received and processed an Initial or Retry packet
 from the server, it MUST discard any subsequent Retry packets that it receives.
 
-Clients MUST discard Retry packets that contain an Original Destination
-Connection ID field that does not match the Destination Connection ID from its
-Initial packet.  This prevents an off-path attacker from injecting a Retry
-packet.
+Clients MUST discard Retry packets whose Retry Integrity Tag cannot be
+validated, see the Retry Packet Integrity section of {{QUIC-TLS}}. This
+mitigates an off-path attacker's ability to inject a Retry packet.

I think that we want to add: "and protects against accidental corruption of Retry packets."

-- 
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/3120#pullrequestreview-304931396