Fix route manager to support IPv6 server addresses with IPv4 tunnel #330
+239
−37
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kp-samuel-tam
force-pushed
the
fix-ipv6-on-route-manager
branch
4 times, most recently
from
cdd25a8 to
7d1c0c9
Compare
|
Code coverage summary for 2ec4f18: ✅ Region coverage 67% passes |
kp-samuel-tam
force-pushed
the
fix-ipv6-on-route-manager
branch
2 times, most recently
from
4e02561 to
2f55f24
Compare
lightway-client/src/route_manager.rs
Outdated
| #[cfg(windows)] | ||
| let server_route = server_route.with_metric(0); | ||
|
|
||
| // For NoExec mode, store the route but don't actually add it |
Contributor
There was a problem hiding this comment.
Not clear, why do we need to move this.
Please add changes like this in a separate commit with clear description on why it is needed.
Member
Author
There was a problem hiding this comment.
I understood the code wrong and wrote a wrong internal state check for NoExec. It should be fixed now, thanks!
lightway-client/src/route_manager.rs
Outdated
| // and routed networks (host systems with gateways) | ||
| let server_route = Route::new(server_ip, 32).with_if_index(default_interface_index); | ||
| // Use /32 for IPv4, /128 for IPv6 | ||
| let prefix = match server_ip { |
Contributor
There was a problem hiding this comment.
This code is repeated. Better to move this to a separate function.
And use https://doc.rust-lang.org/std/net/struct.Ipv6Addr.html#associatedconstant.BITS for prefix
Member
Author
There was a problem hiding this comment.
Done, for this and checking same family as well.
kp-samuel-tam
force-pushed
the
fix-ipv6-on-route-manager
branch
5 times, most recently
from
117d4bc to
74026a6
Compare
This ensures correct route setup when Lightway client is connected to an IPv6 endpoint. The IPv4 tunnel stays unchanged. Changes: - Use /128 prefix for IPv6 server routes (was incorrectly using /32) - Use /32 prefix for IPv4 server routes (unchanged) - Monitor IPv6 default route changes when server is IPv6 - Monitor IPv4 default route changes when server is IPv4 - Fix LAN mode to only use gateway if address family matches route - Fix find_route to return proper error instead of panicking on None - Add mock IPv6 default route via loopback for test environments without IPv6 - Add host_prefix_len() and same_ip_family() helper functions Tests: - Add IPv6 and IPv4 test constants, update all tests to use them explicitly - Update create_test_setup() to accept server_ip parameter - Add test_ipv6_server_route_manager_creation() - Add test_privileged_ipv6_server_initialize_route_manager() - Add test_privileged_ipv6_server_route_update() - Mock route only looks for ::1 (not 127.0.0.1) when finding loopback The tunnel interface remains IPv4-only (TUN_PEER_IP, TUN_DNS_IP), only the server connection endpoint supports IPv6.
kp-samuel-tam
force-pushed
the
fix-ipv6-on-route-manager
branch
from
74026a6 to
2ec4f18
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This ensures correct route setup when Lightway client is connected to an IPv6 endpoint. The IPv4 tunnel stays unchanged.
Changes:
route add 192.168.0.0/16 via 2001:db8::1(breaks)find_routeto return proper error instead of panicking on None::1host_prefix_len()andsame_ip_family()helper functionsTests:
create_test_setup()to accept server_ip parametertest_ipv6_server_route_manager_creation()test_privileged_ipv6_server_initialize_route_manager()test_privileged_ipv6_server_route_update()The tunnel interface remains IPv4-only (
TUN_PEER_IP,TUN_DNS_IP), only the server connection endpoint supports IPv6.Motivation and Context
Right now when connecting to an IPv6 endpoint, a route like
8321:8bec:666c:0305:9005:9f3e:e29f:3920/32(which is functionally8321:8bec::/32) will be established which is not correct; everything of8321:8bec::/32will leak. Only/128should be correct.How Has This Been Tested?
Wrote tests; if they work it works as intended.
Types of changes
Checklist:
main