Subject: Re: [quicwg/base-drafts] Perform stateless reset token comparisons in constant time (#2993)

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Oct 2019 16:50:45 -0700

@martinthomson pushed 1 commit.

a5671a8421f243b0525ebcc5c13eba8c5bd3e615  Fix stateless reset ordering; use datagrams

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/8908632504a0db7467ea73f05ceb99b0537ee72c..a5671a8421f243b0525ebcc5c13eba8c5bd3e615

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Oct 2019 16:53:36 -0700

2115f59b419d13fea3bca7e53981dc4b6dfde6d1  Reword again

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/a5671a8421f243b0525ebcc5c13eba8c5bd3e615..2115f59b419d13fea3bca7e53981dc4b6dfde6d1

---

From: Mike Bishop <[email-masked]>
Date: Tue, 15 Oct 2019 10:43:30 -0700

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/2993#pullrequestreview-302083889

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 15 Oct 2019 10:47:32 -0700

janaiyengar approved this pull request.

take it or leave it

> -which the datagram is received.  This includes Stateless Reset Tokens from
-NEW_CONNECTION_ID frames and the server's transport parameters.  An endpoint
-MUST NOT check for any Stateless Reset Tokens associated with connection IDs it
-has not used or for connection IDs that have been retired.
-
-If the last 16 bytes of the packet values are identical to a Stateless Reset
+An endpoint detects a potential stateless reset using the trailing 16 bytes of
+the UDP datagram.  The endpoint compare the last 16 bytes of the datagram with
+all Stateless Reset Tokens that are associated with connection IDs that the
+endpoint recently used to send packets from the IP address and port on which the
+datagram is received.
+
+This comparison can be performed for every inbound datagram.  Endpoints MAY skip
+this check if packets from a datagram are successfully processed.  However, the
+comparison MUST be performed when the first packet in an incoming datagram
+either cannot be associated with a connection, cannot be decrypted, or is

```suggestion
either cannot be associated with a connection, cannot be decrypted, or carries
```

> -NEW_CONNECTION_ID frames and the server's transport parameters.  An endpoint
-MUST NOT check for any Stateless Reset Tokens associated with connection IDs it
-has not used or for connection IDs that have been retired.
-
-If the last 16 bytes of the packet values are identical to a Stateless Reset
+An endpoint detects a potential stateless reset using the trailing 16 bytes of
+the UDP datagram.  The endpoint compare the last 16 bytes of the datagram with
+all Stateless Reset Tokens that are associated with connection IDs that the
+endpoint recently used to send packets from the IP address and port on which the
+datagram is received.
+
+This comparison can be performed for every inbound datagram.  Endpoints MAY skip
+this check if packets from a datagram are successfully processed.  However, the
+comparison MUST be performed when the first packet in an incoming datagram
+either cannot be associated with a connection, cannot be decrypted, or is
+identified as a duplicate.

```suggestion
a duplicate packet number.
```

-- 
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/2993#pullrequestreview-302085583

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 15 Oct 2019 10:48:53 -0700

janaiyengar commented on this pull request.

> @@ -2529,20 +2529,35 @@ the packet other than the last 16 bytes for carrying data.
 
 ### Detecting a Stateless Reset
 
-An endpoint detects a potential stateless reset when an incoming packet either
-cannot be associated with a connection, cannot be decrypted, or is marked as a
-duplicate packet.  The endpoint MUST then compare the last 16 bytes of the
-packet with all Stateless Reset Tokens that are associated with connection IDs
-that the endpoint recently used to send packets from the IP address and port on
-which the datagram is received.  This includes Stateless Reset Tokens from
-NEW_CONNECTION_ID frames and the server's transport parameters.  An endpoint
-MUST NOT check for any Stateless Reset Tokens associated with connection IDs it
-has not used or for connection IDs that have been retired.
-
-If the last 16 bytes of the packet values are identical to a Stateless Reset
+An endpoint detects a potential stateless reset using the trailing 16 bytes of
+the UDP datagram.  The endpoint compare the last 16 bytes of the datagram with

```suggestion
the UDP datagram.  The endpoint compares the last 16 bytes of the datagram with
```

-- 
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/2993#pullrequestreview-302086957

---

From: Mike Bishop <[email-masked]>
Date: Tue, 15 Oct 2019 10:49:10 -0700

MikeBishop commented on this pull request.

 

```suggestion

-- 
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/2993#pullrequestreview-302086959

---

From: Martin Thomson <[email-masked]>
Date: Tue, 15 Oct 2019 10:49:19 -0700

58c17b8410fa58b82b19276a4bbad79e18c3bc15  duplicates are just duplicate packet numbers

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/2115f59b419d13fea3bca7e53981dc4b6dfde6d1..58c17b8410fa58b82b19276a4bbad79e18c3bc15

---

From: Martin Thomson <[email-masked]>
Date: Tue, 15 Oct 2019 10:49:49 -0700

7c8bbea1d07e170eb5f7e152b3e073740e2f576c  compares

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/58c17b8410fa58b82b19276a4bbad79e18c3bc15..7c8bbea1d07e170eb5f7e152b3e073740e2f576c

---

From: ianswett <[email-masked]>
Date: Tue, 15 Oct 2019 10:52:24 -0700

ianswett approved this pull request.

Some small suggestions, but LG

 

```suggestion

```suggestion
Endpoints MAY skip
```

-- 
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/2993#pullrequestreview-302084652

---

From: Martin Thomson <[email-masked]>
Date: Tue, 15 Oct 2019 10:55:52 -0700

201b0f66b50c0b2f341df3157392ee671ced3ff5  any packet

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/7c8bbea1d07e170eb5f7e152b3e073740e2f576c..201b0f66b50c0b2f341df3157392ee671ced3ff5

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 15 Oct 2019 11:00:50 -0700

+the UDP datagram.  The endpoint compares the last 16 bytes of the datagram with

To detect stateless resets, an endpoint remembers all Stateless Reset Tokens along with the associated connection IDs and the corresponding IP address and port that it recently used to send packets. The endpoint detects a received packet as a stateless reset by comparing the last 16 bytes of a received datagram with Stateless Reset Tokens that are associated with the IP address and port on which the datagram was received.

-- 
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/2993#pullrequestreview-302093897

---

From: Martin Thomson <[email-masked]>
Date: Tue, 15 Oct 2019 11:11:19 -0700

439b719d8198bf3db4d51b11d32fca590ef65e2b  Apply Jana's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/201b0f66b50c0b2f341df3157392ee671ced3ff5..439b719d8198bf3db4d51b11d32fca590ef65e2b

---

From: Martin Thomson <[email-masked]>
Date: Tue, 15 Oct 2019 16:37:49 -0700

b5dba271d4bd271ec14ab50840271e984be08db9  Drop an unnecessary sentence

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/439b719d8198bf3db4d51b11d32fca590ef65e2b..b5dba271d4bd271ec14ab50840271e984be08db9

---

From: ekr <[email-masked]>
Date: Wed, 16 Oct 2019 14:19:24 -0700

ekr commented on this pull request.

+An endpoint detects a potential stateless reset using the trailing 16 bytes of
+the UDP datagram.  An endpoint remembers all Stateless Reset Tokens associated
+with the connection IDs and remote addresses for datagrams it has recently sent.
+This includes Stateless Reset Tokens from NEW_CONNECTION_ID frames and the
+server's transport parameters but excludes Stateless Reset Tokens associated
+with connection IDs that are either unused or retired.  The endpoint identifies
+a received datagram as a stateless reset by comparing the last 16 bytes of the
+datagram with all Stateless Reset Tokens associated with the remote address on
+which the datagram was received.
+
+This comparison can be performed for every inbound datagram.  Endpoints MAY skip
+this check if any packet from a datagram is successfully processed.  However,
+the comparison MUST be performed when the first packet in an incoming datagram
+either cannot be associated with a connection, cannot be decrypted, or carries
+a duplicate packet number.

I don't think I understand the rationale for this duplicate packet number check. If you have a valid packet, it seems like one ought to assume that it's not also a stateless reset or coalesced with one.

-- 
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/2993#pullrequestreview-302883710

---

From: Martin Thomson <[email-masked]>
Date: Wed, 16 Oct 2019 14:22:29 -0700

963d9161da75d43f9500818c8f012a84100d38ac  Remove duplicate requirement as being bad/nonsensical

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2993/files/59a8238dd289a5c7e54a40ca9a791227ae69284d..963d9161da75d43f9500818c8f012a84100d38ac

---

From: Martin Thomson <[email-masked]>
Date: Wed, 16 Oct 2019 14:26:08 -0700

martinthomson commented on this pull request.

Yeah, I just worked through it and this is negatively good, because it implies that you have an early exit from the packet protection removal code.  That wouldn't be constant time in any way.

-- 
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/2993#discussion_r335716617

---

From: Martin Thomson <[email-masked]>
Date: Wed, 30 Oct 2019 22:29:36 -0700

Merged #2993 into master.

-- 
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/2993#event-2758501023