diff --git a/cmd/cli/ad_os_lookup_test.go b/cmd/cli/ad_os_lookup_test.go new file mode 100644 index 0000000..0834a94 --- /dev/null +++ b/cmd/cli/ad_os_lookup_test.go @@ -0,0 +1,155 @@ +package cli + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + "time" + + "github.com/miekg/dns" + + "github.com/Control-D-Inc/ctrld" +) + +// 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 + withActiveDirectoryDomain(t, "corp.lab") + p := &prog{cfg: cfg, um: newUpstreamMonitor(cfg, mainLog.Load())} + p.logger.Store(mainLog.Load()) + p.recoveryCancel = func() {} + 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 := newResolverFn + t.Cleanup(func() { newResolverFn = origNewResolver }) + newResolverFn = func(_ context.Context, uc *ctrld.UpstreamConfig) (ctrld.Resolver, error) { + return adLookupResolver{name: uc.Name, seen: &seen}, nil + } + 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 a009dd5..73f6f99 100644 --- a/cmd/cli/dns_proxy.go +++ b/cmd/cli/dns_proxy.go @@ -765,7 +765,15 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *proxyResponse { // general DNS failure does not authorize disclosing private names to DHCP DNS. if dnsIntercept && p.recoveryBypass.Load() && !internalDomainExplicitUpstreams(req.ufr.upstreams) { ctrld.Log(ctx, p.Debug(), "Recovery bypass active: forwarding to OS resolver") - answer := p.queryUpstream(ctx, req, upstreamOS, 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 len(req.msg.Question) > 0 && inActiveDirectoryDomain(req.msg.Question[0].Name) { + bypassCtx = ctrld.LanQueryCtx(ctx) + ctrld.Log(ctx, p.Debug(), "Active Directory domain lookup, skipping public DNS") + } + answer := p.queryUpstream(bypassCtx, req, upstreamOS, osUpstreamConfig) if answer != nil { return &proxyResponse{answer: answer, upstream: osUpstreamConfig.Endpoint} } @@ -1201,7 +1209,7 @@ func (p *prog) queryUpstream(ctx context.Context, req *proxyRequest, upstream st } ctrld.Log(ctx, p.Debug(), "Sending query to %s: %s", upstream, upstreamConfig.Name) - dnsResolver, err := ctrld.NewResolver(ctx, upstreamConfig) + dnsResolver, err := newResolverFn(ctx, upstreamConfig) if err != nil { ctrld.Log(ctx, p.Error().Err(err), "Failed to create resolver") return nil @@ -1274,6 +1282,12 @@ func (p *prog) triggerRecovery(isOSFailure bool) { // Logs success or failure of the query attempt and returns a proxyResponse or nil based on query result. func (p *prog) tryOSResolver(ctx context.Context, req *proxyRequest) *proxyResponse { ctrld.Log(ctx, p.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 len(req.msg.Question) > 0 && inActiveDirectoryDomain(req.msg.Question[0].Name) { + ctx = ctrld.LanQueryCtx(ctx) + ctrld.Log(ctx, p.Debug(), "Active Directory domain lookup, skipping public DNS") + } answer := p.queryUpstream(ctx, req, upstreamOS, osUpstreamConfig) if answer != nil { ctrld.Log(ctx, p.Debug(), "OS resolver retry query successful") diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go index b26a0c2..fc937f7 100644 --- a/cmd/cli/prog.go +++ b/cmd/cli/prog.go @@ -1477,6 +1477,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 d43464e..337cd73 100644 --- a/docs/internal-domains.md +++ b/docs/internal-domains.md @@ -102,7 +102,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: