-
-
Notifications
You must be signed in to change notification settings - Fork 784
[client] Implement dns routes for Android #3989
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
6343387
to
e277a92
Compare
e277a92
to
bb74e90
Compare
a521410
to
51b9e93
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.
Pull Request Overview
This PR refactors route handlers to use a unified HandlerParams
object, adds a fake-IP allocation system for Android DNS routing (with DNAT support), and renames/updates the server-router setup to use a new SetFirewall
method.
- Introduce
common.HandlerParams
to consolidate handler constructor arguments - Implement
fakeip.Manager
for allocating 240.0.0.0/8 fake IPs and integrate into DNS interceptor - Rename
EnableServerRouter
toSetFirewall
and wire up fake-IP and DNAT logic in the manager and interceptor
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
client/internal/routemanager/common/params.go | Add HandlerParams struct to package common |
client/internal/routemanager/static/route.go | Refactor NewRoute to accept HandlerParams |
client/internal/routemanager/notifier/notifier.go | Remove dynamic-route filter in SetInitialClientRoutes |
client/internal/routemanager/mock.go | Rename EnableServerRouter to SetFirewall (panic stub) |
client/internal/routemanager/manager.go | Rename EnableServerRouter →SetFirewall , introduce fakeIPManager |
client/internal/routemanager/dnsinterceptor/handler.go | Extend interceptor with DNAT/fake-IP mapping logic |
client/internal/routemanager/fakeip/fakeip.go | Implement fakeip.Manager for IPv4-only 240.0.0.0/8 allocations |
client/internal/routemanager/client/client.go | Update HandlerFromRoute to use HandlerParams |
client/internal/routemanager/client/client_test.go | Update test to construct handlers via HandlerParams |
client/android/client.go | Remove skip of dynamic routes in Android Networks method |
Comments suppressed due to low confidence (2)
client/internal/routemanager/notifier/notifier.go:34
- Removing the
IsDynamic()
filter here will allow dynamic routes into the initial client routes list, which may not be intended. Consider reapplying the filter to skip dynamic routes.
for _, r := range clientRoutes {
|
||
// AllocateFakeIP allocates a fake IP for the given real IP | ||
// Returns the fake IP, or existing fake IP if already allocated | ||
func (m *Manager) AllocateFakeIP(realIP netip.Addr) (netip.Addr, error) { |
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.
It's not fully clear yet when and how we associate these addresses. We allocated a fake IP, but it seems like you never released it. How does this work in real life?
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.
Yes, you are right, we never release them. That's something that can be improved, it would also make fake ip management much more complex
This reverts commit 26fc32f.
2fd8d71
to
0f79a89
Compare
|
Describe your changes
This PR implements dns routes on android by
Issue ticket number and link
Stack
Checklist