Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Immediately close with INVALID_TOKEN #3107

Merged
merged 17 commits into from Dec 3, 2019
Merged

Immediately close with INVALID_TOKEN #3107

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1a563f2
Immediately close with INVALID_TOKEN
ianswett Oct 17, 2019
24d4b63
Update draft-ietf-quic-transport.md
ianswett Oct 17, 2019
8c46914
Update draft-ietf-quic-transport.md
ianswett Oct 17, 2019
6cf5de8
Kazuho and MTs comments
ianswett Oct 22, 2019
b43b5bf
Define the error
ianswett Oct 22, 2019
f827ca3
whitespace
ianswett Oct 22, 2019
9617e1d
Kazuho's suggestion
ianswett Nov 1, 2019
97ed821
Update draft-ietf-quic-transport.md
ianswett Nov 1, 2019
85bac67
Update draft-ietf-quic-transport.md
ianswett Nov 2, 2019
70aef67
Update draft-ietf-quic-transport.md
ianswett Nov 2, 2019
887c909
MAY close the connection without creating state
ianswett Nov 2, 2019
c36760d
Jana's suggestion
ianswett Nov 2, 2019
b16c4cf
Update draft-ietf-quic-transport.md
ianswett Nov 2, 2019
3ab9f77
Update draft-ietf-quic-transport.md
ianswett Nov 2, 2019
474291a
Martin's suggestion
ianswett Nov 12, 2019
4cf3ff9
Update draft-ietf-quic-transport.md
ianswett Nov 12, 2019
1396f07
Update draft-ietf-quic-transport.md
ianswett Nov 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions draft-ietf-quic-transport.md
View file
Open in desktop
Expand Up @@ -1641,12 +1641,12 @@ of connection establishment. By giving the client a different connection ID to
use, a server can cause the connection to be routed to a server instance with
more resources available for new connections.

If a server receives a client Initial with an invalid Retry token,
it knows the client will not accept another Retry token. It can either
proceed with the handshake without verifying the token or immediately close
({{immediate-close}}) the connection with an connection error of
INVALID_TOKEN to cause the handshake to fail quickly instead of waiting
for the client to timeout.
If a server receives a client Initial that can be unprotected but contains an
invalid Retry token, it knows the client will not accept another Retry token.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only applies if the token is a Retry token (and not a NEW_TOKEN token).
When the client uses a NEW_TOKEN token, it would exchange this token for the Retry token when it receives a Retry packet, wouldn't it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the text explicitly says "Retry token", so I don't see how this is a problem.

It can either proceed with the handshake without verifying the token or
immediately close ({{immediate-close}}) the connection with an connection
error of INVALID_TOKEN to cause the handshake to fail quickly instead of
ianswett marked this conversation as resolved.
Show resolved Hide resolved
waiting for the client to timeout.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggests 2 options at the server, when there are 3: it can simply drop the packet and let the client time out. You may want to call out all three options.

Suggestion: "If a server receives a client Initial that can be unprotected but contains an invalid Retry token, it knows the client will not accept another Retry token. The server can drop such a packet and allow the client to time out to detect loss of this connection, but that is a significant latency penalty to the client. If possible, the server SHOULD either immediately close the connection with an INVALID_TOKEN error or proceed with the handshake without verifying the token."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that listing all the three options would be an improvement.

At the same time, I have a concern regarding the suggestion of doing either an immediate close or proceeding the handshake. Those two options both require the server to retain state that cannot be delayed by providing a Retry to token to the client.

I think it we might want to downgrade the SHOULD to MAY, or, suggest that a server can send a CONNECTION_CLOSE frame (with INVALID_TOKEN) without retaining state.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to detect an invalid token without per-connection state, right?

That said, I realized that I don't think we should recommend that a server that might be getting DoSsed SHOULD send out another packet in response. How about "A server that is able to maintain enough state MAY either ..."

Copy link
Member

@kazuho kazuho Nov 2, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By saying state, I mean the closing and the draining state that gets built and retained once you invoke {immediate-close}. We should not recommend such behavior as it would create a DoS vector. But if it is not going to be a SHOULD but instead “can” or MAY, I think we can be less precise and therefor the text we have now is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a sentence about not creating any state in this case. PTAL.


A flow showing the use of a Retry packet is shown in {{fig-retry}}.

Expand Down