-
-
Notifications
You must be signed in to change notification settings - Fork 450
proto: Add option to pad all application data packets to MTU #2274
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
Conversation
Why? |
I updated the PR description with our motivation. |
ae4c27b
to
e3d83ec
Compare
Can't roughly the same analysis still be performed based on timing? |
For our usecase: Yes, both timing and size are relevant features. Removing size makes association of individual packets within a time window a lot harder. And the application layer is free to delay delay packets or even generate fake traffic. Packet size is just the one traffic feature where the transport implementation (e.g. quinn) needs to cooperate. In general (affects our usecase and others): Packet sizes patterns can can be used as client or traffic type fingerprints [1]. [1] https://www.usenix.org/conference/usenixsecurity24/presentation/xue-fingerprinting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, makes sense that this would be pretty effective when there's many users.
8bb5ee1
to
969dc67
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still need to take a careful look at the interaction with GSO batching.
969dc67
to
adb49e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. It's a little non-obvious how segment_size
is guaranteed to always be the MTU when this option is set, but I don't immediately see a way to make that clearer, the change in logic is basically simple, and I am satisfied that it's correct.
Thanks! |
Hmm, not sure why CI is failing in the merge queue: error[E0283]: type annotations needed
--> quinn-proto/src/tests/mod.rs:3256:52
|
3256 | assert_eq!(pair.client.outbound[0].0.size, MTU.into());
| ^^^^
|
= note: cannot satisfy `_: From<u16>`
= note: required for `u16` to implement `Into<_>`
help: try using a fully qualified path to specify the expected types
|
3256 - assert_eq!(pair.client.outbound[0].0.size, MTU.into());
3256 + assert_eq!(pair.client.outbound[0].0.size, <u16 as Into<T>>::into(MTU)); |
Strange... Maybe a different Rust version? I'll change |
adb49e1
to
32c9b11
Compare
The goal is to provide an option to pad all outgoing packets that contain application data to MTU. Our usecase is a VPN, which transmits packets via QUIC datagrams. The benefit is that src <-> relay packets (the tunnel) can't be as easily associated with resulting relay <-> dest traffic if all tunnel packets are padded to MTU.
This generally mitigates some fingerprinting concerns in other usecases as well.