-
Notifications
You must be signed in to change notification settings - Fork 3
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
Punch me now, but let's keep it safe #19
Comments
Thanks for opening this issue @huitema! You're right that in the current iteration the details of the amplification protection still need a lot of work.
Right. There's a risk of a request forgery attack here, but the risk is a lot smaller than what RFC 9000 section 21.5 describes. The attacker doesn't control more than 20 bytes of the packet (the CID), the size of the packet is set to at least 1200 bytes, all of which (except for the first one) will be indistinguishable from random for a 3rd party.
This will work in certain scenarios, and break in others. We can list this as a possible mitigation strategy, but I don't think we can require it.
Limiting concurrency sounds reasonable, and the protocol provides a mechanism for that: The transport parameter already allows limiting the number of concurrent path probes. We should add some text that this can be used to mitigate the amplification attack as well.
We could also reuse the byte counting logic we're using during the handshake: A node is only allowed to send 3x the number of received bytes to unverified addresses (note the plural here). You wouldn't necessarily need to pad the PUNCH_ME_NOW packets, any bytes sent on the connection would suffice.
This would make people concerned about bandwidth requirements happy. However, it would also mean that you don't verify that the path actually supports QUIC (i.e. UDP datagrams >= 1200 bytes), which seems bad. Maybe a 2-step procedure would help here: First verify connectivity using small probe packets, then confirm that the path supports 1200 bytes. Obviously, this takes one additional roundtrip, which is a tradeoff that might or might not be worthwhile. |
PR #11 introduced the "Punch me now" mechanism, as part of bringing the ICE capability inside QUIC. This is great, but we need to beef up the security analysis. As it stands, the client sends a list of "punch me" addresses to the server, and the server send probing packets to each address. This can cause a lot of amplification: one packet from the client, as many packets as there are addresses to be punched from the server. It could also cause request spoofing attacks, targeting unrelated services with packets that can be partially predicted by the client.
I can think of a few mitigations:
Discuss?
The text was updated successfully, but these errors were encountered: