Subject: [quicwg/base-drafts] Move stateless reset token to the end (#842)

From: Martin Thomson <[email-masked]>
Date: Mon, 09 Oct 2017 01:12:52 +0000 (UTC)

This makes the location of the token fixed with respect to the end of
packets, which makes it easier to find.  It also expands the conditions
for checking the token, which now include cases where packets appear
to be duplicated.

Closes #820.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/842

-- Commit Summary --

  * Move stateless reset token to the end

-- File Changes --

    M draft-ietf-quic-transport.md (35)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/842.patch
https://github.com/quicwg/base-drafts/pull/842.diff

-- 
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/842

---

From: ekr <[email-masked]>
Date: Mon, 09 Oct 2017 03:17:09 +0000 (UTC)

ekr approved this pull request.

This seems generally fine. comments below

>  A server copies the connection ID field from the packet that triggers the
 stateless reset.  A server omits the connection ID if explicitly configured to
 do so, or if the client packet did not include a connection ID.
 
+The Packet Number field is set to a randomized value.  This packet SHOULD use

Nit: I would say "The server SHOULD use..."

>  A server copies the connection ID field from the packet that triggers the
 stateless reset.  A server omits the connection ID if explicitly configured to
 do so, or if the client packet did not include a connection ID.
 
+The Packet Number field is set to a randomized value.  This packet SHOULD use
+the short header form with the shortest possible packet number encoding.  This

I think I would phrase this as "the packet type should be X, which represents the shortest packet encoding", because you aren't allowed to use long format under any conditions.

> @@ -1595,11 +1600,11 @@ CONNECTION_CLOSE or APPLICATION_CLOSE frame if it has sufficient state to do so.
 #### Detecting a Stateless Reset
 
 A client detects a potential stateless reset when a packet with a short header
-cannot be decrypted.  The client then performs a constant-time comparison of the
-16 octets that follow the Connection ID with the Stateless Reset Token provided
-by the server in its transport parameters.  If this comparison is successful,
-the connection MUST be terminated immediately.  Otherwise, the packet can be
-discarded.
+either cannot be decrypted or is marked as a potential duplicate.  The client
+then performs a constant-time comparison of the last 16 octets of the packet
+with the Stateless Reset Token provided by the server in its transport
+parameters.  If this comparison is successful, the connection MUST be terminated

Can you say what "terminated" means? Is it the same as having received CONNECTION_CLOSE?

-- 
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/842#pullrequestreview-67883034

---

From: Martin Thomson <[email-masked]>
Date: Sun, 08 Oct 2017 21:37:19 -0700

@martinthomson pushed 2 commits.

53466f0  Define the packet type more precisely
b67d605  Define what termination means for stateless reset

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/842/files/83e28811e050c77c84b691fc6a1ce3685e9b887c..b67d605b9a76419d01aed2992173759e78f3baa3

---

From: janaiyengar <[email-masked]>
Date: Tue, 10 Oct 2017 22:53:53 +0000 (UTC)

janaiyengar commented on this pull request.

Couple of comments.

+The Packet Number field is set to a randomized value.  The server SHOULD send a
+packet with a short header and a type of 0x01.  This produces the shortest
+possible packet number encoding, which minimizes the perceived gap between the
+last packet that the server sent and this packet.  A server MAY use a different
+short header type, indicating a different packet number length, but this allows
+for the message to be identified as a stateless reset more easily using

nit: "this allows for" - I think you mean to say that using a 1-byte packet number makes life easier, but "this" is ambiguous here. Maybe something like: "A server MAY use a different short header type, indicating a different packet number length, but using 0x01 allows for the message to be identified as a stateless reset more easily using heuristics."

Perhaps "all connection state MUST be discarded."

-- 
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/842#pullrequestreview-68450033

---

From: Mike Bishop <[email-masked]>
Date: Tue, 10 Oct 2017 23:07:08 +0000 (UTC)

MikeBishop approved this pull request.

LGTM.

It's not the identification we care about, but the fact that an observer could identify the SR by seeing a packet number jump.  If the number is only eight bits long, then it's harder for an observer to infer a large jump.

Any of the above seem fine for now; we'll want to tie this in with @janaiyengar's Unified Theory of Connection Closure when that lands.

-- 
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/842#pullrequestreview-68452414

---

From: Mike Bishop <[email-masked]>
Date: Tue, 10 Oct 2017 23:07:48 +0000 (UTC)

MikeBishop commented on this pull request.

Or rather, we're not trying to make identification easy for the client, we're trying to make it hard for everyone else.

-- 
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/842#discussion_r143876446

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Oct 2017 23:37:34 +0000 (UTC)

martinthomson commented on this pull request.

Yeah, but I take the comment- I will revise along the lines suggested.

-- 
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/842#discussion_r143880367

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Oct 2017 16:39:28 -0700

You can't discard state if you are going to enter the draining period.  But we don't need draining, we just need to stop.  (Any subsequent stateless resets we get will get discarded according to our new packet handling rules.)

-- 
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/842#discussion_r143880589

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Oct 2017 23:40:33 +0000 (UTC)

d63e582  Larger packet number encodings make identification easier
0484dea  Clients can dump state on stateless reset

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/842/files/b67d605b9a76419d01aed2992173759e78f3baa3..0484dead76efc4300749176900557c03a8dfd32a

---

From: ianswett <[email-masked]>
Date: Wed, 11 Oct 2017 00:26:53 +0000 (UTC)

ianswett commented on this pull request.

A few small suggestions.

> @@ -1595,11 +1601,12 @@ CONNECTION_CLOSE or APPLICATION_CLOSE frame if it has sufficient state to do so.

nit: I'd go with "is marked as a duplicate packet."

> @@ -1595,11 +1601,12 @@ CONNECTION_CLOSE or APPLICATION_CLOSE frame if it has sufficient state to do so.

Is it necessary to mention it's constant time?

> @@ -1595,11 +1601,12 @@ CONNECTION_CLOSE or APPLICATION_CLOSE frame if it has sufficient state to do so.
 
+parameters.  If this comparison is successful, the client MUST discard all

Instead of successful, how about "If they are identical" ?

> @@ -1595,11 +1601,12 @@ CONNECTION_CLOSE or APPLICATION_CLOSE frame if it has sufficient state to do so.
 
 A client detects a potential stateless reset when a packet with a short header
-cannot be decrypted.  The client then performs a constant-time comparison of the
-16 octets that follow the Connection ID with the Stateless Reset Token provided
-by the server in its transport parameters.  If this comparison is successful,
-the connection MUST be terminated immediately.  Otherwise, the packet can be
-discarded.
+either cannot be decrypted or is marked as a potential duplicate.  The client
+then performs a constant-time comparison of the last 16 octets of the packet
+with the Stateless Reset Token provided by the server in its transport
+parameters.  If this comparison is successful, the client MUST discard all
+connection state and not send any further packets on this connection. If the
+comparison is unsuccessful, the packet can be discarded.

is unsuccessful => fails?

-- 
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/842#pullrequestreview-68464290

---

From: Martin Thomson <[email-masked]>
Date: Wed, 11 Oct 2017 02:43:50 +0000 (UTC)

@martinthomson pushed 1 commit.

d167ca1  Corrections suggested by @ianswett

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/842/files/0484dead76efc4300749176900557c03a8dfd32a..d167ca1aec7f4a4a51e7b9ba9af22a7e583f1978

---

From: ianswett <[email-masked]>
Date: Wed, 11 Oct 2017 13:12:57 +0000 (UTC)

ianswett 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/842#pullrequestreview-68599761

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Oct 2017 21:56:20 +0000 (UTC)

janaiyengar 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/842#pullrequestreview-69076465

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Oct 2017 21:56:33 +0000 (UTC)

Merged #842.

-- 
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/842#event-1291215595