Move stateless reset token to the end #842

martinthomson · 2017-10-09T01:12:49Z

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.

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.

ekr

This seems generally fine. comments below

ekr · 2017-10-09T03:14:42Z

draft-ietf-quic-transport.md

 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..."

ekr · 2017-10-09T03:15:04Z

draft-ietf-quic-transport.md

 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.

ekr · 2017-10-09T03:16:45Z

draft-ietf-quic-transport.md

+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?

Perhaps "all connection state MUST be discarded."

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 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.)

janaiyengar

Couple of comments.

janaiyengar · 2017-10-10T22:52:08Z

draft-ietf-quic-transport.md

+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."

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.

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

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

janaiyengar · 2017-10-10T22:53:36Z

draft-ietf-quic-transport.md

+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


Perhaps "all connection state MUST be discarded."

MikeBishop

LGTM.

MikeBishop · 2017-10-10T23:05:47Z

draft-ietf-quic-transport.md

+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


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.

MikeBishop · 2017-10-10T23:06:54Z

draft-ietf-quic-transport.md

+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


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.

ianswett

A few small suggestions.

ianswett · 2017-10-11T00:24:31Z

draft-ietf-quic-transport.md

-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


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

ianswett · 2017-10-11T00:25:14Z

draft-ietf-quic-transport.md

-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


Is it necessary to mention it's constant time?

ianswett · 2017-10-11T00:25:55Z

draft-ietf-quic-transport.md

+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


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

ianswett · 2017-10-11T00:26:23Z

draft-ietf-quic-transport.md

+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?

Move stateless reset token to the end

83e2881

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.

martinthomson added -transport design An issue that affects the design of the protocol; resolution requires consensus. labels Oct 9, 2017

martinthomson requested a review from ekr October 9, 2017 01:12

ekr approved these changes Oct 9, 2017

View reviewed changes

martinthomson added 2 commits October 9, 2017 15:36

Define the packet type more precisely

53466f0

Define what termination means for stateless reset

b67d605

janaiyengar reviewed Oct 10, 2017

View reviewed changes

MikeBishop approved these changes Oct 10, 2017

View reviewed changes

martinthomson added 2 commits October 11, 2017 10:39

Larger packet number encodings make identification easier

d63e582

Clients can dump state on stateless reset

0484dea

ianswett reviewed Oct 11, 2017

View reviewed changes

Corrections suggested by @ianswett

d167ca1

ianswett approved these changes Oct 11, 2017

View reviewed changes

janaiyengar approved these changes Oct 12, 2017

View reviewed changes

janaiyengar merged commit 82eefde into master Oct 12, 2017

martinthomson deleted the move-stateless-reset branch October 12, 2017 23:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move stateless reset token to the end #842

Move stateless reset token to the end #842

martinthomson commented Oct 9, 2017

ekr left a comment

ekr Oct 9, 2017

ekr Oct 9, 2017

ekr Oct 9, 2017

janaiyengar Oct 10, 2017

MikeBishop Oct 10, 2017

martinthomson Oct 10, 2017

janaiyengar left a comment

janaiyengar Oct 10, 2017

MikeBishop Oct 10, 2017

MikeBishop Oct 10, 2017

martinthomson Oct 10, 2017

janaiyengar Oct 10, 2017

MikeBishop left a comment

MikeBishop Oct 10, 2017

MikeBishop Oct 10, 2017

ianswett left a comment

ianswett Oct 11, 2017

ianswett Oct 11, 2017

ianswett Oct 11, 2017

ianswett Oct 11, 2017

Move stateless reset token to the end #842

Move stateless reset token to the end #842

Conversation

martinthomson commented Oct 9, 2017

ekr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janaiyengar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeBishop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment