Skip to content

[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

Merged
merged 23 commits into from
Jul 4, 2025
Merged

[client] Implement dns routes for Android #3989

merged 23 commits into from
Jul 4, 2025

Conversation

lixmal
Copy link
Collaborator

@lixmal lixmal commented Jun 17, 2025

Describe your changes

This PR implements dns routes on android by

  • Routing a 240.0.0.0/8 block on startup
  • On DNS traffic, assigning IPs from this block and returning those as DNS response
  • The client sends packets to these fake IPs, which are then DNATed to the real IPs
  • On the interface we assign the real IPs as allowed IPs
  • For return traffic we apply the same process in reverse
  • Requires the wildcard feature to be enabled, otherwise we cannot replace the IPs in DNS responses

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

@lixmal lixmal force-pushed the android-dns-routes branch from 6343387 to e277a92 Compare June 17, 2025 00:47
@lixmal lixmal force-pushed the android-dns-routes branch from e277a92 to bb74e90 Compare June 17, 2025 00:48
@lixmal lixmal marked this pull request as ready for review June 17, 2025 13:10
@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 13:10
@lixmal lixmal force-pushed the android-dns-routes branch from a521410 to 51b9e93 Compare June 17, 2025 13:12
Copy link
Contributor

@Copilot Copilot AI left a 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 to SetFirewall 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 EnableServerRouterSetFirewall, 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 {

@lixmal lixmal requested a review from pappz June 18, 2025 15:26

// 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) {
Copy link
Contributor

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?

Copy link
Collaborator Author

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

@pappz
Copy link
Contributor

pappz commented Jun 26, 2025

Screenshot_20250626-144834 (1)
Looks like we do not propagate any name for the network list

@lixmal lixmal requested a review from pappz July 2, 2025 13:54
@lixmal lixmal force-pushed the android-dns-routes branch from 2fd8d71 to 0f79a89 Compare July 2, 2025 15:24
Copy link

sonarqubecloud bot commented Jul 4, 2025

@lixmal lixmal merged commit 77ec32d into main Jul 4, 2025
32 checks passed
@lixmal lixmal deleted the android-dns-routes branch July 4, 2025 14:43
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.

2 participants