Skip to content

Log whether DNS queries were DNSSEC-validated #8459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Log whether DNS queries were DNSSEC-validated #8459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
acdb058
Include AD bit in ValidationRecords
aarongable Oct 24, 2025
baea2d0
And log it with CAA logs
aarongable Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 19 additions & 18 deletions bdns/dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ type ResolverAddrs []string

// Client queries for DNS records
type Client interface {
LookupTXT(context.Context, string) (txts []string, resolver ResolverAddrs, err error)
LookupHost(context.Context, string) ([]netip.Addr, ResolverAddrs, error)
LookupCAA(context.Context, string) ([]*dns.CAA, string, ResolverAddrs, error)
LookupTXT(context.Context, string) (txts []string, resolver ResolverAddrs, ad bool, err error)
LookupHost(context.Context, string) ([]netip.Addr, ResolverAddrs, bool, error)
LookupCAA(context.Context, string) ([]*dns.CAA, string, ResolverAddrs, bool, error)
}

// impl represents a client that talks to an external resolver
Expand Down Expand Up @@ -322,13 +322,13 @@ type dnsResp struct {
// LookupTXT sends a DNS query to find all TXT records associated with
// the provided hostname which it returns along with the returned
// DNS authority section.
func (dnsClient *impl) LookupTXT(ctx context.Context, hostname string) ([]string, ResolverAddrs, error) {
func (dnsClient *impl) LookupTXT(ctx context.Context, hostname string) ([]string, ResolverAddrs, bool, error) {
var txt []string
dnsType := dns.TypeTXT
r, resolver, err := dnsClient.exchangeOne(ctx, hostname, dnsType)
errWrap := wrapErr(dnsType, hostname, r, err)
if errWrap != nil {
return nil, ResolverAddrs{resolver}, errWrap
return nil, ResolverAddrs{resolver}, false, errWrap
}

for _, answer := range r.Answer {
Expand All @@ -339,10 +339,10 @@ func (dnsClient *impl) LookupTXT(ctx context.Context, hostname string) ([]string
}
}

return txt, ResolverAddrs{resolver}, err
return txt, ResolverAddrs{resolver}, r.AuthenticatedData, err
}

func (dnsClient *impl) lookupIP(ctx context.Context, hostname string, ipType uint16) ([]dns.RR, string, error) {
func (dnsClient *impl) lookupIP(ctx context.Context, hostname string, ipType uint16) ([]dns.RR, string, bool, error) {
resp, resolver, err := dnsClient.exchangeOne(ctx, hostname, ipType)
switch ipType {
case dns.TypeA:
Expand All @@ -356,27 +356,28 @@ func (dnsClient *impl) lookupIP(ctx context.Context, hostname string, ipType uin
}
errWrap := wrapErr(ipType, hostname, resp, err)
if errWrap != nil {
return nil, resolver, errWrap
return nil, resolver, false, errWrap
}
return resp.Answer, resolver, nil
return resp.Answer, resolver, resp.AuthenticatedData, nil
}

// LookupHost sends a DNS query to find all A and AAAA records associated with
// the provided hostname. This method assumes that the external resolver will
// chase CNAME/DNAME aliases and return relevant records. It will retry
// requests in the case of temporary network errors. It returns an error if
// both the A and AAAA lookups fail or are empty, but succeeds otherwise.
func (dnsClient *impl) LookupHost(ctx context.Context, hostname string) ([]netip.Addr, ResolverAddrs, error) {
func (dnsClient *impl) LookupHost(ctx context.Context, hostname string) ([]netip.Addr, ResolverAddrs, bool, error) {
var recordsA, recordsAAAA []dns.RR
var errA, errAAAA error
var resolverA, resolverAAAA string
var adA, adAAAA bool
var wg sync.WaitGroup

wg.Go(func() {
recordsA, resolverA, errA = dnsClient.lookupIP(ctx, hostname, dns.TypeA)
recordsA, resolverA, adA, errA = dnsClient.lookupIP(ctx, hostname, dns.TypeA)
})
wg.Go(func() {
recordsAAAA, resolverAAAA, errAAAA = dnsClient.lookupIP(ctx, hostname, dns.TypeAAAA)
recordsAAAA, resolverAAAA, adAAAA, errAAAA = dnsClient.lookupIP(ctx, hostname, dns.TypeAAAA)
})
wg.Wait()

Expand Down Expand Up @@ -427,17 +428,17 @@ func (dnsClient *impl) LookupHost(ctx context.Context, hostname string) ([]netip
// branching. We don't use ProblemDetails and SubProblemDetails here, because
// this error will get wrapped in a DNSError and further munged by higher
// layers in the stack.
return nil, resolvers, fmt.Errorf("%w; %s", errA, errAAAA)
return nil, resolvers, false, fmt.Errorf("%w; %s", errA, errAAAA)
}

return append(addrsA, addrsAAAA...), resolvers, nil
return append(addrsA, addrsAAAA...), resolvers, adA && adAAAA, nil
Copy link
Contributor

@jsha jsha Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has somewhat confusing semantics in this case:

  • Response for A is successful, with AD bit set.
  • Response for AAAA is an error.

This results in returning false for the ad boolean.

This makes me think maybe LookupHost combining A and AAAA lookups is the wrong abstraction. In the VA, in newHTTPValidationTarget and tryGetChallengeCert, the first thing we do with the combined response is to split it into v4 addresses and v6 addresses. Perhaps we can simplify things with LookupA and LookupAAAA being separate.

}

// LookupCAA sends a DNS query to find all CAA records associated with
// the provided hostname and the complete dig-style RR `response`. This
// response is quite verbose, however it's only populated when the CAA
// response is non-empty.
func (dnsClient *impl) LookupCAA(ctx context.Context, hostname string) ([]*dns.CAA, string, ResolverAddrs, error) {
func (dnsClient *impl) LookupCAA(ctx context.Context, hostname string) ([]*dns.CAA, string, ResolverAddrs, bool, error) {
dnsType := dns.TypeCAA
r, resolver, err := dnsClient.exchangeOne(ctx, hostname, dnsType)

Expand All @@ -448,12 +449,12 @@ func (dnsClient *impl) LookupCAA(ctx context.Context, hostname string) ([]*dns.C
// rechecking. But allow NXDOMAIN for TLDs to fall through to the error code
// below, so we don't issue for gTLDs that have been removed by ICANN.
if err == nil && r.Rcode == dns.RcodeNameError && strings.Contains(hostname, ".") {
return nil, "", ResolverAddrs{resolver}, nil
return nil, "", ResolverAddrs{resolver}, false, nil
}

errWrap := wrapErr(dnsType, hostname, r, err)
if errWrap != nil {
return nil, "", ResolverAddrs{resolver}, errWrap
return nil, "", ResolverAddrs{resolver}, false, errWrap
}

var CAAs []*dns.CAA
Expand All @@ -466,7 +467,7 @@ func (dnsClient *impl) LookupCAA(ctx context.Context, hostname string) ([]*dns.C
if len(CAAs) > 0 {
response = r.String()
}
return CAAs, response, ResolverAddrs{resolver}, nil
return CAAs, response, ResolverAddrs{resolver}, r.AuthenticatedData, nil
}

// logDNSError logs the provided err result from making a query for hostname to
Expand Down
62 changes: 31 additions & 31 deletions bdns/dns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,14 +287,14 @@ func TestDNSNoServers(t *testing.T) {

obj := New(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)

_, resolvers, err := obj.LookupHost(context.Background(), "letsencrypt.org")
_, resolvers, _, err := obj.LookupHost(context.Background(), "letsencrypt.org")
test.AssertEquals(t, len(resolvers), 0)
test.AssertError(t, err, "No servers")

_, _, err = obj.LookupTXT(context.Background(), "letsencrypt.org")
_, _, _, err = obj.LookupTXT(context.Background(), "letsencrypt.org")
test.AssertError(t, err, "No servers")

_, _, _, err = obj.LookupCAA(context.Background(), "letsencrypt.org")
_, _, _, _, err = obj.LookupCAA(context.Background(), "letsencrypt.org")
test.AssertError(t, err, "No servers")
}

Expand All @@ -304,7 +304,7 @@ func TestDNSOneServer(t *testing.T) {

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)

_, resolvers, err := obj.LookupHost(context.Background(), "cps.letsencrypt.org")
_, resolvers, _, err := obj.LookupHost(context.Background(), "cps.letsencrypt.org")
test.AssertEquals(t, len(resolvers), 2)
slices.Sort(resolvers)
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})
Expand All @@ -317,7 +317,7 @@ func TestDNSDuplicateServers(t *testing.T) {

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)

_, resolvers, err := obj.LookupHost(context.Background(), "cps.letsencrypt.org")
_, resolvers, _, err := obj.LookupHost(context.Background(), "cps.letsencrypt.org")
test.AssertEquals(t, len(resolvers), 2)
slices.Sort(resolvers)
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})
Expand All @@ -331,13 +331,13 @@ func TestDNSServFail(t *testing.T) {
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
bad := "servfail.com"

_, _, err = obj.LookupTXT(context.Background(), bad)
_, _, _, err = obj.LookupTXT(context.Background(), bad)
test.AssertError(t, err, "LookupTXT didn't return an error")

_, _, err = obj.LookupHost(context.Background(), bad)
_, _, _, err = obj.LookupHost(context.Background(), bad)
test.AssertError(t, err, "LookupHost didn't return an error")

emptyCaa, _, _, err := obj.LookupCAA(context.Background(), bad)
emptyCaa, _, _, _, err := obj.LookupCAA(context.Background(), bad)
test.Assert(t, len(emptyCaa) == 0, "Query returned non-empty list of CAA records")
test.AssertError(t, err, "LookupCAA should have returned an error")
}
Expand All @@ -348,11 +348,11 @@ func TestDNSLookupTXT(t *testing.T) {

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)

a, _, err := obj.LookupTXT(context.Background(), "letsencrypt.org")
a, _, _, err := obj.LookupTXT(context.Background(), "letsencrypt.org")
t.Logf("A: %v", a)
test.AssertNotError(t, err, "No message")

a, _, err = obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org")
a, _, _, err = obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org")
t.Logf("A: %v ", a)
test.AssertNotError(t, err, "No message")
test.AssertEquals(t, len(a), 1)
Expand All @@ -366,44 +366,44 @@ func TestDNSLookupHost(t *testing.T) {

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)

ip, resolvers, err := obj.LookupHost(context.Background(), "servfail.com")
ip, resolvers, _, err := obj.LookupHost(context.Background(), "servfail.com")
t.Logf("servfail.com - IP: %s, Err: %s", ip, err)
test.AssertError(t, err, "Server failure")
test.Assert(t, len(ip) == 0, "Should not have IPs")
slices.Sort(resolvers)
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})

ip, resolvers, err = obj.LookupHost(context.Background(), "nonexistent.letsencrypt.org")
ip, resolvers, _, err = obj.LookupHost(context.Background(), "nonexistent.letsencrypt.org")
t.Logf("nonexistent.letsencrypt.org - IP: %s, Err: %s", ip, err)
test.AssertError(t, err, "No valid A or AAAA records should error")
test.Assert(t, len(ip) == 0, "Should not have IPs")
slices.Sort(resolvers)
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})

// Single IPv4 address
ip, resolvers, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org")
ip, resolvers, _, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org")
t.Logf("cps.letsencrypt.org - IP: %s, Err: %s", ip, err)
test.AssertNotError(t, err, "Not an error to exist")
test.Assert(t, len(ip) == 1, "Should have IP")
slices.Sort(resolvers)
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})
ip, resolvers, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org")
ip, resolvers, _, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org")
t.Logf("cps.letsencrypt.org - IP: %s, Err: %s", ip, err)
test.AssertNotError(t, err, "Not an error to exist")
test.Assert(t, len(ip) == 1, "Should have IP")
slices.Sort(resolvers)
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})

// Single IPv6 address
ip, resolvers, err = obj.LookupHost(context.Background(), "v6.letsencrypt.org")
ip, resolvers, _, err = obj.LookupHost(context.Background(), "v6.letsencrypt.org")
t.Logf("v6.letsencrypt.org - IP: %s, Err: %s", ip, err)
test.AssertNotError(t, err, "Not an error to exist")
test.Assert(t, len(ip) == 1, "Should not have IPs")
slices.Sort(resolvers)
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})

// Both IPv6 and IPv4 address
ip, resolvers, err = obj.LookupHost(context.Background(), "dualstack.letsencrypt.org")
ip, resolvers, _, err = obj.LookupHost(context.Background(), "dualstack.letsencrypt.org")
t.Logf("dualstack.letsencrypt.org - IP: %s, Err: %s", ip, err)
test.AssertNotError(t, err, "Not an error to exist")
test.Assert(t, len(ip) == 2, "Should have 2 IPs")
Expand All @@ -415,7 +415,7 @@ func TestDNSLookupHost(t *testing.T) {
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})

