diff --git a/cmd/cli/dns_target_darwin.go b/cmd/cli/dns_target_darwin.go index 21101c1..023bcba 100644 --- a/cmd/cli/dns_target_darwin.go +++ b/cmd/cli/dns_target_darwin.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "slices" + "time" "tailscale.com/net/netmon" ) @@ -20,6 +21,12 @@ import ( // could neither remove it on shutdown nor keep its bookkeeping consistent. const interceptDNSTargetStateFile = ".intercept_dns_target" +// ipv6OnlyConfirmDelay is how long "no IPv4" must persist before it counts +// as an IPv6-only network rather than DHCPv4 still in progress. A DHCPv4 +// client normally completes in a few seconds; issue #629 allows 45s from +// joining to working DNS, which leaves room for recovery and RFC 7050. +const ipv6OnlyConfirmDelay = 15 * time.Second + var ( interceptDNSTargetStatePathFn = func() string { return absHomeDir(interceptDNSTargetStateFile) } interceptDefaultRouteInterfaceFn = netmon.DefaultRouteInterface @@ -44,6 +51,11 @@ var ( interceptSavedStaticNameserversFn = savedStaticNameservers interceptResetDNSIgnoreUnusableIfaceFn = resetDnsIgnoreUnusableInterface interceptDHCPNameserversForInterfaceFn = readTargetDHCPNameservers + interceptNowFn = time.Now + // interceptIPv6OnlyAfterFn schedules the reconcile that can complete an + // IPv6-only confirmation. The pf watchdog would also get there, but only + // after up to pfWatchdogInterval more. + interceptIPv6OnlyAfterFn = time.AfterFunc ) // A retained target record means cleanup was not completed. The broad saved @@ -143,6 +155,13 @@ func (p *prog) persistInterceptDNSTargetStateLocked() error { // target there, macOS cannot emit the ipv4only.arpa lookup it needs to start // the CLAT, so the CLAT evidence alone can never arrive (issue #629). // +// "No IPv4 yet" also describes a dual-stack network whose router +// advertisement arrived before the DHCPv4 ACK. The IPv6-only evidence must +// therefore hold for ipv6OnlyConfirmDelay on the same service and device +// before it admits a target. A lease or IPv4 state arriving in that window +// fails the evidence on the next pass (at the latest the scheduled one) and +// ends the confirmation without a write. +// // Callers pass a non-nil raw system discovery result to prove discovery ran; // an empty slice is a valid DNS-less result. The decision itself uses static // DNS plus DHCP option 6 from the default-route interface, so resolvers on a @@ -155,6 +174,16 @@ func (p *prog) ensureInterceptDNSTarget(systemDiscovery []string) { p.interceptDNSTargetMu.Lock() defer p.interceptDNSTargetMu.Unlock() p.loadInterceptDNSTargetStateLocked() + // A pass that reads the native evidence and does not end waiting on + // IPv6-only confirmation restarts it: continuity means every observation + // in between agreed. Passes that fail before reading evidence know + // nothing new and leave a pending confirmation alone. + keepConfirm := true + defer func() { + if !keepConfirm { + p.resetIPv6OnlyConfirmLocked() + } + }() var diagnostic *dnsTargetDecisionDiagnostic if state, ok := p.dnsInterceptState.(*pfState); ok { diagnostic = &state.targetDiagnostic @@ -191,6 +220,7 @@ func (p *prog) ensureInterceptDNSTarget(systemDiscovery []string) { return } routeDHCPDNS, dhcpErr := interceptDHCPNameserversForInterfaceFn(drIfaceName) + keepConfirm = false nativeFallback := false ctx, cancel := context.WithTimeout(context.Background(), nativeTargetReadBudget) defer cancel() @@ -302,6 +332,11 @@ func (p *prog) ensureInterceptDNSTarget(systemDiscovery []string) { if p.interceptDNSTargetService == iface.Name && p.interceptDNSTargetSetValue == target { return // already set on this service } + if nativeEvidence == "ipv6_only" && !p.ipv6OnlyConfirmedLocked(nativeService) { + keepConfirm = true + resolvedReason = "ipv6_only_confirming" + return + } if nativeFallback { owned := "" if p.interceptDNSTargetService == iface.Name { @@ -349,6 +384,48 @@ func (p *prog) ensureInterceptDNSTarget(systemDiscovery []string) { Msgf("intercept DNS target: service %q provides no usable IPv4 DNS; set %s so macOS can emit DNS queries (removed automatically when the network provides IPv4 DNS)", iface.Name, target) } +// ipv6OnlyConfirmedLocked reports whether the same IPv6-only observation has +// held for ipv6OnlyConfirmDelay. The first observation starts the clock and +// schedules one follow-up reconcile; a different service or device starts it +// again. Callers must hold interceptDNSTargetMu. +func (p *prog) ipv6OnlyConfirmedLocked(svc nativeTargetService) bool { + now := interceptNowFn() + seen := &p.interceptIPv6OnlySeen + if seen.since.IsZero() || seen.service != svc { + p.resetIPv6OnlyConfirmLocked() + *seen = ipv6OnlyObservation{service: svc, since: now} + p.interceptIPv6OnlyTimer = interceptIPv6OnlyAfterFn(ipv6OnlyConfirmDelay, p.confirmIPv6OnlyTarget) + journal(mainLog.Load().Info()).Str("service", svc.Name).Str("interface", svc.Device). + Dur("confirm_after", ipv6OnlyConfirmDelay). + Msgf("intercept DNS target: service %q has no IPv4 yet; confirming no DHCPv4 arrives before setting a target", svc.Name) + return false + } + return now.Sub(seen.since) >= ipv6OnlyConfirmDelay +} + +// confirmIPv6OnlyTarget is the scheduled confirming reconcile. It runs as +// network activity so shutdown waits for or skips it. +func (p *prog) confirmIPv6OnlyTarget() { + if !p.beginNetworkActivity() { + return + } + defer p.netMonitorWG.Done() + if p.dnsInterceptState == nil { + return + } + p.ensureInterceptDNSTarget([]string{}) +} + +// resetIPv6OnlyConfirmLocked drops a pending IPv6-only confirmation and its +// follow-up. Callers must hold interceptDNSTargetMu. +func (p *prog) resetIPv6OnlyConfirmLocked() { + if p.interceptIPv6OnlyTimer != nil { + p.interceptIPv6OnlyTimer.Stop() + p.interceptIPv6OnlyTimer = nil + } + p.interceptIPv6OnlySeen = ipv6OnlyObservation{} +} + // removeInterceptDNSTarget removes a previously set intercept DNS target, // restoring the service's saved static DNS (or empty). Safe no-op when no // target was set. @@ -356,6 +433,7 @@ func (p *prog) removeInterceptDNSTarget(reason string) { p.interceptDNSTargetMu.Lock() defer p.interceptDNSTargetMu.Unlock() p.loadInterceptDNSTargetStateLocked() + p.resetIPv6OnlyConfirmLocked() p.removeInterceptDNSTargetLocked(reason) } diff --git a/cmd/cli/dns_target_darwin_test.go b/cmd/cli/dns_target_darwin_test.go index c71a65c..8547c95 100644 --- a/cmd/cli/dns_target_darwin_test.go +++ b/cmd/cli/dns_target_darwin_test.go @@ -11,6 +11,7 @@ import ( "slices" "strings" "testing" + "time" "github.com/Control-D-Inc/ctrld" ) @@ -26,6 +27,8 @@ type interceptTargetHarness struct { ipv6Only bool ipv6OnlyErr error ipv6Calls int + now time.Time + confirms []time.Duration saveErr error readErr error setErr error @@ -62,6 +65,8 @@ func newInterceptTargetHarness(t *testing.T) *interceptTargetHarness { origNativeStatic := interceptNativeStaticDNSFn origSnapshot := interceptSaveStaticDNSSnapshotFn origIntercept := dnsIntercept + origNow := interceptNowFn + origAfter := interceptIPv6OnlyAfterFn t.Cleanup(func() { interceptDNSTargetStatePathFn = origPath interceptDefaultRouteInterfaceFn = origRoute @@ -78,7 +83,15 @@ func newInterceptTargetHarness(t *testing.T) *interceptTargetHarness { interceptNativeStaticDNSFn = origNativeStatic interceptSaveStaticDNSSnapshotFn = origSnapshot dnsIntercept = origIntercept + interceptNowFn = origNow + interceptIPv6OnlyAfterFn = origAfter }) + h.now = time.Unix(1_700_000_000, 0) + interceptNowFn = func() time.Time { return h.now } + interceptIPv6OnlyAfterFn = func(d time.Duration, _ func()) *time.Timer { + h.confirms = append(h.confirms, d) + return nil + } interceptNativeCLATDefaultServiceFn = func(_ context.Context, device string) (nativeTargetService, error) { if !h.nativeCLAT || h.nativeErr != nil { diff --git a/cmd/cli/dns_target_ipv6only_darwin_test.go b/cmd/cli/dns_target_ipv6only_darwin_test.go index 846c3bd..319a6a5 100644 --- a/cmd/cli/dns_target_ipv6only_darwin_test.go +++ b/cmd/cli/dns_target_ipv6only_darwin_test.go @@ -7,8 +7,17 @@ import ( "errors" "slices" "testing" + "time" ) +// confirmIPv6Only runs the first observation and the confirming pass one +// ipv6OnlyConfirmDelay later, as the scheduled follow-up would. +func confirmIPv6Only(h *interceptTargetHarness, p *prog) { + p.ensureInterceptDNSTarget([]string{}) + h.now = h.now.Add(ipv6OnlyConfirmDelay) + p.ensureInterceptDNSTarget([]string{}) +} + // Issue #629: an IPv6-only network without DHCPv4 and before macOS starts // its CLAT has neither a lease to read nor a CLAT to prove. func TestIPv6OnlyTargetAdmission(t *testing.T) { @@ -34,7 +43,7 @@ func TestIPv6OnlyTargetAdmission(t *testing.T) { h.ipv6Only, h.ipv6OnlyErr, h.nativeCLAT = tc.ipv6Only, tc.ipv6Err, tc.clat h.dns["Wi-Fi"], h.dhcp, h.dhcpErr = slices.Clone(tc.static), tc.dhcp, tc.dhcpErr p := newInterceptTargetProg() - p.ensureInterceptDNSTarget([]string{}) + confirmIPv6Only(h, p) if got := len(h.setCalls) > 0; got != tc.want || len(h.resetCalls) != 0 { t.Fatalf("set=%v reset=%v, want set=%v", h.setCalls, h.resetCalls, tc.want) } @@ -48,6 +57,130 @@ func TestIPv6OnlyTargetAdmission(t *testing.T) { } } +// One IPv6-only observation is not enough: it also matches a dual-stack +// network between its router advertisement and DHCPv4 ACK. The first pass +// only starts the clock and schedules the confirming reconcile. +func TestIPv6OnlyTargetWaitsForConfirmation(t *testing.T) { + h := newInterceptTargetHarness(t) + h.ipv6Only = true + h.dhcpErr = errors.New("no lease") + p := newInterceptTargetProg() + + p.ensureInterceptDNSTarget([]string{}) + if len(h.setCalls) != 0 || !slices.Equal(h.confirms, []time.Duration{ipv6OnlyConfirmDelay}) { + t.Fatalf("first observation: set=%v confirms=%v, want no set and one %s follow-up", h.setCalls, h.confirms, ipv6OnlyConfirmDelay) + } + h.now = h.now.Add(ipv6OnlyConfirmDelay - 1) + p.ensureInterceptDNSTarget([]string{}) + if len(h.setCalls) != 0 || len(h.confirms) != 1 { + t.Fatalf("before the delay: set=%v confirms=%v", h.setCalls, h.confirms) + } + h.now = h.now.Add(1) + p.ensureInterceptDNSTarget([]string{}) + if !slices.Equal(h.dns["Wi-Fi"], []string{"127.0.0.53"}) { + t.Fatalf("confirmed IPv6-only network not admitted: %v", h.dns["Wi-Fi"]) + } + if p.interceptIPv6OnlySeen != (ipv6OnlyObservation{}) { + t.Fatal("confirmation state kept after admission") + } +} + +// Dual-stack network, DHCPv4 late: the RA arrives first, so the first pass +// sees the IPv6-only shape. The lease then arrives before the confirming +// pass, and the service is never modified. +func TestIPv6OnlyTargetDualStackDHCPv4Late(t *testing.T) { + for _, tc := range []struct { + name string + late func(*interceptTargetHarness) + }{ + {name: "lease readable", late: func(h *interceptTargetHarness) { + h.dhcpErr, h.dhcp = nil, []string{"192.168.1.1"} + }}, + {name: "IPv4 state published before the lease is readable", late: func(h *interceptTargetHarness) { + h.ipv6Only = false + }}, + } { + t.Run(tc.name, func(t *testing.T) { + h := newInterceptTargetHarness(t) + h.ipv6Only = true + h.dhcpErr = errors.New("no lease") + h.dns["Wi-Fi"] = []string{"2001:db8::53"} + p := newInterceptTargetProg() + + p.ensureInterceptDNSTarget([]string{}) + tc.late(h) + h.now = h.now.Add(ipv6OnlyConfirmDelay) + p.ensureInterceptDNSTarget([]string{}) + // Even a later return to the IPv6-only shape restarts the clock. + h.ipv6Only, h.dhcpErr, h.dhcp = true, errors.New("no lease"), nil + p.ensureInterceptDNSTarget([]string{}) + + if len(h.setCalls) != 0 || len(h.resetCalls) != 0 || !slices.Equal(h.dns["Wi-Fi"], []string{"2001:db8::53"}) { + t.Fatalf("dual-stack service modified: set=%v reset=%v dns=%v", h.setCalls, h.resetCalls, h.dns["Wi-Fi"]) + } + if len(h.confirms) != 2 { + t.Fatalf("confirmation not restarted after IPv4 appeared: %v", h.confirms) + } + }) + } +} + +// Failing before the evidence is read (no default route mid-transition) +// learns nothing new, so it neither admits nor restarts a confirmation. +func TestIPv6OnlyTargetConfirmationSurvivesNoDecision(t *testing.T) { + h := newInterceptTargetHarness(t) + h.ipv6Only = true + h.dhcpErr = errors.New("no lease") + p := newInterceptTargetProg() + + p.ensureInterceptDNSTarget([]string{}) + route := interceptDefaultRouteInterfaceFn + interceptDefaultRouteInterfaceFn = func() (string, error) { return "", errors.New("no default route") } + p.ensureInterceptDNSTarget([]string{}) + interceptDefaultRouteInterfaceFn = route + h.now = h.now.Add(ipv6OnlyConfirmDelay) + p.ensureInterceptDNSTarget([]string{}) + + if len(h.setCalls) != 1 || len(h.confirms) != 1 { + t.Fatalf("set=%v confirms=%v, want one admission from one confirmation", h.setCalls, h.confirms) + } +} + +// A different service on the same evidence path starts a new confirmation. +func TestIPv6OnlyTargetServiceChangeRestartsConfirmation(t *testing.T) { + h := newInterceptTargetHarness(t) + h.ipv6Only = true + h.dhcpErr = errors.New("no lease") + p := newInterceptTargetProg() + + p.ensureInterceptDNSTarget([]string{}) + read := interceptNativeIPv6OnlyServiceFn + interceptNativeIPv6OnlyServiceFn = func(ctx context.Context, device string) (nativeTargetService, error) { + svc, err := read(ctx, device) + svc.ID = nativeTargetOtherServiceID + return svc, err + } + h.now = h.now.Add(ipv6OnlyConfirmDelay) + p.ensureInterceptDNSTarget([]string{}) + if len(h.setCalls) != 0 || len(h.confirms) != 2 { + t.Fatalf("service change admitted on the old clock: set=%v confirms=%v", h.setCalls, h.confirms) + } +} + +// Shutdown drops a pending confirmation and its follow-up. +func TestIPv6OnlyTargetShutdownDropsConfirmation(t *testing.T) { + h := newInterceptTargetHarness(t) + h.ipv6Only = true + h.dhcpErr = errors.New("no lease") + p := newInterceptTargetProg() + + p.ensureInterceptDNSTarget([]string{}) + p.removeInterceptDNSTarget("intercept shutdown") + if p.interceptIPv6OnlySeen != (ipv6OnlyObservation{}) || p.interceptIPv6OnlyTimer != nil { + t.Fatal("shutdown kept a pending IPv6-only confirmation") + } +} + // The admitted target is removed when the network gains IPv4, and the // original IPv6 static DNS comes back. func TestIPv6OnlyTargetLifecycle(t *testing.T) { @@ -56,7 +189,7 @@ func TestIPv6OnlyTargetLifecycle(t *testing.T) { h.dhcpErr = errors.New("no lease") h.dns["Wi-Fi"] = []string{"2001:db8::53"} p := newInterceptTargetProg() - p.ensureInterceptDNSTarget([]string{}) + confirmIPv6Only(h, p) if p.interceptDNSTargetService != "Wi-Fi" { t.Fatal("no target on IPv6-only network") } @@ -90,14 +223,14 @@ func TestIPv6OnlyTargetRecheck(t *testing.T) { interceptNativeIPv6OnlyServiceFn = func(ctx context.Context, device string) (nativeTargetService, error) { calls++ svc, err := read(ctx, device) - if calls == 2 { + if calls == 4 { svc.ID = nativeTargetOtherServiceID } return svc, err } p := newInterceptTargetProg() - p.ensureInterceptDNSTarget([]string{}) - if calls != 2 || len(h.setCalls) != 0 || p.interceptDNSTargetService != "" { + confirmIPv6Only(h, p) + if calls != 4 || len(h.setCalls) != 0 || p.interceptDNSTargetService != "" { t.Fatalf("changed IPv6-only tuple admitted a target: calls=%d set=%v", calls, h.setCalls) } } diff --git a/cmd/cli/dns_target_ipv6only_reader_test.go b/cmd/cli/dns_target_ipv6only_reader_test.go index 893750e..3e1237e 100644 --- a/cmd/cli/dns_target_ipv6only_reader_test.go +++ b/cmd/cli/dns_target_ipv6only_reader_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "net" - "slices" "strings" "testing" ) @@ -128,7 +127,25 @@ func TestParseNativeIPv6GlobalRejectsUnsafe(t *testing.T) { t.Fatalf("accepted %q", out) } } - if !slices.Equal([]bool{nativeIPv6OnlyAddresses(nativeTargetTestAddrs("2001:db8::1/64"))}, []bool{true}) { - t.Fatal("global IPv6 rejected") +} + +func TestNativeIPv6OnlyAddresses(t *testing.T) { + for _, tc := range []struct { + name string + addrs []string + want bool + }{ + {name: "global IPv6", addrs: []string{"2001:db8::1/64"}, want: true}, + {name: "ULA with link-local", addrs: []string{"fd66:6:6::10/64", "fe80::1/64"}, want: true}, + {name: "link-local only", addrs: []string{"fe80::1/64"}}, + {name: "no addresses"}, + {name: "IPv4 address beside global IPv6", addrs: []string{"2001:db8::1/64", "10.0.0.2/24"}}, + {name: "CLAT address beside global IPv6", addrs: []string{"2001:db8::1/64", "192.0.0.2/32"}}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := nativeIPv6OnlyAddresses(nativeTargetTestAddrs(tc.addrs...)); got != tc.want { + t.Fatalf("got %v, want %v", got, tc.want) + } + }) } } diff --git a/cmd/cli/dns_target_native.go b/cmd/cli/dns_target_native.go index d442915..6251a8a 100644 --- a/cmd/cli/dns_target_native.go +++ b/cmd/cli/dns_target_native.go @@ -21,6 +21,15 @@ const ( nativeTargetMaxOutput = 64 * 1024 ) +// ipv6OnlyObservation is the first sighting of IPv6-only evidence that an +// admission must still match after ipv6OnlyConfirmDelay. +type ipv6OnlyObservation struct { + //lint:ignore U1000 used in Darwin code. + service nativeTargetService + //lint:ignore U1000 used in Darwin code. + since time.Time +} + var errNativeTargetOutputLimit = errors.New("native DNS evidence output exceeds limit") type nativeTargetOutput struct{ buf bytes.Buffer } @@ -85,6 +94,24 @@ func parseNativeServiceName(out string) (string, error) { return name, nil } +// serviceName validates the configured service root for id and returns the +// unique enabled networksetup service mapped to device. Both native readers +// share it, so a fix to either check applies to both. +func (r nativeTargetReader) serviceName(ctx context.Context, id, device string) (string, error) { + out, err := r.run(ctx, "show Setup:/Network/Service/"+id+"\nquit\n", "/usr/sbin/scutil") + if err != nil { + return "", err + } + if _, err := parseNativeServiceName(string(out)); err != nil { + return "", err + } + out, err = r.run(ctx, "", "/usr/sbin/networksetup", "-listnetworkserviceorder") + if err != nil { + return "", err + } + return targetNetworkServiceName(string(out), device) +} + // defaultService returns the actual primary service, not the first service // networksetup happens to list for the device. Unknown or changing state is // never absence evidence. The caller must carry and recheck the whole tuple. @@ -130,19 +157,7 @@ func (r nativeTargetReader) defaultService(ctx context.Context, device string) ( if !matched { return unknown, nil } - out, err := r.run(ctx, "show Setup:/Network/Service/"+id+"\nquit\n", "/usr/sbin/scutil") - if err != nil { - return unknown, err - } - _, err = parseNativeServiceName(string(out)) - if err != nil { - return unknown, err - } - out, err = r.run(ctx, "", "/usr/sbin/networksetup", "-listnetworkserviceorder") - if err != nil { - return unknown, err - } - name, err := targetNetworkServiceName(string(out), device) + name, err := r.serviceName(ctx, id, device) if err != nil { return unknown, err } @@ -373,7 +388,7 @@ func parseNativeIPv6Global(out string) (id, device string, err error) { } id = value case "PrimaryInterface": - if value == "" || strings.ContainsAny(value, " \r\n{}") { + if value == "" || strings.ContainsAny(value, " \t\r\n{}") { return "", "", errors.New("invalid interface") } device = value @@ -439,18 +454,7 @@ func (r nativeTargetReader) ipv6OnlyService(ctx context.Context, device string) if err != nil || !nativeIPv6OnlyAddresses(addrs) { return unknown, err } - out, err = r.run(ctx, "show Setup:/Network/Service/"+id+"\nquit\n", "/usr/sbin/scutil") - if err != nil { - return unknown, err - } - if _, err = parseNativeServiceName(string(out)); err != nil { - return unknown, err - } - out, err = r.run(ctx, "", "/usr/sbin/networksetup", "-listnetworkserviceorder") - if err != nil { - return unknown, err - } - name, err := targetNetworkServiceName(string(out), device) + name, err := r.serviceName(ctx, id, device) if err != nil { return unknown, err } diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go index fa5ebf7..b871580 100644 --- a/cmd/cli/prog.go +++ b/cmd/cli/prog.go @@ -226,6 +226,13 @@ type prog struct { interceptDNSTargetSetValue string //lint:ignore U1000 used in Darwin code. interceptDNSTargetLoaded bool + // interceptIPv6OnlySeen and interceptIPv6OnlyTimer track an IPv6-only + // observation that has not yet held long enough to admit a target + // (issue #629). Guarded by interceptDNSTargetMu. + //lint:ignore U1000 used in Darwin code. + interceptIPv6OnlySeen ipv6OnlyObservation + //lint:ignore U1000 used in Darwin code. + interceptIPv6OnlyTimer *time.Timer // interceptTargetMirror holds the same value as interceptDNSTargetSetValue // for readers that must not wait: the mutex above covers networksetup