Skip to content

Commit 4758132

Browse files
aarongableaarongable
committed
Refactor DNS to pass full messages up to the VA
1 parent f68edb1 commit 4758132

File tree

17 files changed

+926
-979
lines changed

17 files changed

+926
-979
lines changed

‎bdns/dns.go‎

Lines changed: 180 additions & 291 deletions
Large diffs are not rendered by default.

‎bdns/dns_test.go‎

Lines changed: 224 additions & 152 deletions
Large diffs are not rendered by default.

‎bdns/mocks.go‎

Lines changed: 12 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -3,159 +3,29 @@ package bdns
33
import (
44
"context"
55
"errors"
6-
"fmt"
7-
"net"
8-
"net/netip"
9-
"os"
106

117
"github.com/miekg/dns"
12-
13-
blog "github.com/letsencrypt/boulder/log"
148
)
159

1610
// MockClient is a mock
17-
type MockClient struct {
18-
Log blog.Logger
19-
}
11+
type MockClient struct{}
2012

2113
// LookupTXT is a mock
22-
func (mock *MockClient) LookupTXT(_ context.Context, hostname string) ([]string, ResolverAddrs, error) {
23-
// Use the example account-specific label prefix derived from
24-
// "https://example.com/acme/acct/ExampleAccount"
25-
const accountLabelPrefix = "_ujmmovf2vn55tgye._acme-challenge"
26-
27-
if hostname == accountLabelPrefix+".servfail.com" {
28-
// Mirror dns-01 servfail behaviour
29-
return nil, ResolverAddrs{"MockClient"}, fmt.Errorf("SERVFAIL")
30-
}
31-
if hostname == accountLabelPrefix+".good-dns01.com" {
32-
// Mirror dns-01 good record
33-
// base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0"
34-
// + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI"))
35-
return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil
36-
}
37-
if hostname == accountLabelPrefix+".wrong-dns01.com" {
38-
// Mirror dns-01 wrong record
39-
return []string{"a"}, ResolverAddrs{"MockClient"}, nil
40-
}
41-
if hostname == accountLabelPrefix+".wrong-many-dns01.com" {
42-
// Mirror dns-01 wrong-many record
43-
return []string{"a", "b", "c", "d", "e"}, ResolverAddrs{"MockClient"}, nil
44-
}
45-
if hostname == accountLabelPrefix+".long-dns01.com" {
46-
// Mirror dns-01 long record
47-
return []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, ResolverAddrs{"MockClient"}, nil
48-
}
49-
if hostname == accountLabelPrefix+".no-authority-dns01.com" {
50-
// Mirror dns-01 no-authority good record
51-
// base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0"
52-
// + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI"))
53-
return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil
54-
}
55-
if hostname == accountLabelPrefix+".empty-txts.com" {
56-
// Mirror dns-01 zero TXT records
57-
return []string{}, ResolverAddrs{"MockClient"}, nil
58-
}
59-
60-
if hostname == "_acme-challenge.servfail.com" {
61-
return nil, ResolverAddrs{"MockClient"}, fmt.Errorf("SERVFAIL")
62-
}
63-
if hostname == "_acme-challenge.good-dns01.com" {
64-
// base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0"
65-
// + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI"))
66-
// expected token + test account jwk thumbprint
67-
return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil
68-
}
69-
if hostname == "_acme-challenge.wrong-dns01.com" {
70-
return []string{"a"}, ResolverAddrs{"MockClient"}, nil
71-
}
72-
if hostname == "_acme-challenge.wrong-many-dns01.com" {
73-
return []string{"a", "b", "c", "d", "e"}, ResolverAddrs{"MockClient"}, nil
74-
}
75-
if hostname == "_acme-challenge.long-dns01.com" {
76-
return []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, ResolverAddrs{"MockClient"}, nil
77-
}
78-
if hostname == "_acme-challenge.no-authority-dns01.com" {
79-
// base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0"
80-
// + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI"))
81-
// expected token + test account jwk thumbprint
82-
return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil
83-
}
84-
// empty-txts.com always returns zero TXT records
85-
if hostname == "_acme-challenge.empty-txts.com" {
86-
return []string{}, ResolverAddrs{"MockClient"}, nil
87-
}
88-
89-
// Default fallback
90-
return []string{"hostname"}, ResolverAddrs{"MockClient"}, nil
14+
func (mock *MockClient) LookupTXT(_ context.Context, hostname string) (*Result[*dns.TXT], string, error) {
15+
return nil, "MockClient", errors.New("unexpected LookupTXT call on test fake")
9116
}
9217

93-
// makeTimeoutError returns a a net.OpError for which Timeout() returns true.
94-
func makeTimeoutError() *net.OpError {
95-
return &net.OpError{
96-
Err: os.NewSyscallError("ugh timeout", timeoutError{}),
97-
}
98-
}
99-
100-
type timeoutError struct{}
101-
102-
func (t timeoutError) Error() string {
103-
return "so sloooow"
104-
}
105-
func (t timeoutError) Timeout() bool {
106-
return true
18+
// LookupA is a fake
19+
func (mock *MockClient) LookupA(_ context.Context, hostname string) (*Result[*dns.A], string, error) {
20+
return nil, "MockClient", errors.New("unexpected LookupA call on test fake")
10721
}
10822

109-
// LookupHost is a mock
110-
func (mock *MockClient) LookupHost(_ context.Context, hostname string) ([]netip.Addr, ResolverAddrs, error) {
111-
if hostname == "always.invalid" ||
112-
hostname == "invalid.invalid" {
113-
return []netip.Addr{}, ResolverAddrs{"MockClient"}, nil
114-
}
115-
if hostname == "always.timeout" {
116-
return []netip.Addr{}, ResolverAddrs{"MockClient"}, &Error{dns.TypeA, "always.timeout", makeTimeoutError(), -1, nil}
117-
}
118-
if hostname == "always.error" {
119-
err := &net.OpError{
120-
Op: "read",
121-
Net: "udp",
122-
Err: errors.New("some net error"),
123-
}
124-
m := new(dns.Msg)
125-
m.SetQuestion(dns.Fqdn(hostname), dns.TypeA)
126-
m.AuthenticatedData = true
127-
m.SetEdns0(4096, false)
128-
return []netip.Addr{}, ResolverAddrs{"MockClient"}, &Error{dns.TypeA, hostname, err, -1, nil}
129-
}
130-
if hostname == "id.mismatch" {
131-
err := dns.ErrId
132-
m := new(dns.Msg)
133-
m.SetQuestion(dns.Fqdn(hostname), dns.TypeA)
134-
m.AuthenticatedData = true
135-
m.SetEdns0(4096, false)
136-
r := new(dns.Msg)
137-
record := new(dns.A)
138-
record.Hdr = dns.RR_Header{Name: dns.Fqdn(hostname), Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 0}
139-
record.A = net.ParseIP("127.0.0.1")
140-
r.Answer = append(r.Answer, record)
141-
return []netip.Addr{}, ResolverAddrs{"MockClient"}, &Error{dns.TypeA, hostname, err, -1, nil}
142-
}
143-
// dual-homed host with an IPv6 and an IPv4 address
144-
if hostname == "ipv4.and.ipv6.localhost" {
145-
return []netip.Addr{
146-
netip.MustParseAddr("::1"),
147-
netip.MustParseAddr("127.0.0.1"),
148-
}, ResolverAddrs{"MockClient"}, nil
149-
}
150-
if hostname == "ipv6.localhost" {
151-
return []netip.Addr{
152-
netip.MustParseAddr("::1"),
153-
}, ResolverAddrs{"MockClient"}, nil
154-
}
155-
return []netip.Addr{netip.MustParseAddr("127.0.0.1")}, ResolverAddrs{"MockClient"}, nil
23+
// LookupAAAA is a fake
24+
func (mock *MockClient) LookupAAAA(_ context.Context, hostname string) (*Result[*dns.AAAA], string, error) {
25+
return nil, "MockClient", errors.New("unexpected LookupAAAA call on test fake")
15626
}
15727

158-
// LookupCAA returns mock records for use in tests.
159-
func (mock *MockClient) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, string, ResolverAddrs, error) {
160-
return nil, "", ResolverAddrs{"MockClient"}, nil
28+
// LookupCAA is a fake
29+
func (mock *MockClient) LookupCAA(_ context.Context, domain string) (*Result[*dns.CAA], string, error) {
30+
return nil, "MockClient", errors.New("unexpected LookupCAA call on test fake")
16131
}

‎bdns/problem_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import (
77
"net/url"
88
"testing"
99

10-
"github.com/letsencrypt/boulder/test"
1110
"github.com/miekg/dns"
11+
12+
"github.com/letsencrypt/boulder/test"
1213
)
1314

1415
func TestError(t *testing.T) {
@@ -17,9 +18,6 @@ func TestError(t *testing.T) {
1718
expected string
1819
}{
1920
{
20-
&Error{dns.TypeA, "hostname", makeTimeoutError(), -1, nil},
21-
"DNS problem: query timed out looking up A for hostname",
22-
}, {
2321
&Error{dns.TypeMX, "hostname", &net.OpError{Err: errors.New("some net error")}, -1, nil},
2422
"DNS problem: networking error looking up MX for hostname",
2523
}, {

‎cmd/boulder-va/main.go‎

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -100,28 +100,16 @@ func main() {
100100
tlsConfig, err := c.VA.TLS.Load(scope)
101101
cmd.FailOnError(err, "tlsConfig config")
102102

103-
var resolver bdns.Client
104-
if !c.VA.DNSAllowLoopbackAddresses {
105-
resolver = bdns.New(
106-
c.VA.DNSTimeout.Duration,
107-
servers,
108-
scope,
109-
clk,
110-
c.VA.DNSTries,
111-
c.VA.UserAgent,
112-
logger,
113-
tlsConfig)
114-
} else {
115-
resolver = bdns.NewTest(
116-
c.VA.DNSTimeout.Duration,
117-
servers,
118-
scope,
119-
clk,
120-
c.VA.DNSTries,
121-
c.VA.UserAgent,
122-
logger,
123-
tlsConfig)
124-
}
103+
resolver := bdns.New(
104+
c.VA.DNSTimeout.Duration,
105+
servers,
106+
scope,
107+
clk,
108+
c.VA.DNSTries,
109+
c.VA.UserAgent,
110+
logger,
111+
tlsConfig)
112+
125113
var remotes []va.RemoteVA
126114
if len(c.VA.RemoteVAs) > 0 {
127115
for _, rva := range c.VA.RemoteVAs {
@@ -155,6 +143,7 @@ func main() {
155143
"",
156144
iana.IsReservedAddr,
157145
c.VA.SlowRemoteTimeout.Duration,
146+
c.VA.DNSAllowLoopbackAddresses,
158147
)
159148
cmd.FailOnError(err, "Unable to create VA server")
160149

‎cmd/remoteva/main.go‎

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -106,28 +106,15 @@ func main() {
106106
tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven
107107
}
108108

109-
var resolver bdns.Client
110-
if !c.RVA.DNSAllowLoopbackAddresses {
111-
resolver = bdns.New(
112-
c.RVA.DNSTimeout.Duration,
113-
servers,
114-
scope,
115-
clk,
116-
c.RVA.DNSTries,
117-
c.RVA.UserAgent,
118-
logger,
119-
tlsConfig)
120-
} else {
121-
resolver = bdns.NewTest(
122-
c.RVA.DNSTimeout.Duration,
123-
servers,
124-
scope,
125-
clk,
126-
c.RVA.DNSTries,
127-
c.RVA.UserAgent,
128-
logger,
129-
tlsConfig)
130-
}
109+
resolver := bdns.New(
110+
c.RVA.DNSTimeout.Duration,
111+
servers,
112+
scope,
113+
clk,
114+
c.RVA.DNSTries,
115+
c.RVA.UserAgent,
116+
logger,
117+
tlsConfig)
131118

132119
vai, err := va.NewValidationAuthorityImpl(
133120
resolver,
@@ -142,6 +129,7 @@ func main() {
142129
c.RVA.RIR,
143130
iana.IsReservedAddr,
144131
0,
132+
c.RVA.DNSAllowLoopbackAddresses,
145133
)
146134
cmd.FailOnError(err, "Unable to create Remote-VA server")
147135

‎va/caa.go‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,13 @@ func (va *ValidationAuthorityImpl) checkCAA(
130130
return errors.New("expected validationMethod or accountURIID not provided to checkCAA")
131131
}
132132

133-
foundAt, valid, response, err := va.checkCAARecords(ctx, ident, params)
133+
foundAt, valid, err := va.checkCAARecords(ctx, ident, params)
134134
if err != nil {
135135
return berrors.DNSError("%s", err)
136136
}
137137

138-
va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %d, Challenge: %s, Valid for issuance: %t, Found at: %q] Response=%q",
139-
ident.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt, response)
138+
va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %d, Challenge: %s, Valid for issuance: %t, Found at: %q]",
139+
ident.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt)
140140
if !valid {
141141
return berrors.CAAError("CAA record for %s prevents issuance", foundAt)
142142
}
@@ -153,8 +153,7 @@ type caaResult struct {
153153
issue []*dns.CAA
154154
issuewild []*dns.CAA
155155
criticalUnknown bool
156-
dig string
157-
resolvers bdns.ResolverAddrs
156+
resolver string
158157
err error
159158
}
160159

@@ -213,14 +212,17 @@ func (va *ValidationAuthorityImpl) parallelCAALookup(ctx context.Context, name s
213212
// Start the concurrent DNS lookup.
214213
wg.Add(1)
215214
go func(name string, r *caaResult) {
215+
defer wg.Done()
216216
r.name = name
217-
var records []*dns.CAA
218-
records, r.dig, r.resolvers, r.err = va.dnsClient.LookupCAA(ctx, name)
219-
if len(records) > 0 {
217+
var records *bdns.Result[*dns.CAA]
218+
records, r.resolver, r.err = va.dnsClient.LookupCAA(ctx, name)
219+
if r.err != nil {
220+
return
221+
}
222+
if len(records.Final) > 0 {
220223
r.present = true
221224
}
222-
r.issue, r.issuewild, r.criticalUnknown = filterCAA(records)
223-
wg.Done()
225+
r.issue, r.issuewild, r.criticalUnknown = filterCAA(records.Final)
224226
}(strings.Join(labels[i:], "."), &results[i])
225227
}
226228

@@ -276,12 +278,12 @@ func (va *ValidationAuthorityImpl) getCAA(ctx context.Context, hostname string)
276278
// which name (i.e. FQDN or parent thereof) CAA records were found, if any. The
277279
// second is a bool indicating whether issuance for the identifier is valid. The
278280
// unmodified *dns.CAA records that were processed/filtered are returned as the
279-
// third argument. Any errors encountered are returned as the fourth return
281+
// third argument. Any errors encountered are returned as the fourth return
280282
// value (or nil).
281283
func (va *ValidationAuthorityImpl) checkCAARecords(
282284
ctx context.Context,
283285
ident identifier.ACMEIdentifier,
284-
params *caaParams) (string, bool, string, error) {
286+
params *caaParams) (string, bool, error) {
285287
hostname := strings.ToLower(ident.Value)
286288
// If this is a wildcard name, remove the prefix
287289
var wildcard bool
@@ -291,14 +293,10 @@ func (va *ValidationAuthorityImpl) checkCAARecords(
291293
}
292294
caaSet, err := va.getCAA(ctx, hostname)
293295
if err != nil {
294-
return "", false, "", err
295-
}
296-
raw := ""
297-
if caaSet != nil {
298-
raw = caaSet.dig
296+
return "", false, err
299297
}
300298
valid, foundAt := va.validateCAA(caaSet, wildcard, params)
301-
return foundAt, valid, raw, nil
299+
return foundAt, valid, nil
302300
}
303301

304302
// validateCAA checks a provided *caaResult. When the wildcard argument is true

0 commit comments

Comments
 (0)