// IPv6 error, IPv4 success
ip, resolvers, err = obj.LookupHost(context.Background(), "v6error.letsencrypt.org")
ip, resolvers, _, err = obj.LookupHost(context.Background(), "v6error.letsencrypt.org")
t.Logf("v6error.letsencrypt.org - IP: %s, Err: %s", ip, err)
test.AssertNotError(t, err, "Not an error to exist")
test.Assert(t, len(ip) == 1, "Should have 1 IP")
Expand All @@ -425,7 +425,7 @@ func TestDNSLookupHost(t *testing.T) {
test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"})

// IPv6 success, IPv4 error
ip, resolvers, err = obj.LookupHost(context.Background(), "v4error.letsencrypt.org")
ip, resolvers, _, err = obj.LookupHost(context.Background(), "v4error.letsencrypt.org")
t.Logf("v4error.letsencrypt.org - IP: %s, Err: %s", ip, err)
test.AssertNotError(t, err, "Not an error to exist")
test.Assert(t, len(ip) == 1, "Should have 1 IP")
Expand All @@ -437,7 +437,7 @@ func TestDNSLookupHost(t *testing.T) {
// IPv6 error, IPv4 error
// Should return both the IPv4 error (Refused) and the IPv6 error (NotImplemented)
hostname := "dualstackerror.letsencrypt.org"
ip, resolvers, err = obj.LookupHost(context.Background(), hostname)
ip, resolvers, _, err = obj.LookupHost(context.Background(), hostname)
t.Logf("%s - IP: %s, Err: %s", hostname, ip, err)
test.AssertError(t, err, "Should be an error")
test.AssertContains(t, err.Error(), "REFUSED looking up A for")
Expand All @@ -453,11 +453,11 @@ func TestDNSNXDOMAIN(t *testing.T) {
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)

hostname := "nxdomain.letsencrypt.org"
_, _, err = obj.LookupHost(context.Background(), hostname)
_, _, _, err = obj.LookupHost(context.Background(), hostname)
test.AssertContains(t, err.Error(), "NXDOMAIN looking up A for")
test.AssertContains(t, err.Error(), "NXDOMAIN looking up AAAA for")

_, _, err = obj.LookupTXT(context.Background(), hostname)
_, _, _, err = obj.LookupTXT(context.Background(), hostname)
expected := Error{dns.TypeTXT, hostname, nil, dns.RcodeNameError, nil}
test.AssertDeepEquals(t, err, expected)
}
Expand All @@ -469,7 +469,7 @@ func TestDNSLookupCAA(t *testing.T) {
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
removeIDExp := regexp.MustCompile(" id: [[:digit:]]+")

caas, resp, resolvers, err := obj.LookupCAA(context.Background(), "bracewel.net")
caas, resp, resolvers, _, err := obj.LookupCAA(context.Background(), "bracewel.net")
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas) > 0, "Should have CAA records")
test.AssertEquals(t, len(resolvers), 1)
Expand All @@ -485,22 +485,22 @@ bracewel.net. 0 IN CAA 1 issue "letsencrypt.org"
`
test.AssertEquals(t, removeIDExp.ReplaceAllString(resp, " id: XXXX"), expectedResp)

caas, resp, resolvers, err = obj.LookupCAA(context.Background(), "nonexistent.letsencrypt.org")
caas, resp, resolvers, _, err = obj.LookupCAA(context.Background(), "nonexistent.letsencrypt.org")
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas) == 0, "Shouldn't have CAA records")
test.AssertEquals(t, resolvers[0], "127.0.0.1:4053")
expectedResp = ""
test.AssertEquals(t, resp, expectedResp)

caas, resp, resolvers, err = obj.LookupCAA(context.Background(), "nxdomain.letsencrypt.org")
caas, resp, resolvers, _, err = obj.LookupCAA(context.Background(), "nxdomain.letsencrypt.org")
slices.Sort(resolvers)
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas) == 0, "Shouldn't have CAA records")
test.AssertEquals(t, resolvers[0], "127.0.0.1:4053")
expectedResp = ""
test.AssertEquals(t, resp, expectedResp)

caas, resp, resolvers, err = obj.LookupCAA(context.Background(), "cname.example.com")
caas, resp, resolvers, _, err = obj.LookupCAA(context.Background(), "cname.example.com")
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas) > 0, "Should follow CNAME to find CAA")
test.AssertEquals(t, resolvers[0], "127.0.0.1:4053")
Expand All @@ -515,7 +515,7 @@ caa.example.com. 0 IN CAA 1 issue "letsencrypt.org"
`
test.AssertEquals(t, removeIDExp.ReplaceAllString(resp, " id: XXXX"), expectedResp)

_, _, resolvers, err = obj.LookupCAA(context.Background(), "gonetld")
_, _, resolvers, _, err = obj.LookupCAA(context.Background(), "gonetld")
test.AssertError(t, err, "should fail for TLD NXDOMAIN")
test.AssertContains(t, err.Error(), "NXDOMAIN")
test.AssertEquals(t, resolvers[0], "127.0.0.1:4053")
Expand Down Expand Up @@ -678,7 +678,7 @@ func TestRetry(t *testing.T) {
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), tc.maxTries, "", blog.UseMock(), tlsConfig)
dr := testClient.(*impl)
dr.dnsClient = tc.te
_, _, err = dr.LookupTXT(context.Background(), "example.com")
_, _, _, err = dr.LookupTXT(context.Background(), "example.com")
if err == errTooManyRequests {
t.Errorf("#%d, sent more requests than the test case handles", i)
}
Expand Down Expand Up @@ -711,7 +711,7 @@ func TestRetry(t *testing.T) {
dr.dnsClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}}
ctx, cancel := context.WithCancel(context.Background())
cancel()
_, _, err = dr.LookupTXT(ctx, "example.com")
_, _, _, err = dr.LookupTXT(ctx, "example.com")
if err == nil ||
err.Error() != "DNS problem: query timed out (and was canceled) looking up TXT for example.com" {
t.Errorf("expected %s, got %s", context.Canceled, err)
Expand All @@ -720,7 +720,7 @@ func TestRetry(t *testing.T) {
dr.dnsClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}}
ctx, cancel = context.WithTimeout(context.Background(), -10*time.Hour)
defer cancel()
_, _, err = dr.LookupTXT(ctx, "example.com")
_, _, _, err = dr.LookupTXT(ctx, "example.com")
if err == nil ||
err.Error() != "DNS problem: query timed out looking up TXT for example.com" {
t.Errorf("expected %s, got %s", context.DeadlineExceeded, err)
Expand All @@ -729,7 +729,7 @@ func TestRetry(t *testing.T) {
dr.dnsClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}}
ctx, deadlineCancel := context.WithTimeout(context.Background(), -10*time.Hour)
deadlineCancel()
_, _, err = dr.LookupTXT(ctx, "example.com")
_, _, _, err = dr.LookupTXT(ctx, "example.com")
if err == nil ||
err.Error() != "DNS problem: query timed out looking up TXT for example.com" {
t.Errorf("expected %s, got %s", context.DeadlineExceeded, err)
Expand Down Expand Up @@ -829,7 +829,7 @@ func TestRotateServerOnErr(t *testing.T) {
// servers *all* queries should eventually succeed by being retried against
// server "[2606:4700:4700::1111]:53".
for range maxTries * 2 {
_, resolvers, err := client.LookupTXT(context.Background(), "example.com")
_, resolvers, _, err := client.LookupTXT(context.Background(), "example.com")
test.AssertEquals(t, len(resolvers), 1)
test.AssertEquals(t, resolvers[0], "[2606:4700:4700::1111]:53")
// Any errors are unexpected - server "[2606:4700:4700::1111]:53" should
Expand Down
Loading