diff --git a/cmd/cli/ad_os_lookup_test.go b/cmd/cli/ad_os_lookup_test.go new file mode 100644 index 0000000..ea1c145 --- /dev/null +++ b/cmd/cli/ad_os_lookup_test.go @@ -0,0 +1,39 @@ +package cli + +import ( + "testing" + + "github.com/miekg/dns" +) + +// The auto-added AD split rules route to upstream.os, and a rule match skips +// the LAN classification, so without this check every AD lookup was also sent +// to Control D's public resolver in plaintext. +func Test_isAdOSResolverLookup(t *testing.T) { + p := &prog{adDomain: "corp.lab"} + for _, tc := range []struct { + name string + qname string + qtype uint16 + upstreams []string + want bool + }{ + {"AD host via OS", "DC01.corp.lab.", dns.TypeA, []string{upstreamOS}, true}, + {"DC locator SRV via OS", "_ldap._tcp.dc._msdcs.corp.lab.", dns.TypeSRV, []string{upstreamOS}, true}, + {"AD domain itself", "corp.lab.", dns.TypeA, []string{upstreamOS}, true}, + {"AD name, explicit upstream", "dc01.corp.lab.", dns.TypeA, []string{upstreamPrefix + "0"}, false}, + {"bypass rule, public name", "example.com.", dns.TypeA, []string{upstreamOS}, false}, + {"lookalike suffix", "notcorp.lab.", dns.TypeA, []string{upstreamOS}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + m := new(dns.Msg) + m.SetQuestion(tc.qname, tc.qtype) + if got := p.isAdOSResolverLookup(tc.upstreams, m); got != tc.want { + t.Errorf("got %v, want %v", got, tc.want) + } + }) + } + if (&prog{}).isAdOSResolverLookup([]string{upstreamOS}, new(dns.Msg).SetQuestion("dc01.corp.lab.", dns.TypeA)) { + t.Error("no AD domain must match nothing") + } +} diff --git a/cmd/cli/dns_proxy.go b/cmd/cli/dns_proxy.go index 50a1dd6..00527af 100644 --- a/cmd/cli/dns_proxy.go +++ b/cmd/cli/dns_proxy.go @@ -645,6 +645,14 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *proxyResponse { isLanOrPtrQuery := false if req.ufr.matched { ctrld.Log(ctx, mainLog.Load().Debug(), "%s, %s, %s -> %v", req.ufr.matchedPolicy, req.ufr.matchedNetwork, req.ufr.matchedRule, upstreams) + // A rule sending an Active Directory name to the OS resolver (the + // auto-added AD split rule, or an operator's equivalent) is a LAN + // lookup: keep it off Control D's public resolver, which osResolver + // otherwise races against the domain controller in plaintext. + if p.isAdOSResolverLookup(upstreams, req.msg) { + ctx = ctrld.LanQueryCtx(ctx) + ctrld.Log(ctx, mainLog.Load().Debug(), "active directory domain lookup, skipping public DNS") + } } else { switch { case req.ufr.zpaDNSEcho: @@ -2788,3 +2796,10 @@ func (p *prog) buildRecoveryUpstreams(reason RecoveryReason) map[string]*ctrld.U func ValidateDefaultLocalIPsFromDelta(newState *netmon.State) { validateDefaultLocalIPsFromDelta(newState, 0) } + +// isAdOSResolverLookup reports whether a rule-matched query is for the machine's +// 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) +} diff --git a/docs/internal-domains.md b/docs/internal-domains.md index 72e70d6..dfa1f3f 100644 --- a/docs/internal-domains.md +++ b/docs/internal-domains.md @@ -104,6 +104,16 @@ exists for a source is never overwritten. From highest precedence to lowest: rule, so an Internal Domain for the AD domain takes precedence over the auto-detected one. + A query under the AD domain whose matched rule resolves to the OS resolver + alone (the auto-detected rule, a Magic Folder exclude, or an OS-resolver + Internal Domain) is a LAN query, so the OS resolver does not race Control D's + public resolver (76.76.2.0) against the domain controller. Before this, a + rule match skipped the LAN classification and every such lookup + (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. + **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: