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

[WIP] ffh.exitnode: add MTU fix for QUIC #107

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

CodeFetch
Copy link
Contributor

@CodeFetch CodeFetch commented Jan 17, 2021

This commit creates a dummy interface with the "bottleneck" MTU
among our VPN path (currently batadv - see issue #80).
Furthermore it creates an iptables DNAT rule which changes the
destination IP address of incoming QUIC (UDP 443) packets which
exceed the bottleneck MTU to a special IPv4 continuity address
which is part of the subnet of the dummy interface.
When an oversized QUIC packet arrives, it will thus be routed
to the dummy interface which in turn generates an ICMP destination
unreachable (fragmentation needed) packet as the packet does
not fit the MTU of the dummy interface.
The QUIC servers will react to the ICMP packet by changing the
PMTU of their UDP sockets according to the maximum MTU advertised
in the ICMP message, which is the dummy interface's MTU.

It has been tested on sn10. We now need to test if QUIC still works properly. There seems to be still an issue.

@CodeFetch CodeFetch force-pushed the quic_fix branch 2 times, most recently from 67d8792 to 393e2a5 Compare January 17, 2021 18:04
@CodeFetch CodeFetch changed the title [WIP] ffh.exitnode: add MTU fix for QUIC ffh.exitnode: add MTU fix for QUIC Jan 17, 2021
@CodeFetch CodeFetch changed the title ffh.exitnode: add MTU fix for QUIC [WIP] ffh.exitnode: add MTU fix for QUIC Jan 17, 2021
roles/ffh.exitnode/files/mtudummy.network Show resolved Hide resolved
@@ -13,6 +13,12 @@

domain (ip) {
table nat {
chain PREROUTING {
Copy link
Member

Choose a reason for hiding this comment

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

Can we (start) seperating the different functional blocks within ferm's 50-exitnode.conf? Adding more functions to this single file makes is hard to read over time.

This commit creates a dummy interface with the "bottleneck" MTU
among our VPN path (currently batadv - see issue #80).
Furthermore it creates an iptables DNAT rule which changes the
destination IP address of incoming QUIC (UDP 443) packets which
exceed the bottleneck MTU to a special IPv4 continuity address
which is part of the subnet of the dummy interface.
When an oversized QUIC packet arrives, it will thus be routed
to the dummy interface which in turn generates an ICMP destination
unreachable (fragmentation needed) packet as the packet does
not fit the MTU of the dummy interface.
The QUIC servers will react to the ICMP packet by changing the
PMTU of their UDP sockets according to the maximum MTU advertised
in the ICMP message, which is the dummy interface's MTU.
@AiyionPrime
Copy link
Member

Remember to get rid of all the commented out sections, or mark the PR as draft;
so that they don't accidentally become master code ;)

@CodeFetch
Copy link
Contributor Author

@AiyionPrime I'll do... "WIP" means Work-in-progress. I think it's the same as a draft? I wonder why Github does not mark it as a draft automatically.

@CodeFetch CodeFetch marked this pull request as draft January 19, 2021 13:30
@CodeFetch
Copy link
Contributor Author

CodeFetch commented Jan 19, 2021

Google does not respect our ICMP messages. I've tested how the browser behaves when QUIC traffic is filtered. It seems Chrome tests for QUIC support in the background. So blocking QUIC traffic that is too big is not an issue for the UX. Still I've send an email to Google and asked them if this is intended. They promote QUIC so I think they should behave according to the standard and do proper PMTU discovery... Instead they seem to have implemented their own algorithm which does not work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants