Allow packets with impossible CIDs to be ignored rather than reset

quinn-rs / quinn

Async-friendly QUIC implementation in Rust

Apache License 2.0

3.57k stars 364 forks source link

Allow packets with impossible CIDs to be ignored rather than reset #1796

Closed Ralith closed 2 months ago

Ralith commented 3 months ago

Reduces the likelihood of Quinn endpoints responding to non-QUIC packets.

I'm not sure whether this is needed, especially after #1794. However, the cost seems low, it's a more direct solution, and defense in depth is nice.

djc commented 2 months ago

I also wonder if the cost of crypto here actually makes it less attractive than the random generator? @lijunwangs do you think this would help your use case (on top of the other options we've merged recently)?

Ralith commented 2 months ago

That's a good question. A more thoughtful choice of hash function, rather than just using whatever ring has lying around, might help. Maybe even just std's hasher, since we're not trying to be super secure here...

Ralith commented 2 months ago

For clarity, extra cost is paid here in exactly two places:

when generating a CID, which is gated behind the decision to respond to a connection attempt
sending a stateless reset for an unrecognized packet

Ralith commented 2 months ago

Refactored to use a non-cryptographic hash.

lijunwangs commented 2 months ago

I also wonder if the cost of crypto here actually makes it less attractive than the random generator? @lijunwangs do you think this would help your use case (on top of the other options we've merged recently)?

Thanks @djc/ @Ralith -- this will be a great add to strengthen the security. Is the change backward compatible? Can it break existing client code?

Ralith commented 2 months ago

It should be backwards-compatible, yeah.