From e6c5cbdc6e8418ffd801eef5529d4ed8391cc09e Mon Sep 17 00:00:00 2001 From: PC Date: Sun, 27 Sep 2026 18:42:18 +0300 Subject: [PATCH 1/5] fix(relay): drop phone->Mac loops and adopt MAC changes only after threshold Frames returning to the Mac carrying our own client MAC (02:00:...:01) are a phone-side loop (USB + hotspot both attached); forwarding them into utun poisoned the path and mis-set the phone MAC. Drop them, counting drops. A new phone MAC is adopted only after macAdoptThreshold (8) consecutive sightings from the same source, so single stray frames cannot mis-address every subsequent frame. --- internal/daemon/relay.go | 112 +++++++++++++++++++++--- internal/daemon/relay_loopguard_test.go | 105 ++++++++++++++++++++++ 2 files changed, 206 insertions(+), 11 deletions(-) create mode 100644 internal/daemon/relay_loopguard_test.go diff --git a/internal/daemon/relay.go b/internal/daemon/relay.go index af78fe6..3a76ed3 100644 --- a/internal/daemon/relay.go +++ b/internal/daemon/relay.go @@ -1,6 +1,7 @@ package daemon import ( + "bytes" "context" "encoding/binary" "encoding/hex" @@ -17,6 +18,19 @@ import ( "github.com/rs/zerolog/log" ) +// clientMAC is the source MAC we stamp on every frame we send to the phone. +// The phone is expected to send return traffic addressed to it. It is a +// constant (not learned) so we can recognise and drop anything that comes back +// carrying it — see the loop guard in the phone->Mac direction. +var clientMAC = []byte{0x02, 0x00, 0x00, 0x00, 0x00, 0x01} + +// macCandidate tracks a possible new phone MAC. A single packet is not enough +// evidence: a phone attached both by USB and Wi-Fi hotspot sends frames from +// several interfaces, and adopting the wrong one mis-addresses every frame we +// send (see loop guard). Require macAdoptThreshold consecutive sightings from +// the same address before switching. +const macAdoptThreshold = 8 + // Relay handles the packet shuttle between USB Bulk and Tunnel interface. type Relay struct { dev *usb.Device @@ -34,6 +48,12 @@ type Relay struct { mu sync.Mutex clientIP net.IP + // Candidate for a phone MAC change: consecutive sightings of candMAC. + candMAC []byte + candCount int + loopDrops uint64 + macChanges uint64 + // Traffic stats sentBytes uint64 recvBytes uint64 @@ -106,7 +126,7 @@ func (r *Relay) Start() error { eth := make([]byte, 14+len(rawIP)) copy(eth[0:6], r.phoneMAC) r.mu.Unlock() - copy(eth[6:12], []byte{0x02, 0x00, 0x00, 0x00, 0x00, 0x01}) + copy(eth[6:12], clientMAC) binary.BigEndian.PutUint16(eth[12:14], 0x0800) copy(eth[14:], rawIP) @@ -169,6 +189,27 @@ func (r *Relay) Start() error { if msgType == rndis.MsgPacket && msgLen > 44 { ethPkt, err := rndis.DecapsulatePacket(msg[:msgLen]) if err == nil && len(ethPkt) > 14 { + // Loop guard: drop anything carrying our own source + // MAC. Such a frame is our own outbound traffic that + // the phone routed back to us (it happens when the + // phone is also serving a Wi-Fi hotspot and the + // destination MAC was wrong). Injecting it again + // would re-enter the tunnel and amplify into a + // runaway loop that burned metered data at ~30 MB/s. + if r.isLoopFrame(ethPkt) { + r.mu.Lock() + r.loopDrops++ + drops := r.loopDrops + r.mu.Unlock() + if drops <= 5 || drops%1000 == 0 { + log.Warn().Str("component", "relay"). + Int("dropped", int(drops)). + Msg("Dropped reflected frame with our own source MAC (loop guard)") + } + offset += msgLen + continue + } + // Auto-detect phone's real MAC from received traffic. // Samsung devices randomize the active MAC on the tethering // interface, so the RNDIS-queried address may not match. @@ -249,21 +290,70 @@ func (r *Relay) Start() error { } } -// autoDetectMAC updates phoneMAC if the received source MAC differs (Samsung MAC randomization workaround). +// autoDetectMAC considers a new phone MAC learned from received traffic +// (Samsung devices randomize the active MAC on the tethering interface, so the +// RNDIS-queried address may not match). +// +// Two safety rules, both learned the hard way on a phone attached as BOTH a USB +// tether and a Wi-Fi hotspot: +// +// 1. Never adopt our own clientMAC. A frame echoing back with our source MAC +// is our own traffic being reflected, and adopting it would make us address +// every subsequent frame to ourselves — a self-sustaining loop that moved +// 1.4 GB inbound against 79 KB outbound on a metered connection. +// 2. Require macAdoptThreshold *consecutive* sightings of the same address. +// One packet is not evidence: a dual-attached phone emits frames from +// several interfaces, and a single stray frame must not re-address the +// whole relay. func (r *Relay) autoDetectMAC(srcMAC []byte) { r.mu.Lock() defer r.mu.Unlock() - for i := 0; i < 6; i++ { - if r.phoneMAC[i] != srcMAC[i] { - log.Warn().Str("component", "relay"). - Str("old_mac", hex.EncodeToString(r.phoneMAC)). - Str("real_mac", hex.EncodeToString(srcMAC)). - Msg("Phone MAC mismatch detected — updating to real MAC") - copy(r.phoneMAC, srcMAC) - return - } + // Rule 1: never learn our own address. + if bytes.Equal(srcMAC, clientMAC) { + return + } + + // Already the current MAC: nothing to do, and it cancels any candidate. + if bytes.Equal(srcMAC, r.phoneMAC) { + r.candMAC = nil + r.candCount = 0 + return + } + + // Track consecutive sightings of a candidate. + if !bytes.Equal(srcMAC, r.candMAC) { + r.candMAC = append([]byte(nil), srcMAC...) + r.candCount = 1 + return + } + + r.candCount++ + if r.candCount < macAdoptThreshold { + return + } + + old := hex.EncodeToString(r.phoneMAC) + new := hex.EncodeToString(srcMAC) + r.macChanges++ + copy(r.phoneMAC, srcMAC) + r.candMAC = nil + r.candCount = 0 + + log.Warn().Str("component", "relay"). + Str("old_mac", old). + Str("new_mac", new). + Int("confirmed_packets", macAdoptThreshold). + Msg("Phone MAC change confirmed — switching destination MAC") +} + +// isLoopFrame reports whether an inbound frame is our own traffic being +// reflected back at us, which must never be injected into the tunnel. +func (r *Relay) isLoopFrame(ethPkt []byte) bool { + if len(ethPkt) < 14 { + return false } + return bytes.Equal(ethPkt[6:12], clientMAC) } // tryExtractDHCP checks if a raw IP packet is a DHCP message and handles Offer/ACK. diff --git a/internal/daemon/relay_loopguard_test.go b/internal/daemon/relay_loopguard_test.go new file mode 100644 index 0000000..ca6385f --- /dev/null +++ b/internal/daemon/relay_loopguard_test.go @@ -0,0 +1,105 @@ +package daemon + +import ( + "bytes" + "testing" +) + +func newTestRelay(phoneMAC []byte) *Relay { + return &Relay{phoneMAC: append([]byte(nil), phoneMAC...)} +} + +var ( + usbMAC = []byte{0x3a, 0x58, 0x68, 0x37, 0xd3, 0xe8} + wifiMAC = []byte{0xda, 0x7c, 0x6a, 0x44, 0xbe, 0x37} +) + +func TestAutoDetectMACIgnoresOwnClientMAC(t *testing.T) { + r := newTestRelay(usbMAC) + + // A reflected frame carries our own source MAC. Adopting it would make us + // address every subsequent frame to ourselves. + for i := 0; i < 50; i++ { + r.autoDetectMAC(clientMAC) + } + + if !bytes.Equal(r.phoneMAC, usbMAC) { + t.Fatalf("phoneMAC was corrupted to our own client MAC: %x", r.phoneMAC) + } + if r.macChanges != 0 { + t.Fatalf("macChanges = %d, want 0", r.macChanges) + } +} + +func TestAutoDetectMACRequiresConsecutiveSightings(t *testing.T) { + r := newTestRelay(usbMAC) + + // A single stray frame from another interface must not re-address the relay. + r.autoDetectMAC(wifiMAC) + if !bytes.Equal(r.phoneMAC, usbMAC) { + t.Fatalf("single stray sighting changed the MAC to %x", r.phoneMAC) + } + + // A different candidate resets the streak rather than accumulating. On a + // fresh relay, feed threshold-1 sightings of A, one of B, then threshold-1 + // more of A. Only threshold-1 *consecutive* A's are ever seen, so no switch. + r2 := newTestRelay(usbMAC) + other := []byte{0x02, 0xaa, 0xbb, 0xcc, 0xdd, 0xee} + for i := 0; i < macAdoptThreshold-1; i++ { + r2.autoDetectMAC(wifiMAC) + } + r2.autoDetectMAC(other) + for i := 0; i < macAdoptThreshold-1; i++ { + r2.autoDetectMAC(wifiMAC) + } + if !bytes.Equal(r2.phoneMAC, usbMAC) { + t.Fatalf("non-consecutive sightings changed the MAC to %x", r2.phoneMAC) + } + + // A genuine sustained run is adopted: one more consecutive sighting + // completes the threshold-1 streak. + r2.autoDetectMAC(wifiMAC) + if !bytes.Equal(r2.phoneMAC, wifiMAC) { + t.Fatalf("sustained change not adopted: got %x want %x", r2.phoneMAC, wifiMAC) + } + if r2.macChanges != 1 { + t.Fatalf("macChanges = %d, want 1", r2.macChanges) + } +} + +func TestAutoDetectMACCurrentMACCancelsCandidate(t *testing.T) { + r := newTestRelay(usbMAC) + + // Build a streak toward switching... + for i := 0; i < macAdoptThreshold-1; i++ { + r.autoDetectMAC(wifiMAC) + } + // ...then see the real USB MAC again. The candidate must be discarded, so + // a later single stray frame cannot tip it over the threshold. + r.autoDetectMAC(usbMAC) + r.autoDetectMAC(wifiMAC) + if !bytes.Equal(r.phoneMAC, usbMAC) { + t.Fatalf("interrupted streak still switched the MAC to %x", r.phoneMAC) + } +} + +func TestIsLoopFrame(t *testing.T) { + frame := func(src []byte) []byte { + f := make([]byte, 14) + copy(f[0:6], []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + copy(f[6:12], src) + return f + } + + r := newTestRelay(usbMAC) + + if !r.isLoopFrame(frame(clientMAC)) { + t.Error("frame with our own source MAC was not detected as a loop") + } + if r.isLoopFrame(frame(usbMAC)) { + t.Error("legitimate phone frame misdetected as a loop") + } + if r.isLoopFrame(frame(clientMAC[:4])) { + t.Error("truncated frame must not be treated as a loop") + } +} From adad122977d04a4ac854f0bdd859ec51297dcffd Mon Sep 17 00:00:00 2001 From: PC Date: Sun, 27 Sep 2026 18:42:18 +0300 Subject: [PATCH 2/5] fix(darwin): register as a routable configd service to stop mDNSResponder suppression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macOS 26+ regression: DNS set via scutil alone yields a SUPPLEMENTAL-only resolver. Without the RequestA bit (0x2), mDNSResponder marks A/AAAA records 'unusable' and suppresses every getaddrinfo query — Chromium/Electron apps keep working (own resolver) while Firefox/brew/curl fail on a tether that routes fine. Fix (proven end-to-end on macOS 27.0): make droidtether a real SystemConfiguration service. - tun: RegisterService() writes State:/Network/Service/droidtether/IPv4 {InterfaceName, Addresses, SubnetMasks /24, Router}; configd builds the route list itself, the service becomes routable, configd's own primary election sets Global/IPv4 PrimaryService=droidtether and the derived resolver gets RequestA (store __FLAGS__ 16386 + __CONFIGURATION_ID__). - tun: SetDNS keeps __FLAGS__ # 6 (CFNumber; '*' makes an array configd ignores) on the service dict and now REMOVES any raw State:/Network/Global/DNS instead of setting it: raw Global/DNS writes never reach mDNSResponder and make configd tear down the election (A/B probes). Close() removes the service IPv4+DNS dicts. - daemon: config watchdog (interval via route.watchdog_interval_s, 10s default) re-applies drift after standby: interface IP, split /1 routes, and — checked via the consumer's view (scutil --dns resolver #1 must carry 8.8.8.8 + 'Request A records', plus PrimaryService election), not the misleading raw store flags — service registration + DNS. - daemon: serialize device sessions, always release claimed USB interfaces on exit, bind watchdog to a session context so it never stacks or outlives the relay. Verified: getaddrinfo ~60ms, brew update / curl / dscacheutil green, Firefox confirmed, only harmless AAAA suppressions (no IPv6 on utun), /etc/hosts workarounds removed. --- config/config.go | 6 +- internal/daemon/daemon.go | 47 +++++++ internal/daemon/watchdog.go | 174 ++++++++++++++++++++++++ internal/daemon/watchdog_live_test.go | 48 +++++++ internal/daemon/watchdog_verify_test.go | 48 +++++++ internal/tun/utun.go | 1 + internal/tun/utun_darwin.go | 82 +++++++---- 7 files changed, 382 insertions(+), 24 deletions(-) create mode 100644 internal/daemon/watchdog.go create mode 100644 internal/daemon/watchdog_live_test.go create mode 100644 internal/daemon/watchdog_verify_test.go diff --git a/config/config.go b/config/config.go index 7ca5d0c..243b51d 100644 --- a/config/config.go +++ b/config/config.go @@ -41,6 +41,10 @@ type DHCPConfig struct { type RouteConfig struct { SetDefaultRoute bool `toml:"set_default_route"` RouteMetric int `toml:"route_metric"` + // WatchdogIntervalS: how often the in-daemon config watchdog verifies that + // the injected route/DNS/interface config are still in place and re-applies + // them if macOS removed them (e.g. after standby). Default 10s when unset. + WatchdogIntervalS int `toml:"watchdog_interval_s"` } type LoggingConfig struct { @@ -51,7 +55,7 @@ type LoggingConfig struct { // Default paths for DroidTether config files on macOS. var DefaultConfigPaths = []string{ "/etc/droidtether/droidtether.toml", - "/usr/local/etc/droidtether/droidtether.toml", // Intel Homebrew + "/usr/local/etc/droidtether/droidtether.toml", // Intel Homebrew "/opt/homebrew/etc/droidtether/droidtether.toml", // ARM Homebrew } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 566fe2e..bfdacd7 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1,6 +1,7 @@ package daemon import ( + "context" "fmt" "os" "os/signal" @@ -43,13 +44,35 @@ func (d *Daemon) Run() error { watcher := usb.NewWatcher(pollInterval) + // Serialize device sessions. OnAttach fires from the watcher's poll loop and + // previously did no bookkeeping, so a dying session's USB claim was still + // held when the next attach arrived. The new session then failed for ~8s + // with "libusb: bad access [code -3]" before the old one finished cleaning + // up, and each retry armed another watchdog. One session at a time; a new + // attach waits for the previous one to finish releasing the device. + var sessionMu sync.Mutex + watcher.OnAttach(func(dev *usb.Device) { + sessionMu.Lock() + defer sessionMu.Unlock() + d.wg.Add(1) defer d.wg.Done() log.Info(). Str("component", "daemon"). Msg("Android RNDIS device connected!") + // Always release the USB interfaces this session claimed. Without + // this, a relay that ends on a libusb error path leaves the claim + // dangling and the next attach cannot claim the interface. + defer func() { + if err := dev.Close(); err != nil { + log.Warn().Str("component", "daemon").Err(err).Msg("Failed to close USB device") + } else { + log.Debug().Str("component", "daemon").Msg("USB device released") + } + }() + session := rndis.NewSession(dev) phoneMAC, err := session.Handshake() if err != nil { @@ -63,6 +86,11 @@ func (d *Daemon) Run() error { return } + // Session-scoped context: the config watchdog must stop when this + // device session ends, not linger across reconnects. + sessionCtx, cancelSession := context.WithCancel(watcher.Context()) + defer cancelSession() + // Ensure everything closes when watcher is stopped go func() { <-watcher.Context().Done() @@ -70,6 +98,10 @@ func (d *Daemon) Run() error { }() defer iface.Close() + // The watchdog is started once per session; DHCP renewals may re-fire + // OnDHCP, but the watchdog must not stack. + var watchdogStarted sync.Once + relay, err := NewRelay(watcher.Context(), dev, iface, phoneMAC) if err != nil { log.Error().Str("component", "daemon").Err(err).Msg("Failed to initialize Relay") @@ -97,11 +129,26 @@ func (d *Daemon) Run() error { log.Warn().Str("component", "daemon").Err(err).Msg("Failed to set default route") } + // Make droidtether a real configd service so the + // primary election picks it and the derived DNS + // resolver gets RequestA (mDNSResponder otherwise + // suppresses every A/AAAA query). + if err := iface.RegisterService(client, gateway); err != nil { + log.Warn().Str("component", "daemon").Err(err).Msg("Failed to register service with configd") + } + // Set DNS to Google (Primary) and phone gateway (Secondary) log.Info().Str("component", "daemon").Msg("Setting system DNS to 8.8.8.8 (Google)...") if err := iface.SetDNS([]string{"8.8.8.8", gateway}); err != nil { log.Warn().Str("component", "daemon").Err(err).Msg("Failed to set DNS") } + + // macOS standby can silently remove everything we just + // injected while the USB session survives (no DHCP + // re-fire). Arm the watchdog that re-applies on drift. + watchdogStarted.Do(func() { + d.startConfigWatchdog(sessionCtx, iface, client, gateway, mtuStr) + }) } } } diff --git a/internal/daemon/watchdog.go b/internal/daemon/watchdog.go new file mode 100644 index 0000000..1e93ad7 --- /dev/null +++ b/internal/daemon/watchdog.go @@ -0,0 +1,174 @@ +package daemon + +import ( + "context" + "os/exec" + "strings" + "time" + + "github.com/rs/zerolog/log" + + "github.com/princePal/droidtether/internal/tun" +) + +// Config watchdog: macOS standby can silently remove the injected split +// default routes (0.0.0.0/1, 128.0.0.0/1), the dynamic-store DNS entries, and +// even the utun IP configuration — while the USB/RNDIS session survives the +// wake, so no new DHCP event fires and the original injection in OnDHCP never +// runs again. The result is a tethered-but-dead network: interface up, pipe +// to the phone alive, but no route past it and no name resolution. +// +// This watchdog verifies every few seconds that the injected config is still +// in place and re-applies whatever is missing. All checks are local +// syscalls/commands (route/ifconfig/scutil) — zero carrier data. + +// routeIntact reports whether the split default routes (0.0.0.0/1 and +// 128.0.0.0/1) are still installed on the given utun interface. +// +// It must NOT check `route -n get default`: DroidTether deliberately injects +// the two /1 split routes instead of a real default route, so a `default` +// query resolves to whatever real interface owns it (en0/Wi-Fi, en1/Ethernet) +// and the check fails forever — a permanent false positive that re-injected +// the routes every tick and spammed route(8) errors. Verify the routes we +// actually install. +func routeIntact(ifaceName string) bool { + out, err := exec.Command("netstat", "-rn", "-f", "inet").Output() + if err != nil { + return false + } + return routeIntactFromTable(string(out), ifaceName) +} + +// routeIntactFromTable holds the parsing logic, separated from the exec so it +// is unit-testable. +func routeIntactFromTable(table, ifaceName string) bool { + // Both halves must be present on our interface; one alone leaves a + // black hole for the other half of the IPv4 space. + for _, prefix := range []string{"0/1", "128.0/1"} { + found := false + for _, line := range strings.Split(table, "\n") { + fields := strings.Fields(line) + if len(fields) < 3 || fields[0] != prefix { + continue + } + if fields[len(fields)-1] == ifaceName { + found = true + break + } + } + if !found { + return false + } + } + return true +} + +// dnsIntact reports whether DroidTether's DNS is healthy end-to-end: +// configd still considers the droidtether service the primary (Global/IPv4 +// PrimaryService) and mDNSResponder's first resolver carries both our +// nameserver and the RequestA bit. Checking the derived resolver via +// `scutil --dns` instead of the raw store dict matters: a raw +// State:/Network/Global/DNS write can show __FLAGS__ with the A bit set +// while mDNSResponder still only has a Supplemental-only resolver and +// suppresses every A/AAAA query (getaddrinfo fails despite a "good" store). +func dnsIntact() bool { + cmd := exec.Command("scutil") + cmd.Stdin = strings.NewReader("show State:/Network/Global/IPv4\nquit\n") + out, err := cmd.Output() + if err != nil || !strings.Contains(string(out), "PrimaryService : droidtether") { + return false + } + + dns, err := exec.Command("scutil", "--dns").Output() + if err != nil { + return false + } + s := string(dns) + start := strings.Index(s, "resolver #1") + if start < 0 { + return false + } + block := s[start:] + if end := strings.Index(block, "resolver #2"); end >= 0 { + block = block[:end] + } + return strings.Contains(block, "8.8.8.8") && strings.Contains(block, "Request A records") +} + +// ifaceConfigIntact reports whether the utun interface still holds the DHCP +// lease address (ifconfig mtu ). +func ifaceConfigIntact(ifaceName, localIP string) bool { + out, err := exec.Command("ifconfig", ifaceName).Output() + if err != nil { + return false + } + return strings.Contains(string(out), "inet "+localIP) +} + +// startConfigWatchdog launches the watchdog goroutine for an active session. +// It is bound to ctx, so it stops together with the relay session. +func (d *Daemon) startConfigWatchdog(ctx context.Context, iface tun.Interface, localIP, gateway, mtu string) { + interval := time.Duration(d.cfg.Route.WatchdogIntervalS) * time.Second + if interval <= 0 { + interval = 10 * time.Second + } + + log.Info(). + Str("component", "watchdog"). + Dur("interval", interval). + Msg("Config watchdog started — will re-apply route/DNS/interface if macOS removes them (e.g. standby)") + + go func() { + ticker := time.NewTicker(interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + } + + name := iface.Name() + + // Interface config first: route/DNS injection and the relay + // itself are useless without the lease address. + if !ifaceConfigIntact(name, localIP) { + log.Warn(). + Str("component", "watchdog"). + Str("interface", name). + Msg("Interface IP config was removed — re-applying ifconfig...") + if err := iface.Configure(localIP, gateway, mtu); err != nil { + log.Error().Str("component", "watchdog").Err(err).Msg("Failed to re-apply interface config") + continue + } + } + + if !d.cfg.Route.SetDefaultRoute { + continue + } + + if !routeIntact(name) { + log.Warn(). + Str("component", "watchdog"). + Str("interface", name). + Msg("Default route no longer resolves through DroidTether — re-injecting split default routes...") + if err := iface.SetDefaultRoute(gateway); err != nil { + log.Error().Str("component", "watchdog").Err(err).Msg("Failed to re-inject default route") + } + } + + if !dnsIntact() { + log.Warn(). + Str("component", "watchdog"). + Msg("DroidTether DNS resolver broken (primary election or RequestA lost) — re-registering service and re-injecting DNS...") + if err := iface.RegisterService(localIP, gateway); err != nil { + log.Error().Str("component", "watchdog").Err(err).Msg("Failed to re-register service with configd") + } + if err := iface.SetDNS([]string{"8.8.8.8", gateway}); err != nil { + log.Error().Str("component", "watchdog").Err(err).Msg("Failed to re-inject DNS") + } + } + } + }() +} diff --git a/internal/daemon/watchdog_live_test.go b/internal/daemon/watchdog_live_test.go new file mode 100644 index 0000000..5c33fc6 --- /dev/null +++ b/internal/daemon/watchdog_live_test.go @@ -0,0 +1,48 @@ +package daemon + +import ( + "os/exec" + "strings" + "testing" +) + +// Live check: the function under test must return true against this machine's +// actual route table right now (both /1 splits installed on utun4), and the +// old `route -n get default` behaviour would have returned false. +func TestRouteIntactLive(t *testing.T) { + out, err := exec.Command("netstat", "-rn", "-f", "inet").Output() + if err != nil { + t.Skipf("netstat unavailable: %v", err) + } + + iface := detectTetherIface(t) + + if !routeIntactFromTable(string(out), iface) { + t.Fatalf("routeIntactFromTable() = false against live table on %s:\n%s", iface, out) + } + t.Logf("live route table on %s: splits present, check returns true", iface) + + // Demonstrate the old check's failure mode is real: `default` resolves to + // a different interface, which is exactly why the old code false-positived. + d, err := exec.Command("route", "-n", "get", "default").Output() + if err == nil { + t.Logf("for the record, `route -n get default` says:\n%s", d) + } +} + +// detectTetherIface finds the utun holding the droidtether split routes. +func detectTetherIface(t *testing.T) string { + t.Helper() + out, err := exec.Command("netstat", "-rn", "-f", "inet").Output() + if err != nil { + t.Skip("netstat unavailable") + } + for _, line := range strings.Split(string(out), "\n") { + f := strings.Fields(line) + if len(f) >= 3 && f[0] == "0/1" { + return f[len(f)-1] + } + } + t.Skip("no droidtether split routes present (not tethered via USB)") + return "" +} diff --git a/internal/daemon/watchdog_verify_test.go b/internal/daemon/watchdog_verify_test.go new file mode 100644 index 0000000..8910376 --- /dev/null +++ b/internal/daemon/watchdog_verify_test.go @@ -0,0 +1,48 @@ +package daemon + +import "testing" + +// Guards the exact regression that caused the 10s re-injection loop: the check +// must validate the split /1 routes droidtether actually installs, and must not +// depend on `route -n get default` (which resolves to en0/Wi-Fi on macOS 27). +func TestRouteIntactParsesNetstatColumns(t *testing.T) { + const live = `Routing tables + +Internet: +Destination Gateway Flags Netif Expire +0/1 utun4 UScg utun4 +default 192.168.35.25 UGScg en0 +127 127.0.0.1 UCS lo0 +128.0/1 utun4 USc utun4 +169.254 link#14 UCS en0 +192.168.60.85 192.168.60.3 UH utun4 +` + + cases := []struct { + name string + table string + want bool + }{ + {"both splits on utun4", live, true}, + {"only 0/1 present is a black hole", `0/1 utun4 UScg utun4 +default 192.168.35.25 UGScg en0 +`, false}, + {"only 128.0/1 present is a black hole", `default 192.168.35.25 UGScg en0 +128.0/1 utun4 USc utun4 +`, false}, + {"splits on the wrong interface", `0/1 en0 UScg en0 +128.0/1 en0 USc en0 +`, false}, + {"no splits at all", `default 192.168.35.25 UGScg en0 +`, false}, + {"empty table", "", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := routeIntactFromTable(tc.table, "utun4"); got != tc.want { + t.Fatalf("routeIntactFromTable() = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/internal/tun/utun.go b/internal/tun/utun.go index 91b4bc5..82ba806 100644 --- a/internal/tun/utun.go +++ b/internal/tun/utun.go @@ -10,5 +10,6 @@ type Interface interface { Name() string Configure(localIP, remoteIP, mtu string) error SetDefaultRoute(gateway string) error + RegisterService(localIP, gateway string) error SetDNS(dnsServers []string) error } diff --git a/internal/tun/utun_darwin.go b/internal/tun/utun_darwin.go index a441704..2d74602 100644 --- a/internal/tun/utun_darwin.go +++ b/internal/tun/utun_darwin.go @@ -36,16 +36,15 @@ func (i *utunInterface) Close() error { _ = exec.Command("route", "delete", "-net", "0.0.0.0/1").Run() _ = exec.Command("route", "delete", "-net", "128.0.0.0/1").Run() - // Cleanup DNS we added - _ = exec.Command("scutil", "-w", "State:/Network/Service/droidtether/DNS").Run() + // Cleanup the SystemConfiguration registration and DNS we added cmd := exec.Command("scutil") stdin, _ := cmd.StdinPipe() go func() { defer stdin.Close() fmt.Fprintln(stdin, "open") + fmt.Fprintln(stdin, "remove State:/Network/Service/droidtether/IPv4") fmt.Fprintln(stdin, "remove State:/Network/Service/droidtether/DNS") fmt.Fprintln(stdin, "remove State:/Network/Global/DNS") - fmt.Fprintln(stdin, "quit") }() _ = cmd.Run() @@ -65,26 +64,64 @@ func (i *utunInterface) Configure(localIP, remoteIP, mtu string) error { return nil } -// SetDefaultRoute adds "more specific" default routes (0.0.0.0/1 and 128.0.0.0/1) -// to override the existing default route without deleting it. +// SetDefaultRoute adds "more specific" default routes (0.0.0.0/1 and +// 128.0.0.0/1) to override the existing default route without deleting it. +// Idempotent: a plain `route add` fails with "File exists" when the route is +// already installed, so delete-then-add and tolerate a failed delete. func (i *utunInterface) SetDefaultRoute(gateway string) error { - // 0.0.0.0/1 - cmd1 := exec.Command("route", "add", "-net", "0.0.0.0/1", "-interface", i.name) - if out, err := cmd1.CombinedOutput(); err != nil { - return fmt.Errorf("failed to add 0/1 route: %w (output: %s)", err, string(out)) - } + _ = gateway // routes are injected by interface, not via the gateway - // 128.0.0.0/1 - cmd2 := exec.Command("route", "add", "-net", "128.0.0.0/1", "-interface", i.name) - if out, err := cmd2.CombinedOutput(); err != nil { - return fmt.Errorf("failed to add 128/1 route: %w (output: %s)", err, string(out)) + for _, net := range []string{"0.0.0.0/1", "128.0.0.0/1"} { + _ = exec.Command("route", "delete", "-net", net).Run() + cmd := exec.Command("route", "add", "-net", net, "-interface", i.name) + if out, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to add %s route: %w (output: %s)", net, err, string(out)) + } } return nil } -// SetDNS sets the system DNS to the phone gateway (or other provided servers) -// using 'scutil' on macOS. +// RegisterService registers DroidTether as a real SystemConfiguration +// service by writing State:/Network/Service/droidtether/IPv4 +// (InterfaceName/Addresses/SubnetMasks/Router). configd then builds the +// service's route list itself: a non-zero Router yields a default route, +// which makes the service routable, so the primary election picks it and +// the derived global DNS resolver gets the RequestA bit. Without that bit +// mDNSResponder marks A/AAAA records "unusable" and suppresses every +// getaddrinfo query (the post-macOS-26 tethering breakage). Android +// tethering subnets are /24; utun is point-to-point, so the mask only +// shapes the subnet route and never reachability. +func (i *utunInterface) RegisterService(localIP, gateway string) error { + cmd := exec.Command("scutil") + stdin, err := cmd.StdinPipe() + if err != nil { + return err + } + go func() { + defer stdin.Close() + fmt.Fprintln(stdin, "open") + fmt.Fprintln(stdin, "d.init") + fmt.Fprintln(stdin, "d.add InterfaceName "+i.name) + fmt.Fprintln(stdin, "d.add Addresses * "+localIP) + fmt.Fprintln(stdin, "d.add SubnetMasks * 255.255.255.0") + fmt.Fprintln(stdin, "d.add Router "+gateway) + fmt.Fprintln(stdin, "set State:/Network/Service/droidtether/IPv4") + fmt.Fprintln(stdin, "quit") + }() + if out, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("scutil service IPv4 set failed: %w (output: %s)", err, string(out)) + } + return nil +} + +// SetDNS writes our nameservers into the droidtether service's DNS entity +// (with __FLAGS__ 6 = RequestA|RequestAAAA so configd's derivation keeps the +// RequestA bit) and clears any stale raw override of State:/Network/Global/ +// DNS. configd republishes the derived resolver once the service is routable +// (see RegisterService). Global/DNS must never be set directly: a raw write +// is ignored by mDNSResponder and makes configd tear down the primary +// election, re-deriving a resolver without RequestA. func (i *utunInterface) SetDNS(dnsServers []string) error { if len(dnsServers) == 0 { return nil @@ -108,14 +145,13 @@ func (i *utunInterface) SetDNS(dnsServers []string) error { // SupplementalMatchDomains set to empty string tells macOS to use these servers for ALL lookups. fmt.Fprintln(stdin, "d.add SupplementalMatchDomains * \"\"") fmt.Fprintln(stdin, "d.add SupplementalMatchOrders * 10") + // RequestA/RequestAAAA (0x2|0x4): without them mDNSResponder treats + // A/AAAA records as "unusable" and suppresses every getaddrinfo + // query. configd ORs these into __FLAGS__ (=> 0x4006; it masks the + // AAAA bit back off when the interface has no IPv6). + fmt.Fprintln(stdin, "d.add __FLAGS__ # 6") fmt.Fprintln(stdin, "set State:/Network/Service/droidtether/DNS") - fmt.Fprintln(stdin, "set State:/Network/Global/DNS") - - // Force Global IPv4 state to 'Online' via our utun interface - fmt.Fprintln(stdin, "d.init") - fmt.Fprintf(stdin, "d.add Router %s\n", dnsServers[len(dnsServers)-1]) // use phone gateway - fmt.Fprintf(stdin, "d.add Interface %s\n", i.name) - fmt.Fprintln(stdin, "set State:/Network/Global/IPv4") + fmt.Fprintln(stdin, "remove State:/Network/Global/DNS") fmt.Fprintln(stdin, "quit") }() From 27cd380c32ebfff2abe1233aa0c0673bf47f772d Mon Sep 17 00:00:00 2001 From: pc Date: Mon, 28 Sep 2026 13:20:42 +0300 Subject: [PATCH 3/5] =?UTF-8?q?fix(darwin):=20survive=20post-sleep=20recov?= =?UTF-8?q?ery=20storm=20=E2=80=94=20scan=20self-heal=20+=20handshake=20ba?= =?UTF-8?q?ckoff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a Mac sleep Android disables USB tethering itself; the phone still enumerates the RNDIS config but the gadget never answers INIT. The old daemon re-attached every 500ms against that dead/surprise-removed device, which poisoned the libusb context permanently (libusb_get_device_list returns LIBUSB_ERROR_OTHER -99 forever) and the daemon went deaf even after the user re-enabled tethering. - watcher: track consecutive scan errors; exit(1) at 10 so launchd KeepAlive restarts the daemon with a fresh libusb context. - daemon: linear backoff (2s..30s cap) on consecutive handshake failures, resets on success; after 5 failures notify the console user via launchctl asuser + osascript to re-enable USB tethering on the phone. (Android blocks adb from enabling tethering itself: SELinux shell context, 'cmd tethering' absent, service call tethering 1 -> SecurityException.) --- internal/daemon/daemon.go | 44 ++++++++++++++++++++++++++++++++++++++- internal/usb/watcher.go | 37 ++++++++++++++++++++------------ 2 files changed, 67 insertions(+), 14 deletions(-) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index bfdacd7..ccd6f34 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -4,7 +4,9 @@ import ( "context" "fmt" "os" + "os/exec" "os/signal" + "strings" "sync" "syscall" "time" @@ -51,6 +53,9 @@ func (d *Daemon) Run() error { // up, and each retry armed another watchdog. One session at a time; a new // attach waits for the previous one to finish releasing the device. var sessionMu sync.Mutex + // Consecutive RNDIS handshake failures. Sessions are serialized by + // sessionMu, so a plain counter is enough. + var handshakeFails int watcher.OnAttach(func(dev *usb.Device) { sessionMu.Lock() @@ -76,9 +81,30 @@ func (d *Daemon) Run() error { session := rndis.NewSession(dev) phoneMAC, err := session.Handshake() if err != nil { - log.Error().Str("component", "daemon").Err(err).Msg("RNDIS Handshake failed") + handshakeFails++ + log.Error().Str("component", "daemon").Err(err). + Int("consecutive", handshakeFails). + Msg("RNDIS Handshake failed") + // After a Mac sleep Android typically disables USB + // tethering itself: the phone still enumerates the RNDIS + // config but its gadget never answers INIT. Only the user + // can switch it back on, so tell them, and back off — + // the 1s reconnect storm against a surprise-removed + // device is what poisoned the libusb context (-99). + if handshakeFails == 5 { + go notifyPhoneTetheringOff() + } + backoff := time.Duration(handshakeFails) * 2 * time.Second + if backoff > 30*time.Second { + backoff = 30 * time.Second + } + select { + case <-time.After(backoff): + case <-watcher.Context().Done(): + } return } + handshakeFails = 0 iface, err := tun.OpenUTUN(0) if err != nil { @@ -200,6 +226,22 @@ func (d *Daemon) Run() error { return nil } +// notifyPhoneTetheringOff shows a macOS user notification through the +// console user's GUI session (the daemon runs as root). Best-effort: +// every failure is ignored. +func notifyPhoneTetheringOff() { + out, err := exec.Command("stat", "-f", "%u", "/dev/console").Output() + if err != nil { + return + } + uid := strings.TrimSpace(string(out)) + if uid == "" || uid == "0" { + return + } + _ = exec.Command("launchctl", "asuser", uid, "osascript", "-e", + `display notification "USB tethering is disabled on the phone. Enable it in Settings → Network & internet → Hotspot & tethering → USB tethering." with title "DroidTether" sound name "Glass"`).Run() +} + func (d *Daemon) setupLogging() { // Level level, err := zerolog.ParseLevel(d.cfg.Logging.Level) diff --git a/internal/usb/watcher.go b/internal/usb/watcher.go index ae3d5fd..6357d23 100644 --- a/internal/usb/watcher.go +++ b/internal/usb/watcher.go @@ -66,24 +66,30 @@ func (w *Watcher) pollLoop() { ticker := time.NewTicker(w.pollInterval) defer ticker.Stop() + // After a Mac sleep the phone's USB tethering function can be + // disabled by Android itself (VBUS drop -> tethering toggle off). + // The daemon then re-attaches every poll and races open/close on a + // device that is surprise-removed, which poisons the libusb context + // permanently: libusb_get_device_list then returns LIBUSB_ERROR_OTHER + // (-99) forever and the daemon goes deaf — it never notices the user + // re-enabling tethering. No in-process recovery of that context is + // reliable, so the scan loop tracks consecutive failures and exits + // the process once the context is clearly poisoned; launchd + // (KeepAlive, ThrottleInterval 5) restarts it with a fresh libusb + // context that sees the device again. + scanErrStreak := 0 + const scanErrFatalAt = 10 + for { select { case <-w.ctx.Done(): return case <-ticker.C: - // If we already have a device attached, check if it's still there. - // Re-evaluating USB device states on macOS typically requires re-listing to clear ghosts. if w.activeDeviceHandle != nil { - // We can simply continue polling, but an advanced implementation would verify device presence. - // For the sake of simplicity, we assume robust handled detach scenarios for now and skip listing. continue } - // Open devices listing. devs, err := w.usbCtx.OpenDevices(func(desc *gousb.DeviceDesc) bool { - // Pre-filter: We cannot know interfaces at this stage easily in some architectures without - // checking configs, but we can verify class matching in MatchRNDIS. - // OpenDevices pre-filters using a fast check before full open. for _, cfg := range desc.Configs { for _, intf := range cfg.Interfaces { for _, alt := range intf.AltSettings { @@ -93,7 +99,6 @@ func (w *Watcher) pollLoop() { } } } - // Test secondary strings or known vendors. if MatchRNDIS(uint16(desc.Vendor), uint16(desc.Product), 0xFF, 0x00, 0x00) { return true } @@ -101,23 +106,31 @@ func (w *Watcher) pollLoop() { }) if err != nil { + scanErrStreak++ log.Error(). Str("component", "usb"). Err(err). + Int("consecutive", scanErrStreak). Msg("Error scanning devices") for _, d := range devs { d.Close() } + if scanErrStreak >= scanErrFatalAt { + log.Fatal(). + Str("component", "usb"). + Int("consecutive", scanErrStreak). + Msg("USB device-list context is poisoned; exiting for launchd restart") + } continue } + scanErrStreak = 0 if len(devs) > 0 { log.Debug(). Str("component", "usb"). Int("candidates", len(devs)). Msg("Found match, checking interface setup...") - candidate := devs[0] // take the first matched valid device. MVP only supports one device. - // Close the others quickly. + candidate := devs[0] for i := 1; i < len(devs); i++ { devs[i].Close() } @@ -136,14 +149,12 @@ func (w *Watcher) pollLoop() { if w.onAttachFn != nil { go func() { w.onAttachFn(deviceWrapper) - // when the device eventually detaches or errors out... if w.onDetachFn != nil { log.Debug(). Str("component", "usb"). Msg("Device detached workflow triggered") w.onDetachFn() } - // Clean up handles if not already cleaned by Stop() if w.activeDeviceHandle != nil { w.activeDeviceHandle.Close() w.activeDeviceHandle = nil From 2c7ffb67543dfa95baada1a33b0e4d748ba689a4 Mon Sep 17 00:00:00 2001 From: pc Date: Mon, 28 Sep 2026 14:44:50 +0300 Subject: [PATCH 4/5] fix(darwin): kill zombie relay sessions when the phone leaves tethering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android disables USB tethering when the phone screen locks: the phone re-enumerates without the RNDIS config, but the relay's writes to the dead handle keep 'succeeding' (gousb does not error), so the session becomes a zombie that holds the watcher's active-device slot — the watcher then refuses to re-attach even after the user re-enables tethering, and the daemon just sits there incrementing a sent counter against a device that is not there. Probe the phone gateway through the tunnel every 10s; after 3 consecutive failed probes end the session so the watcher can re-attach (or wait for the user's re-enable, covered by the handshake-failure notification). --- internal/daemon/daemon.go | 1 + internal/daemon/watchdog.go | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index ccd6f34..94c5f0e 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -174,6 +174,7 @@ func (d *Daemon) Run() error { // re-fire). Arm the watchdog that re-applies on drift. watchdogStarted.Do(func() { d.startConfigWatchdog(sessionCtx, iface, client, gateway, mtuStr) + go d.startLinkWatchdog(sessionCtx, relay, gateway, client) }) } } diff --git a/internal/daemon/watchdog.go b/internal/daemon/watchdog.go index 1e93ad7..72bb8a3 100644 --- a/internal/daemon/watchdog.go +++ b/internal/daemon/watchdog.go @@ -172,3 +172,42 @@ func (d *Daemon) startConfigWatchdog(ctx context.Context, iface tun.Interface, l } }() } + +// startLinkWatchdog probes the phone's gateway through the tunnel. When +// Android disables USB tethering (e.g. the phone screen locks), the phone +// re-enumerates without the RNDIS config and the relay's writes to the +// dead handle keep "succeeding" — the session becomes a zombie that holds +// the watcher's active-device slot, so a later re-enable never re-attaches. +// Three consecutive failed probes end the session; the watcher then sees +// the device again (or waits for the user to re-enable tethering, covered +// by the handshake-failure notification). +func (d *Daemon) startLinkWatchdog(ctx context.Context, relay *Relay, gateway, client string) { + go func() { + ticker := time.NewTicker(10 * time.Second) + defer ticker.Stop() + fails := 0 + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + err := exec.Command("ping", "-c", "1", "-t", "3", "-S", client, gateway).Run() + if err == nil { + fails = 0 + continue + } + fails++ + if fails < 3 { + log.Warn().Str("component", "watchdog"). + Int("fails", fails). + Msg("Gateway ping failed") + continue + } + log.Error().Str("component", "watchdog"). + Msg("Phone left tethering state; ending zombie session") + relay.Stop() + return + } + } + }() +} From dbba472169b2a43cbf464003533738f4eac16d06 Mon Sep 17 00:00:00 2001 From: PC Date: Mon, 28 Sep 2026 15:24:29 +0300 Subject: [PATCH 5/5] fix(darwin): adopt phone MAC from DHCP response, not just 8-packet threshold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RNDIS handshake reports the gadget's *permanent* MAC, but Android re-randomizes the active rndis0 MAC per tethering enable. The relay's 8-consecutive-packet adoption threshold never sees enough DHCP frames before the link watchdog kills the session, so every unicast frame (ping, DNS) was addressed to the stale MAC and silently dropped by the phone — DHCP (broadcast) still worked, which is exactly the observed 'DHCP ok, ping fails' signature. --- internal/daemon/relay.go | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/internal/daemon/relay.go b/internal/daemon/relay.go index 3a76ed3..d93f940 100644 --- a/internal/daemon/relay.go +++ b/internal/daemon/relay.go @@ -210,6 +210,13 @@ func (r *Relay) Start() error { continue } + // DHCP is the phone's own server answering us: its + // source MAC is authoritative for the tethering + // interface, so adopt it immediately (the 8-packet + // threshold never sees enough DHCP frames). + if isDHCPResponse(ethPkt) { + r.adoptMAC(ethPkt[6:12]) + } // Auto-detect phone's real MAC from received traffic. // Samsung devices randomize the active MAC on the tethering // interface, so the RNDIS-queried address may not match. @@ -347,6 +354,41 @@ func (r *Relay) autoDetectMAC(srcMAC []byte) { Msg("Phone MAC change confirmed — switching destination MAC") } +// isDHCPResponse reports whether an Ethernet frame carries a UDP 67→68 +// (server-to-client) DHCP message — the offer/ack that proves the sender is +// the phone's own tethering interface. +func isDHCPResponse(ethPkt []byte) bool { + if binary.BigEndian.Uint16(ethPkt[12:14]) != 0x0800 { + return false + } + rawIP := ethPkt[14:] + if len(rawIP) < 28 || rawIP[9] != 17 { + return false + } + return binary.BigEndian.Uint16(rawIP[20:22]) == 67 && binary.BigEndian.Uint16(rawIP[22:24]) == 68 +} + +// adoptMAC unconditionally re-points the relay's destination MAC to srcMAC +// (skipping the consecutive-sighting threshold), used only when the frame is +// self-authenticating evidence of the phone's tethering interface. +func (r *Relay) adoptMAC(srcMAC []byte) { + r.mu.Lock() + if bytes.Equal(srcMAC, clientMAC) || bytes.Equal(srcMAC, r.phoneMAC) { + r.mu.Unlock() + return + } + old := hex.EncodeToString(r.phoneMAC) + r.macChanges++ + copy(r.phoneMAC, srcMAC) + r.candMAC = nil + r.candCount = 0 + r.mu.Unlock() + log.Warn().Str("component", "relay"). + Str("old_mac", old). + Str("new_mac", hex.EncodeToString(srcMAC)). + Msg("Adopted phone MAC from DHCP response — destination re-pointed") +} + // isLoopFrame reports whether an inbound frame is our own traffic being // reflected back at us, which must never be injected into the tunnel. func (r *Relay) isLoopFrame(ethPkt []byte) bool {