diff --git a/cmd/cli/ad_os_lookup_test.go b/cmd/cli/ad_os_lookup_test.go index ea1c145..8152896 100644 --- a/cmd/cli/ad_os_lookup_test.go +++ b/cmd/cli/ad_os_lookup_test.go @@ -1,9 +1,16 @@ package cli import ( + "context" + "errors" + "slices" + "strings" "testing" + "time" "github.com/miekg/dns" + + "github.com/Control-D-Inc/ctrld" ) // The auto-added AD split rules route to upstream.os, and a rule match skips @@ -37,3 +44,142 @@ func Test_isAdOSResolverLookup(t *testing.T) { t.Error("no AD domain must match nothing") } } + +// adLookupResolver records whether each resolve carried the LAN mark, which is +// what keeps osResolver off Control D's public resolver. Only the OS resolver +// answers, so an explicit upstream fails over to it. +type adLookupResolver struct { + name string + seen *[]adLookupResolve +} + +type adLookupResolve struct { + resolver string + lan bool +} + +func (r adLookupResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, error) { + lan, _ := ctx.Value(ctrld.LanQueryCtxKey{}).(bool) + *r.seen = append(*r.seen, adLookupResolve{r.name, lan}) + if r.name != osUpstreamConfig.Name { + return nil, errors.New("upstream unreachable") + } + answer := new(dns.Msg) + answer.SetReply(msg) + return answer, nil +} + +func newAdLookupProg(t *testing.T, leak bool) (*prog, *[]adLookupResolve) { + t.Helper() + cfg := &ctrld.Config{Upstream: map[string]*ctrld.UpstreamConfig{ + "0": {Name: "explicit resolver", Type: ctrld.ResolverTypeLegacy, Endpoint: "192.0.2.53:53", Timeout: 100}, + }} + cfg.Service.LeakOnUpstreamFailure = &leak + p := &prog{cfg: cfg, adDomain: "corp.lab", um: newUpstreamMonitor(cfg)} + p.um.after = func(time.Duration, func()) {} + p.lanLoopGuard = newLoopGuard() + p.ptrLoopGuard = newLoopGuard() + t.Cleanup(func() { p.querySampler.closeExpired(time.Now().Add(querySampleWindow)) }) + + var seen []adLookupResolve + origNewResolver, origRecovery := newResolverFn, queryRecoveryFn + t.Cleanup(func() { newResolverFn, queryRecoveryFn = origNewResolver, origRecovery }) + newResolverFn = func(uc *ctrld.UpstreamConfig) (ctrld.Resolver, error) { + return adLookupResolver{name: uc.Name, seen: &seen}, nil + } + queryRecoveryFn = func(*prog, RecoveryReason) {} + return p, &seen +} + +func adLookupQuery(p *prog, name string, ufr *upstreamForResult) *proxyResponse { + msg := new(dns.Msg) + qtype := dns.TypeA + if strings.HasPrefix(name, "_") { + qtype = dns.TypeSRV + } + msg.SetQuestion(name, qtype) + ufr.srcAddr = "192.168.0.1:1234" + return p.proxy(context.Background(), &proxyRequest{msg: msg, ufr: ufr}) +} + +func wantAdLookupResolves(t *testing.T, got []adLookupResolve, want ...adLookupResolve) { + t.Helper() + if !slices.Equal(got, want) { + t.Fatalf("resolves = %+v, want %+v", got, want) + } +} + +// Drive proxy() itself, so the test fails if a path stops marking the query. +func Test_proxyKeepsAdLookupsOffThePublicResolver(t *testing.T) { + captureDebugMainLog(t) + osName := osUpstreamConfig.Name + for _, tc := range []struct { + name string + qname string + ufr *upstreamForResult + leak bool + want []adLookupResolve + }{ + { + // The auto-added AD rule has no targets, so proxy() routes it to the OS resolver. + name: "auto-added AD rule", + qname: "dc01.corp.lab.", + ufr: &upstreamForResult{matched: true}, + want: []adLookupResolve{{osName, true}}, + }, + { + name: "DC locator SRV through an AD rule", + qname: "_ldap._tcp.dc._msdcs.corp.lab.", + ufr: &upstreamForResult{matched: true}, + want: []adLookupResolve{{osName, true}}, + }, + { + name: "OS rule for a public name keeps the public fallback", + qname: "example.com.", + ufr: &upstreamForResult{matched: true}, + want: []adLookupResolve{{osName, false}}, + }, + { + name: "explicit rule for an AD name falls back to the OS resolver", + qname: "dc01.corp.lab.", + ufr: &upstreamForResult{matched: true, upstreams: []string{upstreamPrefix + "0"}}, + leak: true, + want: []adLookupResolve{{"explicit resolver", false}, {osName, true}}, + }, + { + name: "explicit rule for a public name falls back with the public resolver", + qname: "example.com.", + ufr: &upstreamForResult{matched: true, upstreams: []string{upstreamPrefix + "0"}}, + leak: true, + want: []adLookupResolve{{"explicit resolver", false}, {osName, false}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + p, seen := newAdLookupProg(t, tc.leak) + if res := adLookupQuery(p, tc.qname, tc.ufr); res == nil || res.answer == nil { + t.Fatal("the proxy returned no answer") + } + wantAdLookupResolves(t, *seen, tc.want...) + }) + } +} + +// The DNS-intercept recovery bypass forwards queries before any rule handling. +func Test_proxyRecoveryBypassKeepsAdLookupsOffThePublicResolver(t *testing.T) { + captureDebugMainLog(t) + useDNSIntercept(t) + osName := osUpstreamConfig.Name + for _, tc := range []struct { + qname string + lan bool + }{{"dc01.corp.lab.", true}, {"example.com.", false}} { + t.Run(tc.qname, func(t *testing.T) { + p, seen := newAdLookupProg(t, false) + p.recoveryBypass.Store(true) + if res := adLookupQuery(p, tc.qname, &upstreamForResult{upstreams: []string{upstreamPrefix + "0"}}); res == nil || res.answer == nil { + t.Fatal("the proxy returned no answer") + } + wantAdLookupResolves(t, *seen, adLookupResolve{osName, tc.lan}) + }) + } +} diff --git a/cmd/cli/dns_proxy.go b/cmd/cli/dns_proxy.go index 00527af..b9246cf 100644 --- a/cmd/cli/dns_proxy.go +++ b/cmd/cli/dns_proxy.go @@ -585,9 +585,17 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *proxyResponse { // may use. if dnsIntercept && p.recoveryBypass.Load() && !p.internalDomainExplicitUpstreams(req.ufr.upstreams) { ctrld.Log(ctx, mainLog.Load().Debug(), "Recovery bypass active: forwarding to OS resolver") - resolver, err := ctrld.NewResolver(osUpstreamConfig) + // The bypass runs before rule handling, so an Active Directory name + // needs its LAN mark here too, or it races the domain controller + // against Control D's public resolver in plaintext. + bypassCtx := ctx + if p.isAdDomainLookup(req.msg) { + bypassCtx = ctrld.LanQueryCtx(ctx) + ctrld.Log(ctx, mainLog.Load().Debug(), "active directory domain lookup, skipping public DNS") + } + resolver, err := newResolverFn(osUpstreamConfig) if err == nil { - resolveCtx, cancel := osUpstreamConfig.Context(ctx) + resolveCtx, cancel := osUpstreamConfig.Context(bypassCtx) defer cancel() answer, _ := resolver.Resolve(resolveCtx, req.msg) if answer != nil { @@ -855,7 +863,7 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *proxyResponse { resolve1 := func(upstream string, upstreamConfig *ctrld.UpstreamConfig, msg *dns.Msg) (*dns.Msg, error) { ctrld.Log(ctx, mainLog.Load().Debug(), "sending query to %s: %s", upstream, upstreamConfig.Name) - dnsResolver, err := ctrld.NewResolver(upstreamConfig) + dnsResolver, err := newResolverFn(upstreamConfig) if err != nil { ctrld.Log(ctx, mainLog.Load().Error().Err(err), "failed to create resolver") return nil, err @@ -1082,6 +1090,12 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *proxyResponse { // we dont want this to happen if leakOnUpstreamFailure is false if upstreams[0] != upstreamOS { ctrld.Log(ctx, mainLog.Load().Debug(), "attempting query to OS resolver as a retry catch all") + // An Active Directory name that falls back from its explicit + // upstream still must not reach Control D's public resolver. + if p.isAdDomainLookup(req.msg) { + ctx = ctrld.LanQueryCtx(ctx) + ctrld.Log(ctx, mainLog.Load().Debug(), "active directory domain lookup, skipping public DNS") + } answer := resolve(upstreamOS, osUpstreamConfig, req.msg) if answer != nil { ctrld.Log(ctx, mainLog.Load().Debug(), "OS resolver retry query successful") @@ -2801,5 +2815,11 @@ func ValidateDefaultLocalIPsFromDelta(newState *netmon.State) { // Active Directory domain and goes to the OS resolver alone. Such a query is a // LAN lookup; osResolver must not race it against Control D's public resolver. func (p *prog) isAdOSResolverLookup(upstreams []string, msg *dns.Msg) bool { - return len(msg.Question) > 0 && len(upstreams) == 1 && upstreams[0] == upstreamOS && p.isAdDomainQuery(msg) + return len(upstreams) == 1 && upstreams[0] == upstreamOS && p.isAdDomainLookup(msg) +} + +// isAdDomainLookup reports whether msg asks for a name under the machine's +// Active Directory domain. A public answer for such a name is never correct. +func (p *prog) isAdDomainLookup(msg *dns.Msg) bool { + return len(msg.Question) > 0 && p.isAdDomainQuery(msg) } diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go index 09c3809..a64941a 100644 --- a/cmd/cli/prog.go +++ b/cmd/cli/prog.go @@ -1439,6 +1439,7 @@ var ( ensureInterceptDNSTargetFn = (*prog).ensureInterceptDNSTarget removeInterceptDNSTargetFn = (*prog).removeInterceptDNSTarget initializeOsResolverWithSystemNameserversFn = ctrld.InitializeOsResolverWithSystemNameserversReason + newResolverFn = ctrld.NewResolver setDnsForRunningIfaceFn = (*prog).setDnsForRunningIface resetDNSFn = (*prog).resetDNS // refuseFallbackFatal reports a startup failure the interface-DNS fallback diff --git a/docs/internal-domains.md b/docs/internal-domains.md index dfa1f3f..492f1ad 100644 --- a/docs/internal-domains.md +++ b/docs/internal-domains.md @@ -112,7 +112,10 @@ exists for a source is never overwritten. From highest precedence to lowest: (DC hostnames, DC-locator SRV names that carry the machine name, WPAD) also went to 76.76.2.0 in plaintext; when both failed, the public NXDOMAIN could be the answer returned. An explicit-resolver rule for the AD domain is - unchanged. + unchanged. The same LAN mark applies wherever an AD name reaches the OS + resolver outside rule handling: the DNS-intercept recovery bypass, and the + OS-resolver retry after an explicit upstream fails with + `leak_on_upstream_failure` enabled. **VPN DNS auto-detection** is evaluated inside `proxy()` in DNS-intercept mode, after policy matching and independently of it, so it needs its own rule: