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.

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

Remove recommendation to not include tokens #4089

Merged
merged 2 commits into from
Sep 11, 2020

Conversation

martinthomson
Copy link
Member

This is another judgment call, but as this wasn't a MUST in the first
place, we weren't really preventing an attack. This just removes the
recommendation to remove NEW_TOKEN tokens from Initial packets to new
server addresses.

It leaves the generic guidance, which is far more nuanced.

I've added some commentary about the effect of withholding tokens on
performance, as it seems like that is worth highlighting here.

All in all, this leans more toward saying that request forgery is not
the responsibility of QUIC deployments.

Closes #4076.

This is another judgment call, but as this wasn't a MUST in the first
place, we weren't really preventing an attack.  This just removes the
recommendation to remove NEW_TOKEN tokens from Initial packets to new
server addresses.

It leaves the generic guidance, which is far more nuanced.

I've added some commentary about the effect of withholding tokens on
performance, as it seems like that is worth highlighting here.

All in all, this leans more toward saying that request forgery is not
the responsibility of QUIC deployments.

Closes #4076.
@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Sep 10, 2020
Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

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

Thanks @martinthomson, greatly appreciated.

Copy link
Contributor

@janaiyengar janaiyengar left a comment

Choose a reason for hiding this comment

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

One nit.

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
Co-authored-by: Jana Iyengar <jri.ietf@gmail.com>
@janaiyengar
Copy link
Contributor

I'm merging this. If anyone thinks this ought to be dealt with differently, please say so on this PR or on the issue, and we can revisit.

@janaiyengar janaiyengar merged commit 2d0650a into master Sep 11, 2020
@janaiyengar janaiyengar deleted the remove-new-token-forgery-req branch September 11, 2020 05:46
@janaiyengar
Copy link
Contributor

Apologies for merging this PR. @LPardue @larseggert : we should still run #4076 through the process as a design issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IP restrictions on Tokens makes them of limited use for non-anycast DNS load balancing
4 participants