mirror of
https://github.com/Control-D-Inc/ctrld.git
synced 2026-10-12 01:58:46 +02:00
fix(cli): keep Active Directory lookups off Control D's public resolver
The auto-added AD split rules route corp names to upstream.os, but a rule match returned before the LAN-query marking, so osResolver raced the domain controller against 76.76.2.0 and sent every AD lookup (DC hostnames, DC-locator SRV names carrying the machine name, WPAD) there in plaintext. Mark a matched query under the machine's AD domain whose upstreams are the OS resolver alone as a LAN query, which drops 76.76.2.0 for it.
This commit is contained in:
1 parent
36d4167388
commit
6edce5f94f
5 files changed
+127
No files matched your search
@@ -0,0 +1,36 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
)
|
||||
|
||||
// activeDirectoryDomain is the machine's Active Directory domain, recorded when
|
||||
// addExtraSplitDnsRule detects it (Windows only; empty elsewhere).
|
||||
//
|
||||
// Names in it are resolved by the domain controller through upstream.os. The
|
||||
// OS resolver races the LAN servers against Control D's public resolver unless
|
||||
// the query is marked as a LAN query, and a split-rule match returns before
|
||||
// that marking, so without this AD names (DC hostnames, DC-locator SRV
|
||||
// records carrying the machine name, WPAD) were sent to 76.76.2.0 in plaintext.
|
||||
var activeDirectoryDomain atomic.Pointer[string]
|
||||
|
||||
func setActiveDirectoryDomain(domain string) {
|
||||
d := strings.TrimSuffix(strings.ToLower(strings.TrimSpace(domain)), ".")
|
||||
activeDirectoryDomain.Store(&d)
|
||||
}
|
||||
|
||||
// inActiveDirectoryDomain reports whether name is the AD domain or below it.
|
||||
func inActiveDirectoryDomain(name string) bool {
|
||||
d := activeDirectoryDomain.Load()
|
||||
if d == nil || *d == "" {
|
||||
return false
|
||||
}
|
||||
name = strings.TrimSuffix(strings.ToLower(name), ".")
|
||||
return name == *d || strings.HasSuffix(name, "."+*d)
|
||||
}
|
||||
|
||||
// osUpstreamOnly reports whether upstreams is exactly the OS resolver.
|
||||
func osUpstreamOnly(upstreams []string) bool {
|
||||
return len(upstreams) == 1 && upstreams[0] == upstreamOS
|
||||
}
|
||||
@@ -0,0 +1,73 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/miekg/dns"
|
||||
|
||||
"github.com/Control-D-Inc/ctrld"
|
||||
)
|
||||
|
||||
func withActiveDirectoryDomain(t *testing.T, domain string) {
|
||||
t.Helper()
|
||||
prev := activeDirectoryDomain.Load()
|
||||
setActiveDirectoryDomain(domain)
|
||||
t.Cleanup(func() { activeDirectoryDomain.Store(prev) })
|
||||
}
|
||||
|
||||
func Test_inActiveDirectoryDomain(t *testing.T) {
|
||||
withActiveDirectoryDomain(t, "Corp.Lab.")
|
||||
for name, want := range map[string]bool{
|
||||
"corp.lab.": true,
|
||||
"DC01.corp.lab.": true,
|
||||
"_ldap._tcp.dc._msdcs.corp.lab.": true,
|
||||
"wpad.corp.lab": true,
|
||||
"notcorp.lab.": false,
|
||||
"corp.lab.example.com.": false,
|
||||
"13gmuaedixa.verify.controld.com.": false,
|
||||
} {
|
||||
if got := inActiveDirectoryDomain(name); got != want {
|
||||
t.Errorf("inActiveDirectoryDomain(%q) = %v, want %v", name, got, want)
|
||||
}
|
||||
}
|
||||
activeDirectoryDomain.Store(nil)
|
||||
if inActiveDirectoryDomain("dc01.corp.lab.") {
|
||||
t.Error("no AD domain recorded must match nothing")
|
||||
}
|
||||
}
|
||||
|
||||
// A split-rule match returns before the LAN-query marking; AD names routed to
|
||||
// the OS resolver must still be marked, so osResolver drops 76.76.2.0.
|
||||
func Test_handleSpecialQueryTypes_ADRuleIsLanQuery(t *testing.T) {
|
||||
withActiveDirectoryDomain(t, "corp.lab")
|
||||
p := newTestProg(t)
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
qname string
|
||||
qtype uint16
|
||||
upstreams []string
|
||||
wantLan 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 name, explicit upstream", "dc01.corp.lab.", dns.TypeA, []string{upstreamPrefix + "0"}, false},
|
||||
{"bypass rule, public name", "example.com.", dns.TypeA, []string{upstreamOS}, false},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
m := new(dns.Msg)
|
||||
m.SetQuestion(tc.qname, tc.qtype)
|
||||
req := &proxyRequest{msg: m, ufr: &upstreamForResult{matched: true, matchedPolicy: "My Policy", matchedNetwork: "no network", matchedRule: "*.corp.lab"}}
|
||||
ctx := context.Background()
|
||||
upstreams := tc.upstreams
|
||||
var configs []*ctrld.UpstreamConfig
|
||||
if res := p.handleSpecialQueryTypes(&ctx, req, &upstreams, &configs); res != nil {
|
||||
t.Fatalf("unexpected local answer: %v", res)
|
||||
}
|
||||
lan, _ := ctx.Value(ctrld.LanQueryCtxKey{}).(bool)
|
||||
if lan != tc.wantLan {
|
||||
t.Errorf("LAN query = %v, want %v", lan, tc.wantLan)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -21,6 +21,7 @@ func addExtraSplitDnsRule(cfg *ctrld.Config) bool {
|
||||
// Network rules are lowercase during toml config marshaling,
|
||||
// lowercase the domain here too for consistency.
|
||||
domain = strings.ToLower(domain)
|
||||
setActiveDirectoryDomain(domain)
|
||||
domainRuleAdded := addSplitDnsRule(cfg, domain)
|
||||
wildcardDomainRuleRuleAdded := addSplitDnsRule(cfg, "*."+strings.TrimPrefix(domain, "."))
|
||||
return domainRuleAdded || wildcardDomainRuleRuleAdded
|
||||
|
||||
@@ -709,6 +709,13 @@ func (p *prog) handleSpecialQueryTypes(ctx *context.Context, req *proxyRequest,
|
||||
if req.ufr.matched {
|
||||
ctrld.Log(*ctx, p.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.
|
||||
if osUpstreamOnly(*upstreams) && 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")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -94,6 +94,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 returned ahead of 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:
|
||||
|
||||
|
||||
Reference in new issue
Block a user