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

Document request forgery by version negotiation #4339

Merged
merged 2 commits into from Dec 8, 2020
Merged

Conversation

martinthomson
Copy link
Member

It's pretty bad.

Personally, I'm still comfortable with it. Server deployments are far
less likely to made without due consideration to the environment into
which they find themselves.

Closes #4258.

It's pretty bad.

Personally, I'm still comfortable with it.  Server deployments are far
less likely to made without due consideration to the environment into
which they find themselves.

Closes #4258.
@marten-seemann
Copy link
Contributor

Server deployments are far less likely to made without due consideration to the environment into which they find themselves.

... unless you start considering p2p deployments.

@martinthomson
Copy link
Member Author

Fair point; though I would suggest that p2p doesn't need version negotiation. Once you have to do NAT traversal, you can also ensure that you don't have to ever send Version Negotiation.

@marten-seemann
Copy link
Contributor

I don't think that works in the general case. A node won't know if it's behind a NAT when it's started (it will learn after a while of interacting with other nodes). And even if you solve that problem, you'll need to do some extra work to communicate QUIC versions during hole punching.

Copy link
Contributor

@huitema huitema left a comment

Choose a reason for hiding this comment

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

I agree that documenting the issue is the best we can do at this stage of the process. Short of ditching version negotiation altogether, but that would mean a long delay in delivering QUIC V1, and I don't think we want to go there.

No specific countermeasures are provided for this attack, though generic
protections {{forgery-generic}} could apply. In this case, ingress filtering
{{?BCP38}} is also effective.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have been thinking about potential defenses, and I could not find something obvious there. Rate limiting does not help much, since this is not a volumetric attack. I think this text is about the best we can do.

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.

Ugh. Yeah, this isn't great. I agree that we can lean on servers... but documenting this is super important. This is a pretty big hole... but I think it's ok to leave it as it is in this PR.

Let's leave this open for a while longer to ensure that it gets more visibility, and to see if anyone can come up with any other ideas.

@martinthomson martinthomson added the editorial An issue that does not affect the design of the protocol; does not require consensus. label Dec 8, 2020
@janaiyengar janaiyengar merged commit edf1cbe into master Dec 8, 2020
@janaiyengar janaiyengar deleted the vn-forgery branch December 8, 2020 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request Forgery Attacks through Version Negotiation
5 participants