From 4758132a6b22b788dfe7b3262008c9be5ca844c8 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Fri, 5 Dec 2025 15:43:47 -0800 Subject: [PATCH 1/5] Refactor DNS to pass full messages up to the VA --- bdns/dns.go | 471 ++++++++++++++++------------------------- bdns/dns_test.go | 376 +++++++++++++++++++------------- bdns/mocks.go | 154 ++------------ bdns/problem_test.go | 6 +- cmd/boulder-va/main.go | 33 +-- cmd/remoteva/main.go | 32 +-- va/caa.go | 34 ++- va/caa_test.go | 212 ++++++++----------- va/dns.go | 88 ++++++-- va/dns_account_test.go | 23 +- va/dns_test.go | 104 ++++++++- va/http.go | 7 +- va/http_test.go | 145 ++++++++----- va/tlsalpn.go | 3 +- va/tlsalpn_test.go | 57 +++-- va/va.go | 43 ++-- va/va_test.go | 117 +++++----- 17 files changed, 926 insertions(+), 979 deletions(-) diff --git a/bdns/dns.go b/bdns/dns.go index 5fee207b8f9..36adbbee0bc 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -8,11 +8,8 @@ import ( "io" "net" "net/http" - "net/netip" - "slices" "strconv" "strings" - "sync" "time" "github.com/jmhodges/clock" @@ -20,32 +17,57 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/letsencrypt/boulder/iana" blog "github.com/letsencrypt/boulder/log" "github.com/letsencrypt/boulder/metrics" ) -// ResolverAddrs contains DNS resolver(s) that were chosen to perform a -// validation request or CAA recheck. A ResolverAddr will be in the form of -// host:port, A:host:port, or AAAA:host:port depending on which type of lookup -// was done. -type ResolverAddrs []string +// Result is a wrapper around miekg/dns.Msg, but with all Resource Records from +// the Answer section which match the parameterized record type already pulled +// out for convenient access. +type Result[R dns.RR] struct { + *dns.Msg + CNames []*dns.CNAME + Final []R +} + +// resultFromMsg returns a Result whose CNames and Final fields are populated +// from the underlying Msg's Answer field. +func resultFromMsg[R dns.RR](m *dns.Msg) *Result[R] { + var cnames []*dns.CNAME + var final []R + for _, rr := range m.Answer { + if a, ok := rr.(R); ok { + final = append(final, a) + } else if a, ok := rr.(*dns.CNAME); ok { + cnames = append(cnames, a) + } + } + + return &Result[R]{ + Msg: m, + CNames: cnames, + Final: final, + } +} -// Client queries for DNS records +// Client can make A, AAAA, CAA, and TXT queries. The second return value of +// each method is the address of the resolver used to conduct the query, and +// should be populated even when returning an error. 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) + LookupA(context.Context, string) (*Result[*dns.A], string, error) + LookupAAAA(context.Context, string) (*Result[*dns.AAAA], string, error) + LookupCAA(context.Context, string) (*Result[*dns.CAA], string, error) + LookupTXT(context.Context, string) (*Result[*dns.TXT], string, error) } -// impl represents a client that talks to an external resolver +// impl implements the Client interface via an underlying DNS exchanger. It +// rotates queries across multiple resolvers and tracks a variety of metrics. type impl struct { - dnsClient exchanger - servers ServerProvider - allowRestrictedAddresses bool - maxTries int - clk clock.Clock - log blog.Logger + exchanger exchanger + servers ServerProvider + maxTries int + clk clock.Clock + log blog.Logger queryTime *prometheus.HistogramVec totalLookupTime *prometheus.HistogramVec @@ -54,15 +76,9 @@ type impl struct { var _ Client = &impl{} -type exchanger interface { - Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) -} - -// New constructs a new DNS resolver object that utilizes the -// provided list of DNS servers for resolution. -// -// `tlsConfig` is the configuration used for outbound DoH queries, -// if applicable. +// New constructs a new DNS resolver object that utilizes the provided list of +// DNS servers for resolution, and the provided tlsConfig to speak DoH to those +// servers. func New( readTimeout time.Duration, servers ServerProvider, @@ -73,18 +89,15 @@ func New( log blog.Logger, tlsConfig *tls.Config, ) Client { - var client exchanger - - // Clone the default transport because it comes with various settings - // that we like, which are different from the zero value of an - // `http.Transport`. + // Clone the default transport because it comes with various settings that we + // like, which are different from the zero value of an `http.Transport`. Then + // set it to force HTTP/2, because Unbound will reject non-HTTP/2 DoH + // requests. transport := http.DefaultTransport.(*http.Transport).Clone() transport.TLSClientConfig = tlsConfig - // The default transport already sets this field, but it isn't - // documented that it will always be set. Set it again to be sure, - // because Unbound will reject non-HTTP/2 DoH requests. transport.ForceAttemptHTTP2 = true - client = &dohExchanger{ + + exchanger := &dohExchanger{ clk: clk, hc: http.Client{ Timeout: readTimeout, @@ -107,100 +120,90 @@ func New( Help: "Time taken to perform a DNS lookup, including all retried queries", Buckets: metrics.InternetFacingBuckets, }, - []string{"qtype", "result", "retries", "resolver"}, + []string{"qtype", "result", "resolver", "attempts"}, ) timeoutCounter := promauto.With(stats).NewCounterVec( prometheus.CounterOpts{ Name: "dns_timeout", Help: "Counter of various types of DNS query timeouts", }, - []string{"qtype", "type", "resolver", "isTLD"}, + []string{"qtype", "result", "resolver", "isTLD"}, ) - return &impl{ - dnsClient: client, - servers: servers, - allowRestrictedAddresses: false, - maxTries: maxTries, - clk: clk, - queryTime: queryTime, - totalLookupTime: totalLookupTime, - timeoutCounter: timeoutCounter, - log: log, + + if maxTries < 1 { + // Allowing negative or zero total attempts makes no sense, so default to 1. + maxTries = 1 } -} -// NewTest constructs a new DNS resolver object that utilizes the -// provided list of DNS servers for resolution and will allow loopback addresses. -// This constructor should *only* be called from tests (unit or integration). -func NewTest( - readTimeout time.Duration, - servers ServerProvider, - stats prometheus.Registerer, - clk clock.Clock, - maxTries int, - userAgent string, - log blog.Logger, - tlsConfig *tls.Config, -) Client { - resolver := New(readTimeout, servers, stats, clk, maxTries, userAgent, log, tlsConfig) - resolver.(*impl).allowRestrictedAddresses = true - return resolver + return &impl{ + exchanger: exchanger, + servers: servers, + maxTries: maxTries, + clk: clk, + queryTime: queryTime, + totalLookupTime: totalLookupTime, + timeoutCounter: timeoutCounter, + log: log, + } } -// exchangeOne performs a single DNS exchange with a randomly chosen server -// out of the server list, returning the response, time, and error (if any). -// We assume that the upstream resolver requests and validates DNSSEC records -// itself. -func (dnsClient *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) (resp *dns.Msg, resolver string, err error) { - m := new(dns.Msg) +// exchangeOne performs a single DNS exchange with a randomly chosen server out +// of the server list, returning the response, resolver used, and error (if +// any). If a response received indicates that the resolver encountered an error +// (such as an expired DNSSEC signature), that is converted into an error and +// returned. +func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) (*dns.Msg, string, error) { + req := new(dns.Msg) // Set question type - m.SetQuestion(dns.Fqdn(hostname), qtype) + req.SetQuestion(dns.Fqdn(hostname), qtype) // Set the AD bit in the query header so that the resolver knows that // we are interested in this bit in the response header. If this isn't // set the AD bit in the response is useless (RFC 6840 Section 5.7). // This has no security implications, it simply allows us to gather // metrics about the percentage of responses that are secured with // DNSSEC. - m.AuthenticatedData = true + req.AuthenticatedData = true // Tell the resolver that we're willing to receive responses up to 4096 bytes. // This happens sometimes when there are a very large number of CAA records // present. - m.SetEdns0(4096, false) + req.SetEdns0(4096, false) - servers, err := dnsClient.servers.Addrs() + var resp *dns.Msg + + servers, err := c.servers.Addrs() if err != nil { return nil, "", fmt.Errorf("failed to list DNS servers: %w", err) } - chosenServerIndex := 0 - chosenServer := servers[chosenServerIndex] - resolver = chosenServer - - // Strip off the IP address part of the server address because - // we talk to the same server on multiple ports, and don't want - // to blow up the cardinality. - chosenServerIP, _, err := net.SplitHostPort(chosenServer) - if err != nil { - return - } - start := dnsClient.clk.Now() - client := dnsClient.dnsClient + // Prepare to increment a latency metric no matter whether we succeed or fail. + // The deferred function closes over result, chosenServerIP, and tries, which + // are all modified in the loop below. + start := c.clk.Now() qtypeStr := dns.TypeToString[qtype] - tries := 1 + result := "failed" + chosenServerIP := "" + tries := 0 defer func() { - result := "failed" if resp != nil { result = dns.RcodeToString[resp.Rcode] } - dnsClient.totalLookupTime.With(prometheus.Labels{ + c.totalLookupTime.With(prometheus.Labels{ "qtype": qtypeStr, "result": result, - "retries": strconv.Itoa(tries), "resolver": chosenServerIP, - }).Observe(dnsClient.clk.Since(start).Seconds()) + "attempts": strconv.Itoa(tries), + }).Observe(c.clk.Since(start).Seconds()) }() - for { - ch := make(chan dnsResp, 1) + + type dnsRes struct { + resp *dns.Msg + err error + } + ch := make(chan dnsRes, 1) + + for i := range c.maxTries { + tries = i + 1 + chosenServer := servers[i%len(servers)] // Strip off the IP address part of the server address because // we talk to the same server on multiple ports, and don't want @@ -209,224 +212,99 @@ func (dnsClient *impl) exchangeOne(ctx context.Context, hostname string, qtype u // and ensures that chosenServer can't be a bare port, e.g. ":1337" chosenServerIP, _, err = net.SplitHostPort(chosenServer) if err != nil { - return + return nil, "", err } go func() { - rsp, rtt, err := client.Exchange(m, chosenServer) + resp, rtt, err := c.exchanger.Exchange(req, chosenServer) result := "failed" - if rsp != nil { - result = dns.RcodeToString[rsp.Rcode] + if resp != nil { + result = dns.RcodeToString[resp.Rcode] } if err != nil { - dnsClient.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]", - chosenServer, - hostname, - qtypeStr, - err) + c.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]", chosenServer, hostname, qtypeStr, err) } - dnsClient.queryTime.With(prometheus.Labels{ + c.queryTime.With(prometheus.Labels{ "qtype": qtypeStr, "result": result, "resolver": chosenServerIP, }).Observe(rtt.Seconds()) - ch <- dnsResp{m: rsp, err: err} + ch <- dnsRes{resp: resp, err: err} }() select { case <-ctx.Done(): - if ctx.Err() == context.DeadlineExceeded { - dnsClient.timeoutCounter.With(prometheus.Labels{ - "qtype": qtypeStr, - "type": "deadline exceeded", - "resolver": chosenServerIP, - "isTLD": isTLD(hostname), - }).Inc() - } else if ctx.Err() == context.Canceled { - dnsClient.timeoutCounter.With(prometheus.Labels{ - "qtype": qtypeStr, - "type": "canceled", - "resolver": chosenServerIP, - "isTLD": isTLD(hostname), - }).Inc() - } else { - dnsClient.timeoutCounter.With(prometheus.Labels{ - "qtype": qtypeStr, - "type": "unknown", - "resolver": chosenServerIP, - }).Inc() + switch ctx.Err() { + case context.DeadlineExceeded: + result = "deadline exceeded" + case context.Canceled: + result = "canceled" + default: + result = "unknown" } - err = ctx.Err() - return + c.timeoutCounter.With(prometheus.Labels{ + "qtype": qtypeStr, + "result": result, + "resolver": chosenServerIP, + "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), + }).Inc() + return nil, "", ctx.Err() case r := <-ch: if r.err != nil { - var isRetryable bool - // Check if the error is a timeout error. Network errors - // that can timeout implement the net.Error interface. + // Check if the error is a timeout error, which we want to retry. + // Network errors that can timeout implement the net.Error interface. var netErr net.Error - isRetryable = errors.As(r.err, &netErr) && netErr.Timeout() - hasRetriesLeft := tries < dnsClient.maxTries + isRetryable := errors.As(r.err, &netErr) && netErr.Timeout() + hasRetriesLeft := tries < c.maxTries if isRetryable && hasRetriesLeft { - tries++ - // Chose a new server to retry the query with by incrementing the - // chosen server index modulo the number of servers. This ensures that - // if one dns server isn't available we retry with the next in the - // list. - chosenServerIndex = (chosenServerIndex + 1) % len(servers) - chosenServer = servers[chosenServerIndex] - resolver = chosenServer continue } else if isRetryable && !hasRetriesLeft { - dnsClient.timeoutCounter.With(prometheus.Labels{ + c.timeoutCounter.With(prometheus.Labels{ "qtype": qtypeStr, - "type": "out of retries", + "result": "out of retries", "resolver": chosenServerIP, - "isTLD": isTLD(hostname), + "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), }).Inc() } } - resp, err = r.m, r.err - return - } - } -} - -// isTLD returns a simplified view of whether something is a TLD: does it have -// any dots in it? This returns true or false as a string, and is meant solely -// for Prometheus metrics. -func isTLD(hostname string) string { - if strings.Contains(hostname, ".") { - return "false" - } else { - return "true" - } -} - -type dnsResp struct { - m *dns.Msg - err error -} - -// 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) { - 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 - } - for _, answer := range r.Answer { - if answer.Header().Rrtype == dnsType { - if txtRec, ok := answer.(*dns.TXT); ok { - txt = append(txt, strings.Join(txtRec.Txt, "")) - } + // This is either a success or a non-retryable error; return either way. + return r.resp, chosenServer, r.err } } - return txt, ResolverAddrs{resolver}, err + // It's impossible to get past the bottom of the loop: on the last attempt + // (when tries == c.maxTries), all paths lead to a return from inside the loop. + return nil, "", errors.New("unexpected loop escape in exchangeOne") } -func (dnsClient *impl) lookupIP(ctx context.Context, hostname string, ipType uint16) ([]dns.RR, string, error) { - resp, resolver, err := dnsClient.exchangeOne(ctx, hostname, ipType) - switch ipType { - case dns.TypeA: - if resolver != "" { - resolver = "A:" + resolver - } - case dns.TypeAAAA: - if resolver != "" { - resolver = "AAAA:" + resolver - } - } - errWrap := wrapErr(ipType, hostname, resp, err) - if errWrap != nil { - return nil, resolver, errWrap - } - return resp.Answer, resolver, 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) { - var recordsA, recordsAAAA []dns.RR - var errA, errAAAA error - var resolverA, resolverAAAA string - var wg sync.WaitGroup - - wg.Go(func() { - recordsA, resolverA, errA = dnsClient.lookupIP(ctx, hostname, dns.TypeA) - }) - wg.Go(func() { - recordsAAAA, resolverAAAA, errAAAA = dnsClient.lookupIP(ctx, hostname, dns.TypeAAAA) - }) - wg.Wait() - - resolvers := ResolverAddrs{resolverA, resolverAAAA} - resolvers = slices.DeleteFunc(resolvers, func(a string) bool { - return a == "" - }) - - var addrsA []netip.Addr - if errA == nil { - for _, answer := range recordsA { - if answer.Header().Rrtype == dns.TypeA { - a, ok := answer.(*dns.A) - if ok && a.A.To4() != nil { - netIP, ok := netip.AddrFromSlice(a.A) - if ok && (iana.IsReservedAddr(netIP) == nil || dnsClient.allowRestrictedAddresses) { - addrsA = append(addrsA, netIP) - } - } - } - } - if len(addrsA) == 0 { - errA = fmt.Errorf("no valid A records found for %s", hostname) - } +// LookupA sends a DNS query to find all A records associated with the provided +// hostname. +func (c *impl) LookupA(ctx context.Context, hostname string) (*Result[*dns.A], string, error) { + resp, resolver, err := c.exchangeOne(ctx, hostname, dns.TypeA) + err = wrapErr(dns.TypeA, hostname, resp, err) + if err != nil { + return nil, resolver, err } - var addrsAAAA []netip.Addr - if errAAAA == nil { - for _, answer := range recordsAAAA { - if answer.Header().Rrtype == dns.TypeAAAA { - aaaa, ok := answer.(*dns.AAAA) - if ok && aaaa.AAAA.To16() != nil { - netIP, ok := netip.AddrFromSlice(aaaa.AAAA) - if ok && (iana.IsReservedAddr(netIP) == nil || dnsClient.allowRestrictedAddresses) { - addrsAAAA = append(addrsAAAA, netIP) - } - } - } - } - if len(addrsAAAA) == 0 { - errAAAA = fmt.Errorf("no valid AAAA records found for %s", hostname) - } - } + return resultFromMsg[*dns.A](resp), resolver, wrapErr(dns.TypeA, hostname, resp, err) +} - if errA != nil && errAAAA != nil { - // Construct a new error from both underlying errors. We can only use %w for - // one of them, because the go error unwrapping protocol doesn't support - // 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) +// LookupAAAA sends a DNS query to find all AAAA records associated with the +// provided hostname. +func (c *impl) LookupAAAA(ctx context.Context, hostname string) (*Result[*dns.AAAA], string, error) { + resp, resolver, err := c.exchangeOne(ctx, hostname, dns.TypeAAAA) + err = wrapErr(dns.TypeAAAA, hostname, resp, err) + if err != nil { + return nil, resolver, err } - return append(addrsA, addrsAAAA...), resolvers, nil + return resultFromMsg[*dns.AAAA](resp), resolver, nil } -// 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) { - dnsType := dns.TypeCAA - r, resolver, err := dnsClient.exchangeOne(ctx, hostname, dnsType) +// LookupCAA sends a DNS query to find all CAA records associated with the +// provided hostname. +func (c *impl) LookupCAA(ctx context.Context, hostname string) (*Result[*dns.CAA], string, error) { + resp, resolver, err := c.exchangeOne(ctx, hostname, dns.TypeCAA) // Special case: when checking CAA for non-TLD names, treat NXDOMAIN as a // successful response containing an empty set of records. This can come up in @@ -434,28 +312,39 @@ func (dnsClient *impl) LookupCAA(ctx context.Context, hostname string) ([]*dns.C // for DNS-01 challenge) and then removed after validation but before CAA // 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 + if err == nil && resp.Rcode == dns.RcodeNameError && strings.Contains(hostname, ".") { + return resultFromMsg[*dns.CAA](resp), resolver, nil } - errWrap := wrapErr(dnsType, hostname, r, err) - if errWrap != nil { - return nil, "", ResolverAddrs{resolver}, errWrap + err = wrapErr(dns.TypeCAA, hostname, resp, err) + if err != nil { + return nil, resolver, err } - var CAAs []*dns.CAA - for _, answer := range r.Answer { - if caaR, ok := answer.(*dns.CAA); ok { - CAAs = append(CAAs, caaR) - } - } - var response string - if len(CAAs) > 0 { - response = r.String() + return resultFromMsg[*dns.CAA](resp), resolver, nil +} + +// LookupTXT sends a DNS query to find all TXT records associated with the +// provided hostname. +func (c *impl) LookupTXT(ctx context.Context, hostname string) (*Result[*dns.TXT], string, error) { + resp, resolver, err := c.exchangeOne(ctx, hostname, dns.TypeTXT) + err = wrapErr(dns.TypeTXT, hostname, resp, err) + if err != nil { + return nil, resolver, err } - return CAAs, response, ResolverAddrs{resolver}, nil + + return resultFromMsg[*dns.TXT](resp), resolver, nil +} + +// exchanger represents an underlying DNS client. This interface exists solely +// so that its implementation can be swapped out in unit tests. +type exchanger interface { + Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) } +// dohExchanger implements the exchanger interface. It routes all of its DNS +// queries over DoH, wrapping the request with the appropriate headers and +// unwrapping the response. type dohExchanger struct { clk clock.Clock hc http.Client diff --git a/bdns/dns_test.go b/bdns/dns_test.go index f53abb19ff7..bf1d02ee3e4 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -10,11 +10,9 @@ import ( "log" "net" "net/http" - "net/netip" "net/url" "os" "regexp" - "slices" "strings" "sync" "testing" @@ -287,14 +285,20 @@ 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") - test.AssertEquals(t, len(resolvers), 0) + _, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org") + test.AssertEquals(t, resolver, "") test.AssertError(t, err, "No servers") - _, _, err = obj.LookupTXT(context.Background(), "letsencrypt.org") + _, resolver, err = obj.LookupAAAA(context.Background(), "letsencrypt.org") + test.AssertEquals(t, resolver, "") + test.AssertError(t, err, "No servers") + + _, resolver, err = obj.LookupTXT(context.Background(), "letsencrypt.org") + test.AssertEquals(t, resolver, "") test.AssertError(t, err, "No servers") - _, _, _, err = obj.LookupCAA(context.Background(), "letsencrypt.org") + _, resolver, err = obj.LookupCAA(context.Background(), "letsencrypt.org") + test.AssertEquals(t, resolver, "") test.AssertError(t, err, "No servers") } @@ -304,11 +308,9 @@ 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") - 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"}) + _, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "No message") + test.AssertEquals(t, resolver, "127.0.0.1:4053") } func TestDNSDuplicateServers(t *testing.T) { @@ -317,11 +319,9 @@ 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") - 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"}) + _, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "No message") + test.AssertEquals(t, resolver, "127.0.0.1:4053") } func TestDNSServFail(t *testing.T) { @@ -331,15 +331,17 @@ 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(), "servfail.com") test.AssertError(t, err, "LookupTXT didn't return an error") - _, _, err = obj.LookupHost(context.Background(), bad) - test.AssertError(t, err, "LookupHost didn't return an error") + _, _, err = obj.LookupA(context.Background(), bad) + test.AssertError(t, err, "LookupA didn't return an error") - 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") + _, _, err = obj.LookupAAAA(context.Background(), bad) + test.AssertError(t, err, "LookupAAAA didn't return an error") + + _, _, err = obj.LookupCAA(context.Background(), bad) + test.AssertError(t, err, "LookupCAA didn't return an error") } func TestDNSLookupTXT(t *testing.T) { @@ -348,113 +350,196 @@ 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") - t.Logf("A: %v", a) + _, _, err = obj.LookupTXT(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "No message") - a, _, err = obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org") - t.Logf("A: %v ", a) + txt, _, err := obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org") test.AssertNotError(t, err, "No message") - test.AssertEquals(t, len(a), 1) - test.AssertEquals(t, a[0], "abc") + test.AssertEquals(t, len(txt.Final), 1) + test.AssertEquals(t, strings.Join(txt.Final[0].Txt, ""), "abc") } -// TODO(#8213): Convert this to a table test. -func TestDNSLookupHost(t *testing.T) { +func TestDNSLookupA(t *testing.T) { staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr}) test.AssertNotError(t, err, "Got error creating StaticProvider") obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig) - 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") - 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") - 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") - 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") - 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") - 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") - expected := netip.MustParseAddr("64.112.117.1") - test.Assert(t, ip[0] == expected, "wrong ipv4 address") - expected = netip.MustParseAddr("2602:80a:6000:abad:cafe::1") - test.Assert(t, ip[1] == expected, "wrong ipv6 address") - slices.Sort(resolvers) - 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") - 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") - expected = netip.MustParseAddr("64.112.117.1") - test.Assert(t, ip[0] == expected, "wrong ipv4 address") - slices.Sort(resolvers) - 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") - 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") - expected = netip.MustParseAddr("2602:80a:6000:abad:cafe::1") - test.Assert(t, ip[0] == expected, "wrong ipv6 address") - slices.Sort(resolvers) - test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"}) - - // 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) - 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") - test.AssertContains(t, err.Error(), "NOTIMP looking up AAAA for") - slices.Sort(resolvers) - test.AssertDeepEquals(t, resolvers, ResolverAddrs{"A:127.0.0.1:4053", "AAAA:127.0.0.1:4053"}) + for _, tc := range []struct { + name string + hostname string + wantIPs []net.IP + wantError string + }{ + { + name: "SERVFAIL", + hostname: "servfail.com", + wantError: "SERVFAIL looking up A for servfail.com", + }, + { + name: "No Records", + hostname: "nonexistent.letsencrypt.org", + wantIPs: nil, + }, + { + name: "Single IPv4", + hostname: "cps.letsencrypt.org", + wantIPs: []net.IP{net.ParseIP("64.112.117.1")}, + }, + { + name: "Single IPv6", + hostname: "v6.letsencrypt.org", + wantIPs: nil, + }, + { + name: "Both IPv6 and IPv4", + hostname: "dualstack.letsencrypt.org", + wantIPs: []net.IP{net.ParseIP("64.112.117.1")}, + }, + { + name: "IPv6 error and IPv4 success", + hostname: "v6error.letsencrypt.org", + wantIPs: []net.IP{net.ParseIP("64.112.117.1")}, + }, + { + name: "IPv6 success and IPv4 error", + hostname: "v4error.letsencrypt.org", + wantError: "NOTIMP looking up A for v4error.letsencrypt.org", + }, + { + name: "Both IPv6 and IPv4 error", + hostname: "dualstackerror.letsencrypt.org", + wantError: "REFUSED looking up A for dualstackerror.letsencrypt.org", + }, + } { + t.Run(tc.name, func(t *testing.T) { + res, resolver, err := obj.LookupA(context.Background(), tc.hostname) + + wantResolver := "127.0.0.1:4053" + if resolver != wantResolver { + t.Errorf("LookupA(%s) used resolver %q, but want %q", tc.hostname, resolver, wantResolver) + } + + if tc.wantError != "" { + if err == nil { + t.Fatalf("LookupA(%s) = success, but want error %q", tc.hostname, tc.wantError) + } + if !strings.Contains(err.Error(), tc.wantError) { + t.Errorf("LookupA(%s) = %q, but want error %q", tc.hostname, err, tc.wantError) + } + } else { + if err != nil { + t.Fatalf("LookupA(%s) = %q, but want success", tc.hostname, err) + } + if len(res.Final) != len(tc.wantIPs) { + t.Fatalf("LookupA(%s) retuned %d addrs, but want %d", tc.hostname, len(res.Final), len(tc.wantIPs)) + } + for i := range len(tc.wantIPs) { + if !res.Final[i].A.Equal(tc.wantIPs[i]) { + t.Errorf("LookupA(%s) = %s, but want %s", tc.hostname, res.Final[i].A, tc.wantIPs[i]) + } + } + } + }) + } } -func TestDNSNXDOMAIN(t *testing.T) { +func TestDNSLookupAAAA(t *testing.T) { staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr}) test.AssertNotError(t, err, "Got error creating StaticProvider") obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig) + for _, tc := range []struct { + name string + hostname string + wantIPs []net.IP + wantError string + }{ + { + name: "SERVFAIL", + hostname: "servfail.com", + wantError: "SERVFAIL looking up AAAA for servfail.com", + }, + { + name: "No Records", + hostname: "nonexistent.letsencrypt.org", + wantIPs: nil, + }, + { + name: "Single IPv4", + hostname: "cps.letsencrypt.org", + wantIPs: nil, + }, + { + name: "Single IPv6", + hostname: "v6.letsencrypt.org", + wantIPs: []net.IP{net.ParseIP("2602:80a:6000:abad:cafe::1")}, + }, + { + name: "Both IPv6 and IPv4", + hostname: "dualstack.letsencrypt.org", + wantIPs: []net.IP{net.ParseIP("2602:80a:6000:abad:cafe::1")}, + }, + { + name: "IPv6 error and IPv4 success", + hostname: "v6error.letsencrypt.org", + wantError: "NOTIMP looking up AAAA for v6error.letsencrypt.org", + }, + { + name: "IPv6 success and IPv4 error", + hostname: "v4error.letsencrypt.org", + wantIPs: []net.IP{net.ParseIP("2602:80a:6000:abad:cafe::1")}, + }, + { + name: "Both IPv6 and IPv4 error", + hostname: "dualstackerror.letsencrypt.org", + wantError: "NOTIMP looking up AAAA for dualstackerror.letsencrypt.org", + }, + } { + t.Run(tc.name, func(t *testing.T) { + res, resolver, err := obj.LookupAAAA(context.Background(), tc.hostname) + + wantResolver := "127.0.0.1:4053" + if resolver != wantResolver { + t.Errorf("LookupA(%s) used resolver %q, but want %q", tc.hostname, resolver, wantResolver) + } + + if tc.wantError != "" { + if err == nil { + t.Fatalf("LookupA(%s) = success, but want error %q", tc.hostname, tc.wantError) + } + if !strings.Contains(err.Error(), tc.wantError) { + t.Errorf("LookupA(%s) = %q, but want error %q", tc.hostname, err, tc.wantError) + } + } else { + if err != nil { + t.Fatalf("LookupA(%s) = %q, but want success", tc.hostname, err) + } + if len(res.Final) != len(tc.wantIPs) { + t.Fatalf("LookupA(%s) retuned %d addrs, but want %d", tc.hostname, len(res.Final), len(tc.wantIPs)) + } + for i := range len(tc.wantIPs) { + if !res.Final[i].AAAA.Equal(tc.wantIPs[i]) { + t.Errorf("LookupA(%s) = %s, but want %s", tc.hostname, res.Final[i].AAAA, tc.wantIPs[i]) + } + } + } + }) + } +} + +func TestDNSNXDOMAIN(t *testing.T) { + staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr}) + test.AssertNotError(t, err, "Got error creating StaticProvider") + + 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.LookupA(context.Background(), hostname) test.AssertContains(t, err.Error(), "NXDOMAIN looking up A for") + + _, _, err = obj.LookupAAAA(context.Background(), hostname) test.AssertContains(t, err.Error(), "NXDOMAIN looking up AAAA for") _, _, err = obj.LookupTXT(context.Background(), hostname) @@ -469,11 +554,10 @@ 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, resolver, 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) - test.AssertDeepEquals(t, resolvers, ResolverAddrs{"127.0.0.1:4053"}) + test.Assert(t, len(caas.Final) > 0, "Should have CAA records") + test.AssertEquals(t, resolver, "127.0.0.1:4053") expectedResp := `;; opcode: QUERY, status: NOERROR, id: XXXX ;; flags: qr rd; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0 @@ -483,27 +567,22 @@ func TestDNSLookupCAA(t *testing.T) { ;; ANSWER SECTION: bracewel.net. 0 IN CAA 1 issue "letsencrypt.org" ` - test.AssertEquals(t, removeIDExp.ReplaceAllString(resp, " id: XXXX"), expectedResp) + test.AssertEquals(t, removeIDExp.ReplaceAllString(caas.String(), " id: XXXX"), expectedResp) - caas, resp, resolvers, err = obj.LookupCAA(context.Background(), "nonexistent.letsencrypt.org") + caas, resolver, 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) + test.Assert(t, len(caas.Final) == 0, "Shouldn't have CAA records") + test.AssertEquals(t, resolver, "127.0.0.1:4053") - caas, resp, resolvers, err = obj.LookupCAA(context.Background(), "nxdomain.letsencrypt.org") - slices.Sort(resolvers) + caas, resolver, err = obj.LookupCAA(context.Background(), "nxdomain.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) + test.Assert(t, len(caas.Final) == 0, "Shouldn't have CAA records") + test.AssertEquals(t, resolver, "127.0.0.1:4053") - caas, resp, resolvers, err = obj.LookupCAA(context.Background(), "cname.example.com") + caas, resolver, 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") + test.Assert(t, len(caas.Final) > 0, "Should follow CNAME to find CAA") + test.AssertEquals(t, resolver, "127.0.0.1:4053") expectedResp = `;; opcode: QUERY, status: NOERROR, id: XXXX ;; flags: qr rd; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0 @@ -513,12 +592,12 @@ bracewel.net. 0 IN CAA 1 issue "letsencrypt.org" ;; ANSWER SECTION: caa.example.com. 0 IN CAA 1 issue "letsencrypt.org" ` - test.AssertEquals(t, removeIDExp.ReplaceAllString(resp, " id: XXXX"), expectedResp) + test.AssertEquals(t, removeIDExp.ReplaceAllString(caas.String(), " id: XXXX"), expectedResp) - _, _, resolvers, err = obj.LookupCAA(context.Background(), "gonetld") + _, resolver, 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") + test.AssertEquals(t, resolver, "127.0.0.1:4053") } type testExchanger struct { @@ -678,7 +757,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 + dr.exchanger = tc.te _, _, err = dr.LookupTXT(context.Background(), "example.com") if err == errTooManyRequests { t.Errorf("#%d, sent more requests than the test case handles", i) @@ -696,20 +775,24 @@ func TestRetry(t *testing.T) { test.AssertMetricWithLabelsEquals( t, dr.timeoutCounter, prometheus.Labels{ "qtype": "TXT", - "type": "out of retries", + "result": "out of retries", "resolver": "127.0.0.1", "isTLD": "false", }, tc.metricsAllRetries) } }) } +} +func TestRetryMetrics(t *testing.T) { + isTimeoutErr := &url.Error{Op: "read", Err: testTimeoutError(true)} staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr}) test.AssertNotError(t, err, "Got error creating StaticProvider") testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig) dr := testClient.(*impl) - dr.dnsClient = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} + + dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} ctx, cancel := context.WithCancel(context.Background()) cancel() _, _, err = dr.LookupTXT(ctx, "example.com") @@ -718,7 +801,7 @@ func TestRetry(t *testing.T) { t.Errorf("expected %s, got %s", context.Canceled, err) } - dr.dnsClient = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} + dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} ctx, cancel = context.WithTimeout(context.Background(), -10*time.Hour) defer cancel() _, _, err = dr.LookupTXT(ctx, "example.com") @@ -727,7 +810,7 @@ func TestRetry(t *testing.T) { t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) } - dr.dnsClient = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} + dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} ctx, deadlineCancel := context.WithTimeout(context.Background(), -10*time.Hour) deadlineCancel() _, _, err = dr.LookupTXT(ctx, "example.com") @@ -739,27 +822,17 @@ func TestRetry(t *testing.T) { test.AssertMetricWithLabelsEquals( t, dr.timeoutCounter, prometheus.Labels{ "qtype": "TXT", - "type": "canceled", + "result": "canceled", "resolver": "127.0.0.1", }, 1) - test.AssertMetricWithLabelsEquals( t, dr.timeoutCounter, prometheus.Labels{ "qtype": "TXT", - "type": "deadline exceeded", + "result": "deadline exceeded", "resolver": "127.0.0.1", }, 2) } -func TestIsTLD(t *testing.T) { - if isTLD("com") != "true" { - t.Errorf("expected 'com' to be a TLD, got %q", isTLD("com")) - } - if isTLD("example.com") != "false" { - t.Errorf("expected 'example.com' to not a TLD, got %q", isTLD("example.com")) - } -} - type testTimeoutError bool func (t testTimeoutError) Timeout() bool { return bool(t) } @@ -822,7 +895,7 @@ func TestRotateServerOnErr(t *testing.T) { }, lookups: make(map[string]int), } - client.(*impl).dnsClient = mock + client.(*impl).exchanger = mock // Perform a bunch of lookups. We choose the initial server randomly. Any time // A or B is chosen there should be an error and a retry using the next server @@ -830,9 +903,8 @@ 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") - test.AssertEquals(t, len(resolvers), 1) - test.AssertEquals(t, resolvers[0], "[2606:4700:4700::1111]:53") + _, resolver, err := client.LookupTXT(context.Background(), "example.com") + test.AssertEquals(t, resolver, "[2606:4700:4700::1111]:53") // Any errors are unexpected - server "[2606:4700:4700::1111]:53" should // have responded without error. test.AssertNotError(t, err, "Expected no error from eventual retry with functional server") @@ -878,7 +950,7 @@ func TestDOHMetric(t *testing.T) { testClient := New(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, "", blog.UseMock(), tlsConfig) resolver := testClient.(*impl) - resolver.dnsClient = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: testTimeoutError(true)}} + resolver.exchanger = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: testTimeoutError(true)}} // Starting out, we should count 0 "out of retries" errors. test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 0) diff --git a/bdns/mocks.go b/bdns/mocks.go index 5852aac17ae..d9c3b15ef0e 100644 --- a/bdns/mocks.go +++ b/bdns/mocks.go @@ -3,159 +3,29 @@ package bdns import ( "context" "errors" - "fmt" - "net" - "net/netip" - "os" "github.com/miekg/dns" - - blog "github.com/letsencrypt/boulder/log" ) // MockClient is a mock -type MockClient struct { - Log blog.Logger -} +type MockClient struct{} // LookupTXT is a mock -func (mock *MockClient) LookupTXT(_ context.Context, hostname string) ([]string, ResolverAddrs, error) { - // Use the example account-specific label prefix derived from - // "https://example.com/acme/acct/ExampleAccount" - const accountLabelPrefix = "_ujmmovf2vn55tgye._acme-challenge" - - if hostname == accountLabelPrefix+".servfail.com" { - // Mirror dns-01 servfail behaviour - return nil, ResolverAddrs{"MockClient"}, fmt.Errorf("SERVFAIL") - } - if hostname == accountLabelPrefix+".good-dns01.com" { - // Mirror dns-01 good record - // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" - // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) - return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == accountLabelPrefix+".wrong-dns01.com" { - // Mirror dns-01 wrong record - return []string{"a"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == accountLabelPrefix+".wrong-many-dns01.com" { - // Mirror dns-01 wrong-many record - return []string{"a", "b", "c", "d", "e"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == accountLabelPrefix+".long-dns01.com" { - // Mirror dns-01 long record - return []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == accountLabelPrefix+".no-authority-dns01.com" { - // Mirror dns-01 no-authority good record - // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" - // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) - return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == accountLabelPrefix+".empty-txts.com" { - // Mirror dns-01 zero TXT records - return []string{}, ResolverAddrs{"MockClient"}, nil - } - - if hostname == "_acme-challenge.servfail.com" { - return nil, ResolverAddrs{"MockClient"}, fmt.Errorf("SERVFAIL") - } - if hostname == "_acme-challenge.good-dns01.com" { - // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" - // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) - // expected token + test account jwk thumbprint - return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == "_acme-challenge.wrong-dns01.com" { - return []string{"a"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == "_acme-challenge.wrong-many-dns01.com" { - return []string{"a", "b", "c", "d", "e"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == "_acme-challenge.long-dns01.com" { - return []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, ResolverAddrs{"MockClient"}, nil - } - if hostname == "_acme-challenge.no-authority-dns01.com" { - // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" - // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) - // expected token + test account jwk thumbprint - return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil - } - // empty-txts.com always returns zero TXT records - if hostname == "_acme-challenge.empty-txts.com" { - return []string{}, ResolverAddrs{"MockClient"}, nil - } - - // Default fallback - return []string{"hostname"}, ResolverAddrs{"MockClient"}, nil +func (mock *MockClient) LookupTXT(_ context.Context, hostname string) (*Result[*dns.TXT], string, error) { + return nil, "MockClient", errors.New("unexpected LookupTXT call on test fake") } -// makeTimeoutError returns a a net.OpError for which Timeout() returns true. -func makeTimeoutError() *net.OpError { - return &net.OpError{ - Err: os.NewSyscallError("ugh timeout", timeoutError{}), - } -} - -type timeoutError struct{} - -func (t timeoutError) Error() string { - return "so sloooow" -} -func (t timeoutError) Timeout() bool { - return true +// LookupA is a fake +func (mock *MockClient) LookupA(_ context.Context, hostname string) (*Result[*dns.A], string, error) { + return nil, "MockClient", errors.New("unexpected LookupA call on test fake") } -// LookupHost is a mock -func (mock *MockClient) LookupHost(_ context.Context, hostname string) ([]netip.Addr, ResolverAddrs, error) { - if hostname == "always.invalid" || - hostname == "invalid.invalid" { - return []netip.Addr{}, ResolverAddrs{"MockClient"}, nil - } - if hostname == "always.timeout" { - return []netip.Addr{}, ResolverAddrs{"MockClient"}, &Error{dns.TypeA, "always.timeout", makeTimeoutError(), -1, nil} - } - if hostname == "always.error" { - err := &net.OpError{ - Op: "read", - Net: "udp", - Err: errors.New("some net error"), - } - m := new(dns.Msg) - m.SetQuestion(dns.Fqdn(hostname), dns.TypeA) - m.AuthenticatedData = true - m.SetEdns0(4096, false) - return []netip.Addr{}, ResolverAddrs{"MockClient"}, &Error{dns.TypeA, hostname, err, -1, nil} - } - if hostname == "id.mismatch" { - err := dns.ErrId - m := new(dns.Msg) - m.SetQuestion(dns.Fqdn(hostname), dns.TypeA) - m.AuthenticatedData = true - m.SetEdns0(4096, false) - r := new(dns.Msg) - record := new(dns.A) - record.Hdr = dns.RR_Header{Name: dns.Fqdn(hostname), Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 0} - record.A = net.ParseIP("127.0.0.1") - r.Answer = append(r.Answer, record) - return []netip.Addr{}, ResolverAddrs{"MockClient"}, &Error{dns.TypeA, hostname, err, -1, nil} - } - // dual-homed host with an IPv6 and an IPv4 address - if hostname == "ipv4.and.ipv6.localhost" { - return []netip.Addr{ - netip.MustParseAddr("::1"), - netip.MustParseAddr("127.0.0.1"), - }, ResolverAddrs{"MockClient"}, nil - } - if hostname == "ipv6.localhost" { - return []netip.Addr{ - netip.MustParseAddr("::1"), - }, ResolverAddrs{"MockClient"}, nil - } - return []netip.Addr{netip.MustParseAddr("127.0.0.1")}, ResolverAddrs{"MockClient"}, nil +// LookupAAAA is a fake +func (mock *MockClient) LookupAAAA(_ context.Context, hostname string) (*Result[*dns.AAAA], string, error) { + return nil, "MockClient", errors.New("unexpected LookupAAAA call on test fake") } -// LookupCAA returns mock records for use in tests. -func (mock *MockClient) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, string, ResolverAddrs, error) { - return nil, "", ResolverAddrs{"MockClient"}, nil +// LookupCAA is a fake +func (mock *MockClient) LookupCAA(_ context.Context, domain string) (*Result[*dns.CAA], string, error) { + return nil, "MockClient", errors.New("unexpected LookupCAA call on test fake") } diff --git a/bdns/problem_test.go b/bdns/problem_test.go index 8e925d80bc6..bdf7040b019 100644 --- a/bdns/problem_test.go +++ b/bdns/problem_test.go @@ -7,8 +7,9 @@ import ( "net/url" "testing" - "github.com/letsencrypt/boulder/test" "github.com/miekg/dns" + + "github.com/letsencrypt/boulder/test" ) func TestError(t *testing.T) { @@ -17,9 +18,6 @@ func TestError(t *testing.T) { expected string }{ { - &Error{dns.TypeA, "hostname", makeTimeoutError(), -1, nil}, - "DNS problem: query timed out looking up A for hostname", - }, { &Error{dns.TypeMX, "hostname", &net.OpError{Err: errors.New("some net error")}, -1, nil}, "DNS problem: networking error looking up MX for hostname", }, { diff --git a/cmd/boulder-va/main.go b/cmd/boulder-va/main.go index fecf2ed7fdf..ab196c2ff33 100644 --- a/cmd/boulder-va/main.go +++ b/cmd/boulder-va/main.go @@ -100,28 +100,16 @@ func main() { tlsConfig, err := c.VA.TLS.Load(scope) cmd.FailOnError(err, "tlsConfig config") - var resolver bdns.Client - if !c.VA.DNSAllowLoopbackAddresses { - resolver = bdns.New( - c.VA.DNSTimeout.Duration, - servers, - scope, - clk, - c.VA.DNSTries, - c.VA.UserAgent, - logger, - tlsConfig) - } else { - resolver = bdns.NewTest( - c.VA.DNSTimeout.Duration, - servers, - scope, - clk, - c.VA.DNSTries, - c.VA.UserAgent, - logger, - tlsConfig) - } + resolver := bdns.New( + c.VA.DNSTimeout.Duration, + servers, + scope, + clk, + c.VA.DNSTries, + c.VA.UserAgent, + logger, + tlsConfig) + var remotes []va.RemoteVA if len(c.VA.RemoteVAs) > 0 { for _, rva := range c.VA.RemoteVAs { @@ -155,6 +143,7 @@ func main() { "", iana.IsReservedAddr, c.VA.SlowRemoteTimeout.Duration, + c.VA.DNSAllowLoopbackAddresses, ) cmd.FailOnError(err, "Unable to create VA server") diff --git a/cmd/remoteva/main.go b/cmd/remoteva/main.go index 43b68d621fd..e398f1b8233 100644 --- a/cmd/remoteva/main.go +++ b/cmd/remoteva/main.go @@ -106,28 +106,15 @@ func main() { tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven } - var resolver bdns.Client - if !c.RVA.DNSAllowLoopbackAddresses { - resolver = bdns.New( - c.RVA.DNSTimeout.Duration, - servers, - scope, - clk, - c.RVA.DNSTries, - c.RVA.UserAgent, - logger, - tlsConfig) - } else { - resolver = bdns.NewTest( - c.RVA.DNSTimeout.Duration, - servers, - scope, - clk, - c.RVA.DNSTries, - c.RVA.UserAgent, - logger, - tlsConfig) - } + resolver := bdns.New( + c.RVA.DNSTimeout.Duration, + servers, + scope, + clk, + c.RVA.DNSTries, + c.RVA.UserAgent, + logger, + tlsConfig) vai, err := va.NewValidationAuthorityImpl( resolver, @@ -142,6 +129,7 @@ func main() { c.RVA.RIR, iana.IsReservedAddr, 0, + c.RVA.DNSAllowLoopbackAddresses, ) cmd.FailOnError(err, "Unable to create Remote-VA server") diff --git a/va/caa.go b/va/caa.go index 0a9e248d46e..c0d32e34fad 100644 --- a/va/caa.go +++ b/va/caa.go @@ -130,13 +130,13 @@ func (va *ValidationAuthorityImpl) checkCAA( return errors.New("expected validationMethod or accountURIID not provided to checkCAA") } - foundAt, valid, response, err := va.checkCAARecords(ctx, ident, params) + foundAt, valid, err := va.checkCAARecords(ctx, ident, params) if err != nil { return berrors.DNSError("%s", err) } - va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %d, Challenge: %s, Valid for issuance: %t, Found at: %q] Response=%q", - ident.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt, response) + va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %d, Challenge: %s, Valid for issuance: %t, Found at: %q]", + ident.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt) if !valid { return berrors.CAAError("CAA record for %s prevents issuance", foundAt) } @@ -153,8 +153,7 @@ type caaResult struct { issue []*dns.CAA issuewild []*dns.CAA criticalUnknown bool - dig string - resolvers bdns.ResolverAddrs + resolver string err error } @@ -213,14 +212,17 @@ func (va *ValidationAuthorityImpl) parallelCAALookup(ctx context.Context, name s // Start the concurrent DNS lookup. wg.Add(1) go func(name string, r *caaResult) { + defer wg.Done() r.name = name - var records []*dns.CAA - records, r.dig, r.resolvers, r.err = va.dnsClient.LookupCAA(ctx, name) - if len(records) > 0 { + var records *bdns.Result[*dns.CAA] + records, r.resolver, r.err = va.dnsClient.LookupCAA(ctx, name) + if r.err != nil { + return + } + if len(records.Final) > 0 { r.present = true } - r.issue, r.issuewild, r.criticalUnknown = filterCAA(records) - wg.Done() + r.issue, r.issuewild, r.criticalUnknown = filterCAA(records.Final) }(strings.Join(labels[i:], "."), &results[i]) } @@ -276,12 +278,12 @@ func (va *ValidationAuthorityImpl) getCAA(ctx context.Context, hostname string) // which name (i.e. FQDN or parent thereof) CAA records were found, if any. The // second is a bool indicating whether issuance for the identifier is valid. The // unmodified *dns.CAA records that were processed/filtered are returned as the -// third argument. Any errors encountered are returned as the fourth return +// third argument. Any errors encountered are returned as the fourth return // value (or nil). func (va *ValidationAuthorityImpl) checkCAARecords( ctx context.Context, ident identifier.ACMEIdentifier, - params *caaParams) (string, bool, string, error) { + params *caaParams) (string, bool, error) { hostname := strings.ToLower(ident.Value) // If this is a wildcard name, remove the prefix var wildcard bool @@ -291,14 +293,10 @@ func (va *ValidationAuthorityImpl) checkCAARecords( } caaSet, err := va.getCAA(ctx, hostname) if err != nil { - return "", false, "", err - } - raw := "" - if caaSet != nil { - raw = caaSet.dig + return "", false, err } valid, foundAt := va.validateCAA(caaSet, wildcard, params) - return foundAt, valid, raw, nil + return foundAt, valid, nil } // validateCAA checks a provided *caaResult. When the wildcard argument is true diff --git a/va/caa_test.go b/va/caa_test.go index 3a83dd1553f..619cd5797c4 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -26,24 +26,18 @@ import ( vapb "github.com/letsencrypt/boulder/va/proto" ) -// caaMockDNS implements the `dns.DNSClient` interface with a set of useful test +// caaFakeDNS implements the `dns.DNSClient` interface with a set of useful test // answers for CAA queries. -type caaMockDNS struct{} - -func (mock caaMockDNS) LookupTXT(_ context.Context, hostname string) ([]string, bdns.ResolverAddrs, error) { - return nil, bdns.ResolverAddrs{"caaMockDNS"}, nil -} - -func (mock caaMockDNS) LookupHost(_ context.Context, hostname string) ([]netip.Addr, bdns.ResolverAddrs, error) { - return []netip.Addr{netip.MustParseAddr("127.0.0.1")}, bdns.ResolverAddrs{"caaMockDNS"}, nil +type caaFakeDNS struct { + bdns.Client } -func (mock caaMockDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, string, bdns.ResolverAddrs, error) { +func (mock *caaFakeDNS) LookupCAA(_ context.Context, domain string) (*bdns.Result[*dns.CAA], string, error) { var results []*dns.CAA var record dns.CAA switch strings.TrimRight(domain, ".") { case "caa-timeout.com": - return nil, "", bdns.ResolverAddrs{"caaMockDNS"}, fmt.Errorf("error") + return nil, "caaFakeDNS", fmt.Errorf("error") case "reserved.com": record.Tag = "issue" record.Value = "ca.com" @@ -63,11 +57,10 @@ func (mock caaMockDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, results = append(results, &record) case "com": // com has no CAA records. - return nil, "", bdns.ResolverAddrs{"caaMockDNS"}, nil case "gonetld": - return nil, "", bdns.ResolverAddrs{"caaMockDNS"}, fmt.Errorf("NXDOMAIN") + return nil, "caaFakeDNS", fmt.Errorf("NXDOMAIN") case "servfail.com", "servfail.present.com": - return results, "", bdns.ResolverAddrs{"caaMockDNS"}, fmt.Errorf("SERVFAIL") + return nil, "caaFakeDNS", fmt.Errorf("SERVFAIL") case "multi-crit-present.com": record.Flag = 1 record.Tag = "issue" @@ -185,15 +178,12 @@ func (mock caaMockDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, record.Value = "letsencrypt.org" results = append(results, &record) } - var response string - if len(results) > 0 { - response = "foo" - } - return results, response, bdns.ResolverAddrs{"caaMockDNS"}, nil + + return &bdns.Result[*dns.CAA]{Final: results}, "caaFakeDNS", nil } func TestCAATimeout(t *testing.T) { - va, _ := setup(nil, "", nil, caaMockDNS{}) + va, _ := setup(nil, "", nil, &caaFakeDNS{}) params := &caaParams{ accountURIID: 12345, @@ -416,7 +406,7 @@ func TestCAAChecking(t *testing.T) { method := core.ChallengeTypeHTTP01 params := &caaParams{accountURIID: accountURIID, validationMethod: method} - va, _ := setup(nil, "", nil, caaMockDNS{}) + va, _ := setup(nil, "", nil, &caaFakeDNS{}) va.accountURIPrefixes = []string{"https://letsencrypt.org/acct/reg/"} for _, caaTest := range testCases { @@ -424,7 +414,7 @@ func TestCAAChecking(t *testing.T) { defer mockLog.Clear() t.Run(caaTest.Name, func(t *testing.T) { ident := identifier.NewDNS(caaTest.Domain) - foundAt, valid, _, err := va.checkCAARecords(ctx, ident, params) + foundAt, valid, err := va.checkCAARecords(ctx, ident, params) if err != nil { t.Errorf("checkCAARecords error for %s: %s", caaTest.Domain, err) } @@ -439,7 +429,7 @@ func TestCAAChecking(t *testing.T) { } func TestCAALogging(t *testing.T) { - va, _ := setup(nil, "", nil, caaMockDNS{}) + va, _ := setup(nil, "", nil, &caaFakeDNS{}) testCases := []struct { Name string @@ -452,55 +442,55 @@ func TestCAALogging(t *testing.T) { Domain: "reserved.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"reserved.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"reserved.com\"]", }, { Domain: "reserved.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeDNS01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: dns-01, Valid for issuance: false, Found at: \"reserved.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: dns-01, Valid for issuance: false, Found at: \"reserved.com\"]", }, { Domain: "mixedcase.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for mixedcase.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"mixedcase.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for mixedcase.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"mixedcase.com\"]", }, { Domain: "critical.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for critical.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"critical.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for critical.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"critical.com\"]", }, { Domain: "present.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"]", }, { Domain: "not.here.but.still.present.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for not.here.but.still.present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for not.here.but.still.present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"]", }, { Domain: "multi-crit-present.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for multi-crit-present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"multi-crit-present.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for multi-crit-present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"multi-crit-present.com\"]", }, { Domain: "present-with-parameter.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present-with-parameter.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present-with-parameter.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present-with-parameter.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present-with-parameter.com\"]", }, { Domain: "satisfiable-wildcard-override.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for satisfiable-wildcard-override.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"satisfiable-wildcard-override.com\"] Response=\"foo\"", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for satisfiable-wildcard-override.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"satisfiable-wildcard-override.com\"]", }, } @@ -530,7 +520,7 @@ func TestCAALogging(t *testing.T) { // includes the domain name that was being checked in the failure detail. func TestDoCAAErrMessage(t *testing.T) { t.Parallel() - va, _ := setup(nil, "", nil, caaMockDNS{}) + va, _ := setup(nil, "", nil, &caaFakeDNS{}) // Call the operation with a domain we know fails with a generic error from the // caaMockDNS. @@ -556,7 +546,7 @@ func TestDoCAAErrMessage(t *testing.T) { // Binding checks. func TestDoCAAParams(t *testing.T) { t.Parallel() - va, _ := setup(nil, "", nil, caaMockDNS{}) + va, _ := setup(nil, "", nil, &caaFakeDNS{}) // Calling IsCAAValid without a ValidationMethod should fail. _, err := va.DoCAA(ctx, &vapb.IsCAAValidRequest{ @@ -593,35 +583,24 @@ var errCAABrokenDNSClient = errors.New("dnsClient is broken") // caaBrokenDNS implements the `dns.DNSClient` interface, but always returns // errors. -type caaBrokenDNS struct{} - -func (b caaBrokenDNS) LookupTXT(_ context.Context, hostname string) ([]string, bdns.ResolverAddrs, error) { - return nil, bdns.ResolverAddrs{"caaBrokenDNS"}, errCAABrokenDNSClient +type caaBrokenDNS struct { + bdns.Client } -func (b caaBrokenDNS) LookupHost(_ context.Context, hostname string) ([]netip.Addr, bdns.ResolverAddrs, error) { - return nil, bdns.ResolverAddrs{"caaBrokenDNS"}, errCAABrokenDNSClient -} - -func (b caaBrokenDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, string, bdns.ResolverAddrs, error) { - return nil, "", bdns.ResolverAddrs{"caaBrokenDNS"}, errCAABrokenDNSClient +func (b caaBrokenDNS) LookupCAA(_ context.Context, domain string) (*bdns.Result[*dns.CAA], string, error) { + return nil, "caaBrokenDNS", errCAABrokenDNSClient } // caaHijackedDNS implements the `dns.DNSClient` interface with a set of useful // test answers for CAA queries. It returns alternate CAA records than what -// caaMockDNS returns simulating either a BGP hijack or DNS records that have +// caaFakeDNS returns simulating either a BGP hijack or DNS records that have // changed while queries were inflight. -type caaHijackedDNS struct{} - -func (h caaHijackedDNS) LookupTXT(_ context.Context, hostname string) ([]string, bdns.ResolverAddrs, error) { - return nil, bdns.ResolverAddrs{"caaHijackedDNS"}, nil +type caaHijackedDNS struct { + bdns.Client } -func (h caaHijackedDNS) LookupHost(_ context.Context, hostname string) ([]netip.Addr, bdns.ResolverAddrs, error) { - return []netip.Addr{netip.MustParseAddr("127.0.0.1")}, bdns.ResolverAddrs{"caaHijackedDNS"}, nil -} -func (h caaHijackedDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, string, bdns.ResolverAddrs, error) { - // These records are altered from their caaMockDNS counterparts. Use this to +func (b caaHijackedDNS) LookupCAA(_ context.Context, domain string) (*bdns.Result[*dns.CAA], string, error) { + // These records are altered from their caaFakeDNS counterparts. Use this to // tickle remoteValidationFailures. var results []*dns.CAA var record dns.CAA @@ -631,7 +610,7 @@ func (h caaHijackedDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, record.Value = "other-ca.com" results = append(results, &record) case "present-dns-only.com": - return results, "", bdns.ResolverAddrs{"caaHijackedDNS"}, fmt.Errorf("SERVFAIL") + return nil, "caaHijackedDNS", fmt.Errorf("SERVFAIL") case "satisfiable-wildcard.com": record.Tag = "issuewild" record.Value = ";" @@ -641,11 +620,8 @@ func (h caaHijackedDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, secondRecord.Value = ";" results = append(results, &secondRecord) } - var response string - if len(results) > 0 { - response = "foo" - } - return results, response, bdns.ResolverAddrs{"caaHijackedDNS"}, nil + + return &bdns.Result[*dns.CAA]{Final: results}, "caaHijackedDNS", nil } // parseValidationLogEvent extracts ... from JSON={ ... } in a ValidateChallenge @@ -691,11 +667,11 @@ func TestMultiCAARechecking(t *testing.T) { { name: "all VAs functional, no CAA records", ident: identifier.NewDNS("present-dns-only.com"), - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ - {ua: remoteUA, rir: arin}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: arin, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, expectedLabels: prometheus.Labels{ "operation": opCAA, @@ -712,9 +688,9 @@ func TestMultiCAARechecking(t *testing.T) { expectedProbSubstring: "While processing CAA for present-dns-only.com: dnsClient is broken", expectedProbType: probs.DNSProblem, remoteVAs: []remoteConf{ - {ua: remoteUA, rir: arin}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: arin, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, expectedLabels: prometheus.Labels{ "operation": opCAA, @@ -727,7 +703,7 @@ func TestMultiCAARechecking(t *testing.T) { { name: "functional localVA, 1 broken RVA, no CAA records", ident: identifier.NewDNS("present-dns-only.com"), - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, expectedDiffLogSubstring: `"RemoteSuccesses":2,"RemoteFailures":1`, expectedSummary: &mpicSummary{ Passed: []string{"dc-1-RIPE", "dc-2-APNIC"}, @@ -737,8 +713,8 @@ func TestMultiCAARechecking(t *testing.T) { }, remoteVAs: []remoteConf{ {ua: brokenUA, rir: arin, dns: caaBrokenDNS{}}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, expectedLabels: prometheus.Labels{ "operation": opCAA, @@ -760,11 +736,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{apnic}, QuorumResult: "1/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: brokenUA, rir: arin, dns: caaBrokenDNS{}}, {ua: brokenUA, rir: ripe, dns: caaBrokenDNS{}}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, expectedLabels: prometheus.Labels{ "operation": opCAA, @@ -786,7 +762,7 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{}, QuorumResult: "0/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: brokenUA, rir: arin, dns: caaBrokenDNS{}}, {ua: brokenUA, rir: ripe, dns: caaBrokenDNS{}}, @@ -803,11 +779,11 @@ func TestMultiCAARechecking(t *testing.T) { { name: "all VAs functional, CAA issue type present", ident: identifier.NewDNS("present.com"), - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ - {ua: remoteUA, rir: arin}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: arin, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, expectedLabels: prometheus.Labels{ "operation": opCAA, @@ -827,11 +803,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{ripe, apnic}, QuorumResult: "2/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: brokenUA, rir: arin, dns: caaBrokenDNS{}}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, expectedLabels: prometheus.Labels{ "operation": opCAA, @@ -853,11 +829,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{apnic}, QuorumResult: "1/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: brokenUA, rir: arin, dns: caaBrokenDNS{}}, {ua: brokenUA, rir: ripe, dns: caaBrokenDNS{}}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, expectedLabels: prometheus.Labels{ "operation": opCAA, @@ -879,7 +855,7 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{}, QuorumResult: "0/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: brokenUA, rir: arin, dns: caaBrokenDNS{}}, {ua: brokenUA, rir: ripe, dns: caaBrokenDNS{}}, @@ -900,11 +876,11 @@ func TestMultiCAARechecking(t *testing.T) { ident: identifier.NewDNS("unsatisfiable.com"), expectedProbSubstring: "CAA record for unsatisfiable.com prevents issuance", expectedProbType: probs.CAAProblem, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ - {ua: remoteUA, rir: arin}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: arin, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, }, { @@ -917,11 +893,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{ripe, apnic}, QuorumResult: "2/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, }, { @@ -936,11 +912,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{apnic}, QuorumResult: "1/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, {ua: hijackedUA, rir: ripe, dns: caaHijackedDNS{}}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, }, { @@ -955,7 +931,7 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{}, QuorumResult: "0/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, {ua: hijackedUA, rir: ripe, dns: caaHijackedDNS{}}, @@ -972,11 +948,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{ripe, apnic}, QuorumResult: "2/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, }, { @@ -991,11 +967,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{apnic}, QuorumResult: "1/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, {ua: hijackedUA, rir: ripe, dns: caaHijackedDNS{}}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, }, { @@ -1010,7 +986,7 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{}, QuorumResult: "0/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, {ua: hijackedUA, rir: ripe, dns: caaHijackedDNS{}}, @@ -1027,11 +1003,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{ripe, apnic}, QuorumResult: "2/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, - {ua: remoteUA, rir: ripe}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: ripe, dns: &caaFakeDNS{}}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, }, { @@ -1046,11 +1022,11 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{apnic}, QuorumResult: "1/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, {ua: hijackedUA, rir: ripe, dns: caaHijackedDNS{}}, - {ua: remoteUA, rir: apnic}, + {ua: remoteUA, rir: apnic, dns: &caaFakeDNS{}}, }, }, { @@ -1065,7 +1041,7 @@ func TestMultiCAARechecking(t *testing.T) { PassedRIRs: []string{}, QuorumResult: "0/3", }, - localDNSClient: caaMockDNS{}, + localDNSClient: &caaFakeDNS{}, remoteVAs: []remoteConf{ {ua: hijackedUA, rir: arin, dns: caaHijackedDNS{}}, {ua: hijackedUA, rir: ripe, dns: caaHijackedDNS{}}, @@ -1095,7 +1071,7 @@ func TestMultiCAARechecking(t *testing.T) { test.AssertNotNil(t, isValidRes.Problem, "IsCAAValidRequest returned nil problem, but should not have") test.AssertContains(t, isValidRes.Problem.Detail, tc.expectedProbSubstring) } else if isValidRes.Problem != nil { - test.AssertBoxedNil(t, isValidRes.Problem, "IsCAAValidRequest returned a problem, but should not have") + test.AssertBoxedNil(t, isValidRes.Problem, fmt.Sprintf("IsCAAValidRequest returned problem %q, but should not have", isValidRes.Problem.Detail)) } if tc.expectedProbType != "" { @@ -1131,7 +1107,7 @@ func TestCAAFailure(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, _ := setup(hs, "", nil, caaMockDNS{}) + va, _ := setup(hs, "", nil, &caaFakeDNS{}) err := va.checkCAA(ctx, identifier.NewDNS("reserved.com"), &caaParams{1, core.ChallengeTypeHTTP01}) if err == nil { @@ -1234,9 +1210,9 @@ func TestSelectCAA(t *testing.T) { // A slice of empty caaResults should return nil, "", nil r = []caaResult{ - {"", false, nil, nil, false, "", nil, nil}, - {"", false, nil, nil, false, "", nil, nil}, - {"", false, nil, nil, false, "", nil, nil}, + {"", false, nil, nil, false, "", nil}, + {"", false, nil, nil, false, "", nil}, + {"", false, nil, nil, false, "", nil}, } s, err = selectCAA(r) test.Assert(t, s == nil, "set is not nil") @@ -1245,8 +1221,8 @@ func TestSelectCAA(t *testing.T) { // A slice of caaResults containing an error followed by a CAA // record should return the error r = []caaResult{ - {"foo.com", false, nil, nil, false, "", nil, errors.New("oops")}, - {"com", true, []*dns.CAA{&expected}, nil, false, "foo", nil, nil}, + {"foo.com", false, nil, nil, false, "", errors.New("oops")}, + {"com", true, []*dns.CAA{&expected}, nil, false, "foo", nil}, } s, err = selectCAA(r) test.Assert(t, s == nil, "set is not nil") @@ -1256,26 +1232,24 @@ func TestSelectCAA(t *testing.T) { // A slice of caaResults containing a good record that precedes an // error, should return that good record, not the error r = []caaResult{ - {"foo.com", true, []*dns.CAA{&expected}, nil, false, "foo", nil, nil}, - {"com", false, nil, nil, false, "", nil, errors.New("")}, + {"foo.com", true, []*dns.CAA{&expected}, nil, false, "foo", nil}, + {"com", false, nil, nil, false, "", errors.New("")}, } s, err = selectCAA(r) test.AssertEquals(t, len(s.issue), 1) test.Assert(t, s.issue[0] == &expected, "Incorrect record returned") - test.AssertEquals(t, s.dig, "foo") test.Assert(t, err == nil, "error is not nil") // A slice of caaResults containing multiple CAA records should // return the first non-empty CAA record r = []caaResult{ - {"bar.foo.com", false, []*dns.CAA{}, []*dns.CAA{}, false, "", nil, nil}, - {"foo.com", true, []*dns.CAA{&expected}, nil, false, "foo", nil, nil}, - {"com", true, []*dns.CAA{&expected}, nil, false, "bar", nil, nil}, + {"bar.foo.com", false, []*dns.CAA{}, []*dns.CAA{}, false, "", nil}, + {"foo.com", true, []*dns.CAA{&expected}, nil, false, "foo", nil}, + {"com", true, []*dns.CAA{&expected}, nil, false, "bar", nil}, } s, err = selectCAA(r) test.AssertEquals(t, len(s.issue), 1) test.Assert(t, s.issue[0] == &expected, "Incorrect record returned") - test.AssertEquals(t, s.dig, "foo") test.AssertNotError(t, err, "expect nil error") } diff --git a/va/dns.go b/va/dns.go index ba854f89f44..3ae2a1d68bd 100644 --- a/va/dns.go +++ b/va/dns.go @@ -3,37 +3,87 @@ package va import ( "context" "crypto/sha256" - "crypto/subtle" "encoding/base32" "encoding/base64" "errors" "fmt" "net/netip" + "slices" "strings" + "sync" + + "github.com/miekg/dns" "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/core" berrors "github.com/letsencrypt/boulder/errors" + "github.com/letsencrypt/boulder/iana" "github.com/letsencrypt/boulder/identifier" ) // getAddr will query for all A/AAAA records associated with hostname and return -// the preferred address, the first netip.Addr in the addrs slice, and all -// addresses resolved. This is the same choice made by the Go internal +// all valid addresses resolved. This is the same choice made by the Go internal // resolution library used by net/http. If there is an error resolving the // hostname, or if no usable IP addresses are available then a berrors.DNSError // instance is returned with a nil netip.Addr slice. -func (va ValidationAuthorityImpl) getAddrs(ctx context.Context, hostname string) ([]netip.Addr, bdns.ResolverAddrs, error) { - addrs, resolvers, err := va.dnsClient.LookupHost(ctx, hostname) - if err != nil { - return nil, resolvers, berrors.DNSError("%v", err) +func (va ValidationAuthorityImpl) getAddrs(ctx context.Context, hostname string) ([]netip.Addr, []string, error) { + // Kick off both the A and AAAA lookups in parallel. + var wg sync.WaitGroup + + var resA *bdns.Result[*dns.A] + var resolverA string + var errA error + wg.Go(func() { + resA, resolverA, errA = va.dnsClient.LookupA(ctx, hostname) + }) + + var resAAAA *bdns.Result[*dns.AAAA] + var resolverAAAA string + var errAAAA error + wg.Go(func() { + resAAAA, resolverAAAA, errAAAA = va.dnsClient.LookupAAAA(ctx, hostname) + }) + + wg.Wait() + + // Collect all non-empty resolvers now, so we can return them along with + // either a real result or an error. + resolvers := []string{resolverA, resolverAAAA} + resolvers = slices.DeleteFunc(resolvers, func(a string) bool { + return a == "" + }) + + var addrsA []netip.Addr + if errA == nil { + for _, rr := range resA.Final { + addr := netip.AddrFrom4([4]byte(rr.A.To4())) + if addr.IsValid() && (iana.IsReservedAddr(addr) == nil || va.allowRestrictedAddrs) { + addrsA = append(addrsA, addr) + } + } + if len(addrsA) == 0 { + errA = fmt.Errorf("no valid A records found for %s", hostname) + } } - if len(addrs) == 0 { - // This should be unreachable, as no valid IP addresses being found results - // in an error being returned from LookupHost. - return nil, resolvers, berrors.DNSError("No valid IP addresses found for %s", hostname) + var addrsAAAA []netip.Addr + if errAAAA == nil { + for _, rr := range resAAAA.Final { + addr := netip.AddrFrom16([16]byte(rr.AAAA.To16())) + if addr.IsValid() && (iana.IsReservedAddr(addr) == nil || va.allowRestrictedAddrs) { + addrsAAAA = append(addrsAAAA, addr) + } + } + if len(addrsAAAA) == 0 { + errAAAA = fmt.Errorf("no valid AAAA records found for %s", hostname) + } + } + + if errA != nil && errAAAA != nil { + return nil, nil, berrors.DNSError("%s; %s", errA, errAAAA) } + + addrs := append(addrsAAAA, addrsA...) va.log.Debugf("Resolved addresses for %s: %s", hostname, addrs) return addrs, resolvers, nil } @@ -109,7 +159,7 @@ func (va *ValidationAuthorityImpl) validateDNS(ctx context.Context, ident identi challengeSubdomain := fmt.Sprintf("%s.%s", challengePrefix, ident.Value) // Look for the required record in the DNS - txts, resolvers, err := va.dnsClient.LookupTXT(ctx, challengeSubdomain) + txts, resolver, err := va.dnsClient.LookupTXT(ctx, challengeSubdomain) if err != nil { return nil, berrors.DNSError("%s", err) } @@ -117,24 +167,24 @@ func (va *ValidationAuthorityImpl) validateDNS(ctx context.Context, ident identi // If there weren't any TXT records return a distinct error message to allow // troubleshooters to differentiate between no TXT records and // invalid/incorrect TXT records. - if len(txts) == 0 { + if len(txts.Final) == 0 { return nil, berrors.UnauthorizedError("No TXT record found at %s", challengeSubdomain) } - for _, element := range txts { - if subtle.ConstantTimeCompare([]byte(element), []byte(authorizedKeysDigest)) == 1 { + for _, rr := range txts.Final { + if strings.Join(rr.Txt, "") == authorizedKeysDigest { // Successful challenge validation - return []core.ValidationRecord{{Hostname: ident.Value, ResolverAddrs: resolvers}}, nil + return []core.ValidationRecord{{Hostname: ident.Value, ResolverAddrs: []string{resolver}}}, nil } } - invalidRecord := txts[0] + invalidRecord := strings.Join(txts.Final[0].Txt, "") if len(invalidRecord) > 100 { invalidRecord = invalidRecord[0:100] + "..." } var andMore string - if len(txts) > 1 { - andMore = fmt.Sprintf(" (and %d more)", len(txts)-1) + if len(txts.Final) > 1 { + andMore = fmt.Sprintf(" (and %d more)", len(txts.Final)-1) } return nil, berrors.UnauthorizedError("Incorrect TXT record %q%s found at %s", invalidRecord, andMore, challengeSubdomain) diff --git a/va/dns_account_test.go b/va/dns_account_test.go index 9e652659ffc..1098446c491 100644 --- a/va/dns_account_test.go +++ b/va/dns_account_test.go @@ -6,14 +6,9 @@ import ( "net/netip" "strings" "testing" - "time" - "github.com/jmhodges/clock" - - "github.com/letsencrypt/boulder/bdns" berrors "github.com/letsencrypt/boulder/errors" "github.com/letsencrypt/boulder/identifier" - "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/test" ) @@ -85,7 +80,7 @@ func TestDNSAccount01Validation(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNSAccount01(ctx, tc.ident, expectedKeyAuthorization, testAccountURI) if tc.wantErrMsg != "" { @@ -109,21 +104,9 @@ func TestDNSAccount01Validation(t *testing.T) { } func TestDNSAccount01ValidationNoServer(t *testing.T) { - va, log := setup(nil, "", nil, nil) - staticProvider, err := bdns.NewStaticProvider([]string{}) - test.AssertNotError(t, err, "Couldn't make new static provider") - - va.dnsClient = bdns.NewTest( - time.Second*5, - staticProvider, - metrics.NoopRegisterer, - clock.New(), - 1, - "", - log, - nil) + va, _ := setup(nil, "", nil, nil) - _, err = va.validateDNSAccount01(ctx, identifier.NewDNS("localhost"), expectedKeyAuthorization, testAccountURI) + _, err := va.validateDNSAccount01(ctx, identifier.NewDNS("localhost"), expectedKeyAuthorization, testAccountURI) prob := detailedError(err) test.AssertEquals(t, prob.Type, probs.DNSProblem) } diff --git a/va/dns_test.go b/va/dns_test.go index 1f0a22af81f..55a716ea387 100644 --- a/va/dns_test.go +++ b/va/dns_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/jmhodges/clock" + "github.com/miekg/dns" "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/identifier" @@ -16,8 +17,91 @@ import ( "github.com/letsencrypt/boulder/test" ) +type txtFakeDNS struct { + bdns.Client +} + +func (c *txtFakeDNS) LookupTXT(_ context.Context, hostname string) (*bdns.Result[*dns.TXT], string, error) { + // Use the example account-specific label prefix derived from + // "https://example.com/acme/acct/ExampleAccount" + const accountLabelPrefix = "_ujmmovf2vn55tgye._acme-challenge" + + var wrapTXT = func(txts ...string) (*bdns.Result[*dns.TXT], string, error) { + var rrs []*dns.TXT + for _, txt := range txts { + rrs = append(rrs, &dns.TXT{Txt: []string{txt}}) + } + return &bdns.Result[*dns.TXT]{Final: rrs}, "txtFakeDNS", nil + } + + if hostname == accountLabelPrefix+".servfail.com" { + // Mirror dns-01 servfail behaviour + return nil, "txtFakeDNS", fmt.Errorf("SERVFAIL") + } + if hostname == accountLabelPrefix+".good-dns01.com" { + // Mirror dns-01 good record + // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" + // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) + return wrapTXT("LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo") + } + if hostname == accountLabelPrefix+".wrong-dns01.com" { + // Mirror dns-01 wrong record + return wrapTXT("a") + } + if hostname == accountLabelPrefix+".wrong-many-dns01.com" { + // Mirror dns-01 wrong-many record + return wrapTXT("a", "b", "c", "d", "e") + } + if hostname == accountLabelPrefix+".long-dns01.com" { + // Mirror dns-01 long record + return wrapTXT("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + } + if hostname == accountLabelPrefix+".no-authority-dns01.com" { + // Mirror dns-01 no-authority good record + // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" + // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) + return wrapTXT("LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo") + } + if hostname == accountLabelPrefix+".empty-txts.com" { + // Mirror dns-01 zero TXT records + return wrapTXT() + } + + if hostname == "_acme-challenge.servfail.com" { + return nil, "txtFakeDNS", fmt.Errorf("SERVFAIL") + } + if hostname == "_acme-challenge.good-dns01.com" { + // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" + // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) + // expected token + test account jwk thumbprint + return wrapTXT("LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo") + } + if hostname == "_acme-challenge.wrong-dns01.com" { + return wrapTXT("a") + } + if hostname == "_acme-challenge.wrong-many-dns01.com" { + return wrapTXT("a", "b", "c", "d", "e") + } + if hostname == "_acme-challenge.long-dns01.com" { + return wrapTXT("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + } + if hostname == "_acme-challenge.no-authority-dns01.com" { + // base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0" + // + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI")) + // expected token + test account jwk thumbprint + return wrapTXT("LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo") + } + // empty-txts.com always returns zero TXT records + if hostname == "_acme-challenge.empty-txts.com" { + return wrapTXT() + } + + // Default fallback + return wrapTXT("hostname") +} + func TestDNS01ValidationWrong(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNS01(context.Background(), identifier.NewDNS("wrong-dns01.com"), expectedKeyAuthorization) if err == nil { t.Fatalf("Successful DNS validation with wrong TXT record") @@ -27,7 +111,7 @@ func TestDNS01ValidationWrong(t *testing.T) { } func TestDNS01ValidationWrongMany(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNS01(context.Background(), identifier.NewDNS("wrong-many-dns01.com"), expectedKeyAuthorization) if err == nil { @@ -38,7 +122,7 @@ func TestDNS01ValidationWrongMany(t *testing.T) { } func TestDNS01ValidationWrongLong(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNS01(context.Background(), identifier.NewDNS("long-dns01.com"), expectedKeyAuthorization) if err == nil { @@ -49,7 +133,7 @@ func TestDNS01ValidationWrongLong(t *testing.T) { } func TestDNS01ValidationFailure(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNS01(ctx, identifier.NewDNS("localhost"), expectedKeyAuthorization) prob := detailedError(err) @@ -58,7 +142,7 @@ func TestDNS01ValidationFailure(t *testing.T) { } func TestDNS01ValidationIP(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNS01(ctx, identifier.NewIP(netip.MustParseAddr("127.0.0.1")), expectedKeyAuthorization) prob := detailedError(err) @@ -72,7 +156,7 @@ func TestDNS01ValidationInvalid(t *testing.T) { Value: "790DB180-A274-47A4-855F-31C428CB1072", } - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNS01(ctx, notDNS, expectedKeyAuthorization) prob := detailedError(err) @@ -81,7 +165,7 @@ func TestDNS01ValidationInvalid(t *testing.T) { } func TestDNS01ValidationServFail(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, err := va.validateDNS01(ctx, identifier.NewDNS("servfail.com"), expectedKeyAuthorization) @@ -90,7 +174,7 @@ func TestDNS01ValidationServFail(t *testing.T) { } func TestDNS01ValidationNoServer(t *testing.T) { - va, log := setup(nil, "", nil, nil) + va, log := setup(nil, "", nil, &txtFakeDNS{}) staticProvider, err := bdns.NewStaticProvider([]string{}) test.AssertNotError(t, err, "Couldn't make new static provider") @@ -110,7 +194,7 @@ func TestDNS01ValidationNoServer(t *testing.T) { } func TestDNS01ValidationOK(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, prob := va.validateDNS01(ctx, identifier.NewDNS("good-dns01.com"), expectedKeyAuthorization) @@ -118,7 +202,7 @@ func TestDNS01ValidationOK(t *testing.T) { } func TestDNS01ValidationNoAuthorityOK(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) _, prob := va.validateDNS01(ctx, identifier.NewDNS("no-authority-dns01.com"), expectedKeyAuthorization) diff --git a/va/http.go b/va/http.go index e7b0ec3043e..dd9f0a99849 100644 --- a/va/http.go +++ b/va/http.go @@ -15,7 +15,6 @@ import ( "time" "unicode" - "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/core" berrors "github.com/letsencrypt/boulder/errors" "github.com/letsencrypt/boulder/iana" @@ -178,8 +177,8 @@ type httpValidationTarget struct { next []netip.Addr // the current IP address being used for validation (if any) cur netip.Addr - // the DNS resolver(s) that will attempt to fulfill the validation request - resolvers bdns.ResolverAddrs + // the DNS resolver(s) that were used to look up the host's IP addresses + resolvers []string } // nextIP changes the cur IP by removing the first entry from the next slice and @@ -209,7 +208,7 @@ func (va *ValidationAuthorityImpl) newHTTPValidationTarget( path string, query string) (*httpValidationTarget, error) { var addrs []netip.Addr - var resolvers bdns.ResolverAddrs + var resolvers []string switch ident.Type { case identifier.TypeDNS: // Resolve IP addresses for the identifier diff --git a/va/http_test.go b/va/http_test.go index c70890864fc..605acbe03aa 100644 --- a/va/http_test.go +++ b/va/http_test.go @@ -16,6 +16,8 @@ import ( "time" "unicode/utf8" + "github.com/miekg/dns" + "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/core" berrors "github.com/letsencrypt/boulder/errors" @@ -27,6 +29,52 @@ import ( "testing" ) +type ipFakeDNS struct { + bdns.Client +} + +func (c *ipFakeDNS) LookupA(_ context.Context, hostname string) (*bdns.Result[*dns.A], string, error) { + var wrapA = func(ips ...net.IP) (*bdns.Result[*dns.A], string, error) { + var rrs []*dns.A + for _, ip := range ips { + rrs = append(rrs, &dns.A{A: ip}) + } + res := &bdns.Result[*dns.A]{Final: rrs} + return res, "ipFakeDNS", nil + } + + if strings.HasSuffix(hostname, ".invalid") { + return wrapA() + } + // dual-homed host with an IPv6 and an IPv4 address + if hostname == "ipv4.and.ipv6.localhost" { + return wrapA(net.IPv4(127, 0, 0, 1)) + } + if hostname == "ipv6.localhost" { + return wrapA() + } + return wrapA(net.IPv4(127, 0, 0, 1)) +} + +func (c *ipFakeDNS) LookupAAAA(_ context.Context, hostname string) (*bdns.Result[*dns.AAAA], string, error) { + wrapAAAA := func(ips ...net.IP) (*bdns.Result[*dns.AAAA], string, error) { + var rrs []*dns.AAAA + for _, ip := range ips { + rrs = append(rrs, &dns.AAAA{AAAA: ip}) + } + return &bdns.Result[*dns.AAAA]{Final: rrs}, "ipFakeDNS", nil + } + + // dual-homed host with an IPv6 and an IPv4 address + if hostname == "ipv4.and.ipv6.localhost" { + return wrapAAAA(net.IPv6loopback) + } + if hostname == "ipv6.localhost" { + return wrapAAAA(net.IPv6loopback) + } + return wrapAAAA() +} + // TestDialerMismatchError tests that using a preresolvedDialer for one host for // a dial to another host produces the expected dialerMismatchError. func TestDialerMismatchError(t *testing.T) { @@ -50,15 +98,19 @@ func TestDialerMismatchError(t *testing.T) { test.AssertEquals(t, err.Error(), expectedErr.Error()) } -// dnsMockReturnsUnroutable is a DNSClient mock that always returns an -// unroutable address for LookupHost. This is useful in testing connect -// timeouts. -type dnsMockReturnsUnroutable struct { - *bdns.MockClient +// unroutableFakeDNS is a DNSClient mock that always returns an +// unroutable address for LookupA and no results for LookupAAAA. This is useful +// in testing connect timeouts. +type unroutableFakeDNS struct { + bdns.Client +} + +func (c *unroutableFakeDNS) LookupA(_ context.Context, hostname string) (*bdns.Result[*dns.A], string, error) { + return &bdns.Result[*dns.A]{Final: []*dns.A{{A: net.IPv4(64, 112, 117, 254)}}}, "dnsFakeUnroutable", nil } -func (mock dnsMockReturnsUnroutable) LookupHost(_ context.Context, hostname string) ([]netip.Addr, bdns.ResolverAddrs, error) { - return []netip.Addr{netip.MustParseAddr("64.112.117.254")}, bdns.ResolverAddrs{"dnsMockReturnsUnroutable"}, nil +func (c *unroutableFakeDNS) LookupAAAA(_ context.Context, hostname string) (*bdns.Result[*dns.AAAA], string, error) { + return nil, "dnsFakeUnroutable", errors.New("SERVFAIL") } // TestDialerTimeout tests that the preresolvedDialer's DialContext @@ -67,7 +119,7 @@ func (mock dnsMockReturnsUnroutable) LookupHost(_ context.Context, hostname stri // the appropriate "Timeout during connect" error message, which helps clients // distinguish between firewall problems and server problems. func TestDialerTimeout(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &unroutableFakeDNS{}) // Timeouts below 50ms tend to be flaky. va.singleDialTimeout = 50 * time.Millisecond @@ -76,7 +128,6 @@ func TestDialerTimeout(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), ctxTimeout) defer cancel() - va.dnsClient = dnsMockReturnsUnroutable{&bdns.MockClient{}} // NOTE(@jsha): The only method I've found so far to trigger a connect timeout // is to connect to an unrouteable IP address. This usually generates // a connection timeout, but will rarely return "Network unreachable" instead. @@ -126,8 +177,6 @@ func TestHTTPTransport(t *testing.T) { } func TestHTTPValidationTarget(t *testing.T) { - // NOTE(@cpu): See `bdns/mocks.go` and the mock `LookupHost` function for the - // hostnames used in this test. testCases := []struct { Name string Ident identifier.ACMEIdentifier @@ -137,7 +186,7 @@ func TestHTTPValidationTarget(t *testing.T) { { Name: "No IPs for DNS identifier", Ident: identifier.NewDNS("always.invalid"), - ExpectedError: berrors.DNSError("No valid IP addresses found for always.invalid"), + ExpectedError: berrors.DNSError("no valid A records found for always.invalid; no valid AAAA records found for always.invalid"), }, { Name: "Only IPv4 addrs for DNS identifier", @@ -173,7 +222,7 @@ func TestHTTPValidationTarget(t *testing.T) { exampleQuery = "my-path=was&my=own" ) - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &ipFakeDNS{}) for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { target, err := va.newHTTPValidationTarget( @@ -410,7 +459,7 @@ func TestExtractRequestTarget(t *testing.T) { } func TestSetupHTTPValidation(t *testing.T) { - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &ipFakeDNS{}) mustTarget := func(t *testing.T, host string, port int, path string) *httpValidationTarget { target, err := va.newHTTPValidationTarget( @@ -472,7 +521,7 @@ func TestSetupHTTPValidation(t *testing.T) { URL: "http://ipv4.and.ipv6.localhost/yellow/brick/road", AddressesResolved: []netip.Addr{netip.MustParseAddr("::1"), netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("::1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, ExpectedDialer: &preresolvedDialer{ ip: netip.MustParseAddr("::1"), @@ -490,7 +539,7 @@ func TestSetupHTTPValidation(t *testing.T) { URL: "https://ipv4.and.ipv6.localhost/yellow/brick/road", AddressesResolved: []netip.Addr{netip.MustParseAddr("::1"), netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("::1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, ExpectedDialer: &preresolvedDialer{ ip: netip.MustParseAddr("::1"), @@ -799,8 +848,8 @@ func TestFetchHTTP(t *testing.T) { // Setup VAs. By providing the testSrv to setup the VA will use the testSrv's // randomly assigned port as its HTTP port. - vaIPv4, _ := setup(testSrvIPv4, "", nil, nil) - vaIPv6, _ := setup(testSrvIPv6, "", nil, nil) + vaIPv4, _ := setup(testSrvIPv4, "", nil, &ipFakeDNS{}) + vaIPv6, _ := setup(testSrvIPv6, "", nil, &ipFakeDNS{}) // We need to know the randomly assigned HTTP port for testcases as well httpPortIPv4 := getPort(testSrvIPv4) @@ -827,7 +876,7 @@ func TestFetchHTTP(t *testing.T) { URL: url, AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }) } @@ -848,7 +897,7 @@ func TestFetchHTTP(t *testing.T) { URL: url, AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }) } @@ -871,7 +920,7 @@ func TestFetchHTTP(t *testing.T) { Ident: identifier.NewDNS("always.invalid"), Path: "/.well-known/whatever", ExpectedProblem: probs.DNS( - "No valid IP addresses found for always.invalid"), + "no valid A records found for always.invalid; no valid AAAA records found for always.invalid"), // There are no validation records in this case because the base record // is only constructed once a URL is made. ExpectedRecords: nil, @@ -890,7 +939,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/timeout", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -925,7 +974,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/redir-bad-proto", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -943,7 +992,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/redir-bad-port", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -959,7 +1008,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/redir-bare-ipv4", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, { Hostname: "127.0.0.1", @@ -982,7 +1031,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://ipv6.localhost/redir-bare-ipv6", AddressesResolved: []netip.Addr{netip.MustParseAddr("::1")}, AddressUsed: netip.MustParseAddr("::1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, { Hostname: "::1", @@ -1006,7 +1055,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/redir-path-too-long", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1023,7 +1072,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/bad-status-code", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1040,7 +1089,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/303-see-other", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1058,7 +1107,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/resp-too-big", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1075,7 +1124,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://ipv6.localhost/ok", AddressesResolved: []netip.Addr{netip.MustParseAddr("::1")}, AddressUsed: netip.MustParseAddr("::1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1092,7 +1141,7 @@ func TestFetchHTTP(t *testing.T) { AddressesResolved: []netip.Addr{netip.MustParseAddr("::1"), netip.MustParseAddr("127.0.0.1")}, // The first validation record should have used the IPv6 addr AddressUsed: netip.MustParseAddr("::1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, { Hostname: "ipv4.and.ipv6.localhost", @@ -1101,7 +1150,7 @@ func TestFetchHTTP(t *testing.T) { AddressesResolved: []netip.Addr{netip.MustParseAddr("::1"), netip.MustParseAddr("127.0.0.1")}, // The second validation record should have used the IPv4 addr as a fallback AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1117,7 +1166,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/ok", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1133,7 +1182,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/redir-uppercase-publicsuffix", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, { Hostname: "example.com", @@ -1141,7 +1190,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/ok", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1162,7 +1211,7 @@ func TestFetchHTTP(t *testing.T) { URL: "http://example.com/printf-verbs", AddressesResolved: []netip.Addr{netip.MustParseAddr("127.0.0.1")}, AddressUsed: netip.MustParseAddr("127.0.0.1"), - ResolverAddrs: []string{"MockClient"}, + ResolverAddrs: []string{"ipFakeDNS", "ipFakeDNS"}, }, }, }, @@ -1298,7 +1347,7 @@ func TestHTTPBadPort(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) // Pick a random port between 40000 and 65000 - with great certainty we won't // have an HTTP server listening on this port and the test will fail as @@ -1342,7 +1391,7 @@ func TestHTTPKeyAuthorizationFileMismatch(t *testing.T) { }) hs.Start() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateHTTP01(ctx, identifier.NewDNS("localhost.com"), expectedToken, expectedKeyAuthorization) if err == nil { @@ -1358,7 +1407,7 @@ func TestHTTP(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, log := setup(hs, "", nil, nil) + va, log := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateHTTP01(ctx, identifier.NewDNS("localhost.com"), expectedToken, expectedKeyAuthorization) if err != nil { @@ -1423,7 +1472,7 @@ func TestHTTPIPv6(t *testing.T) { hs := httpSrv(t, expectedToken, true) defer hs.Close() - va, log := setup(hs, "", nil, nil) + va, log := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateHTTP01(ctx, identifier.NewIP(netip.MustParseAddr("::1")), expectedToken, expectedKeyAuthorization) if err != nil { @@ -1436,7 +1485,7 @@ func TestHTTPTimeout(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) started := time.Now() timeout := 250 * time.Millisecond @@ -1464,7 +1513,7 @@ func TestHTTPTimeout(t *testing.T) { func TestHTTPRedirectLookup(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, log := setup(hs, "", nil, nil) + va, log := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateHTTP01(ctx, identifier.NewDNS("localhost.com"), pathMoved, ka(pathMoved)) if err != nil { @@ -1526,7 +1575,7 @@ func TestHTTPRedirectLookup(t *testing.T) { func TestHTTPRedirectLoop(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, prob := va.validateHTTP01(ctx, identifier.NewDNS("localhost"), "looper", ka("looper")) if prob == nil { @@ -1537,7 +1586,7 @@ func TestHTTPRedirectLoop(t *testing.T) { func TestHTTPRedirectUserAgent(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) va.userAgent = rejectUserAgent _, prob := va.validateHTTP01(ctx, identifier.NewDNS("localhost"), pathMoved, ka(pathMoved)) @@ -1573,7 +1622,7 @@ func TestValidateHTTP(t *testing.T) { hs := httpSrv(t, token, false) defer hs.Close() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, prob := va.validateHTTP01(ctx, identifier.NewDNS("localhost"), token, ka(token)) test.Assert(t, prob == nil, "validation failed") @@ -1583,7 +1632,7 @@ func TestLimitedReader(t *testing.T) { token := core.NewToken() hs := httpSrv(t, "012345\xff67890123456789012345678901234567890123456789012345678901234567890123456789", false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) defer hs.Close() _, err := va.validateHTTP01(ctx, identifier.NewDNS("localhost"), token, ka(token)) @@ -1654,7 +1703,7 @@ func TestHTTPHostHeader(t *testing.T) { // Setup VA. By providing the testSrv to setup the VA will use the // testSrv's randomly assigned port as its HTTP port. - va, _ := setup(testSrv, "", nil, nil) + va, _ := setup(testSrv, "", nil, &ipFakeDNS{}) var got string _, _, _ = va.processHTTPValidation(ctx, tc.Ident, "/ok") diff --git a/va/tlsalpn.go b/va/tlsalpn.go index f297d16adf7..7de28674345 100644 --- a/va/tlsalpn.go +++ b/va/tlsalpn.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "crypto/sha256" - "crypto/subtle" "crypto/tls" "crypto/x509" "crypto/x509/pkix" @@ -367,7 +366,7 @@ func (va *ValidationAuthorityImpl) validateTLSALPN01(ctx context.Context, ident return validationRecords, badCertErr( "Received certificate with malformed acmeValidationV1 extension value.") } - if subtle.ConstantTimeCompare(h[:], extValue) != 1 { + if !bytes.Equal(h[:], extValue) { return validationRecords, badCertErr(fmt.Sprintf( "Received certificate with acmeValidationV1 extension value %s but expected %s.", hex.EncodeToString(extValue), diff --git a/va/tlsalpn_test.go b/va/tlsalpn_test.go index 31de4c2ad56..ec6af5cd8f8 100644 --- a/va/tlsalpn_test.go +++ b/va/tlsalpn_test.go @@ -24,7 +24,6 @@ import ( "github.com/prometheus/client_golang/prometheus" - "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/core" "github.com/letsencrypt/boulder/identifier" "github.com/letsencrypt/boulder/probs" @@ -139,7 +138,7 @@ func slowTLSSrv() *httptest.Server { func TestTLSALPNTimeoutAfterConnect(t *testing.T) { hs := slowTLSSrv() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) timeout := 50 * time.Millisecond ctx, cancel := context.WithTimeout(context.Background(), timeout) @@ -176,7 +175,7 @@ func TestTLSALPNTimeoutAfterConnect(t *testing.T) { func TestTLSALPN01DialTimeout(t *testing.T) { hs := slowTLSSrv() - va, _ := setup(hs, "", nil, dnsMockReturnsUnroutable{&bdns.MockClient{}}) + va, _ := setup(hs, "", nil, &unroutableFakeDNS{}) started := time.Now() timeout := 50 * time.Millisecond @@ -225,7 +224,7 @@ func TestTLSALPN01DialTimeout(t *testing.T) { func TestTLSALPN01Refused(t *testing.T) { hs := testTLSALPN01Srv(t) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) // Take down validation server and check that validation fails. hs.Close() @@ -244,7 +243,7 @@ func TestTLSALPN01Refused(t *testing.T) { func TestTLSALPN01TalkingToHTTP(t *testing.T) { hs := testTLSALPN01Srv(t) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) // Make the server only speak HTTP. httpOnly := httpSrv(t, "", false) @@ -273,7 +272,7 @@ func brokenTLSSrv() *httptest.Server { func TestTLSError(t *testing.T) { hs := brokenTLSSrv() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) if err == nil { @@ -289,7 +288,7 @@ func TestTLSError(t *testing.T) { func TestDNSError(t *testing.T) { hs := brokenTLSSrv() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("always.invalid"), expectedKeyAuthorization) if err == nil { @@ -366,7 +365,7 @@ func TestCertNames(t *testing.T) { func TestTLSALPN01SuccessDNS(t *testing.T) { hs := testTLSALPN01Srv(t) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) res, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) if err != nil { @@ -385,7 +384,7 @@ func TestTLSALPN01SuccessIPv4(t *testing.T) { cert := testTLSCert(nil, []net.IP{net.ParseIP("127.0.0.1")}, []pkix.Extension{testACMEExt}) hs := tlsalpn01SrvWithCert(t, cert, 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) res, err := va.validateTLSALPN01(ctx, identifier.NewIP(netip.MustParseAddr("127.0.0.1")), expectedKeyAuthorization) if err != nil { @@ -404,7 +403,7 @@ func TestTLSALPN01SuccessIPv6(t *testing.T) { cert := testTLSCert(nil, []net.IP{net.ParseIP("::1")}, []pkix.Extension{testACMEExt}) hs := tlsalpn01SrvWithCert(t, cert, 0, true) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) res, err := va.validateTLSALPN01(ctx, identifier.NewIP(netip.MustParseAddr("::1")), expectedKeyAuthorization) if err != nil { @@ -432,7 +431,7 @@ func TestTLSALPN01ObsoleteFailure(t *testing.T) { cert := testTLSCert([]string{"expected"}, nil, []pkix.Extension{acmeExtension(IdPeAcmeIdentifierV1Obsolete, expectedKeyAuthorization)}) hs := tlsalpn01SrvWithCert(t, cert, 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertNotNil(t, err, "expected validation to fail") @@ -445,7 +444,7 @@ func TestValidateTLSALPN01BadChallenge(t *testing.T) { cert := testTLSCert([]string{"expected"}, nil, []pkix.Extension{acmeExtension(IdPeAcmeIdentifier, badKeyAuthorization)}) hs := tlsalpn01SrvWithCert(t, cert, 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) if err == nil { @@ -466,7 +465,7 @@ func TestValidateTLSALPN01BadChallenge(t *testing.T) { func TestValidateTLSALPN01BrokenSrv(t *testing.T) { hs := brokenTLSSrv() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) if err == nil { @@ -489,7 +488,7 @@ func TestValidateTLSALPN01UnawareSrv(t *testing.T) { } hs.StartTLS() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) if err == nil { @@ -522,7 +521,7 @@ func TestValidateTLSALPN01MalformedExtnValue(t *testing.T) { for _, badExt := range badExtensions { acmeCert := testTLSCert([]string{"expected"}, nil, []pkix.Extension{badExt}) hs := tlsalpn01SrvWithCert(t, acmeCert, 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) hs.Close() @@ -562,7 +561,7 @@ func TestTLSALPN01TLSVersion(t *testing.T) { // Create a server that only negotiates the given TLS version hs := tlsalpn01SrvWithCert(t, cert, tc.version, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) if !tc.expectError { @@ -587,7 +586,7 @@ func TestTLSALPN01WrongName(t *testing.T) { // Create a cert with a different name from what we're validating hs := tlsalpn01SrvWithCert(t, testACMECert([]string{"incorrect"}), 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -599,7 +598,7 @@ func TestTLSALPN01WrongIPv4(t *testing.T) { cert := testTLSCert(nil, []net.IP{net.ParseIP("10.10.10.10")}, []pkix.Extension{testACMEExt}) hs := tlsalpn01SrvWithCert(t, cert, 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewIP(netip.MustParseAddr("127.0.0.1")), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -611,7 +610,7 @@ func TestTLSALPN01WrongIPv6(t *testing.T) { cert := testTLSCert(nil, []net.IP{net.ParseIP("::2")}, []pkix.Extension{testACMEExt}) hs := tlsalpn01SrvWithCert(t, cert, 0, true) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewIP(netip.MustParseAddr("::1")), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -622,7 +621,7 @@ func TestTLSALPN01ExtraNames(t *testing.T) { // Create a cert with two names when we only want to validate one. hs := tlsalpn01SrvWithCert(t, testACMECert([]string{"expected", "extra"}), 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -633,7 +632,7 @@ func TestTLSALPN01WrongIdentType(t *testing.T) { // Create a cert with an IP address encoded as a name. hs := tlsalpn01SrvWithCert(t, testACMECert([]string{"127.0.0.1"}), 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewIP(netip.MustParseAddr("127.0.0.1")), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -644,7 +643,7 @@ func TestTLSALPN01TooManyIdentTypes(t *testing.T) { // Create a cert with both a name and an IP address when we only want to validate one. hs := tlsalpn01SrvWithCert(t, testTLSCert([]string{"expected"}, []net.IP{net.ParseIP("127.0.0.1")}, []pkix.Extension{testACMEExt}), 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -702,7 +701,7 @@ func TestTLSALPN01NotSelfSigned(t *testing.T) { hs := tlsalpn01SrvWithCert(t, acmeCert, 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err = va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -720,7 +719,7 @@ func TestTLSALPN01NotSelfSigned(t *testing.T) { hs = tlsalpn01SrvWithCert(t, acmeCert, 0, false) - va, _ = setup(hs, "", nil, nil) + va, _ = setup(hs, "", nil, &ipFakeDNS{}) _, err = va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -759,7 +758,7 @@ func TestTLSALPN01ExtraIdentifiers(t *testing.T) { hs := tlsalpn01SrvWithCert(t, acmeCert, tls.VersionTLS12, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err = va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -782,7 +781,7 @@ func TestTLSALPN01ExtraSANs(t *testing.T) { extensions := []pkix.Extension{testACMEExt, subjectAltName, subjectAltName} hs := tlsalpn01SrvWithCert(t, testTLSCert([]string{"expected"}, nil, extensions), 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err = va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -797,7 +796,7 @@ func TestTLSALPN01ExtraAcmeExtensions(t *testing.T) { extensions := []pkix.Extension{testACMEExt, testACMEExt} hs := tlsalpn01SrvWithCert(t, testTLSCert([]string{"expected"}, nil, extensions), 0, false) - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.NewDNS("expected"), expectedKeyAuthorization) test.AssertError(t, err, "validation should have failed") @@ -861,7 +860,7 @@ func TestTLSALPN01BadIdentifier(t *testing.T) { hs := httpSrv(t, expectedToken, false) defer hs.Close() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) _, err := va.validateTLSALPN01(ctx, identifier.ACMEIdentifier{Type: "smime", Value: "dobber@bad.horse"}, expectedKeyAuthorization) test.AssertError(t, err, "Server accepted a hypothetical S/MIME identifier") @@ -938,7 +937,7 @@ func TestTLSALPN01ServerName(t *testing.T) { hs.StartTLS() defer hs.Close() - va, _ := setup(hs, "", nil, nil) + va, _ := setup(hs, "", nil, &ipFakeDNS{}) // The actual test happens in the tlsConfig.GetCertificate function, // which the validation will call and depend on for its success. diff --git a/va/va.go b/va/va.go index fd766262267..ccaa2855882 100644 --- a/va/va.go +++ b/va/va.go @@ -188,22 +188,23 @@ func newDefaultPortConfig() *portConfig { type ValidationAuthorityImpl struct { vapb.UnsafeVAServer vapb.UnsafeCAAServer - log blog.Logger - dnsClient bdns.Client - issuerDomain string - httpPort int - httpsPort int - tlsPort int - userAgent string - clk clock.Clock - remoteVAs []RemoteVA - maxRemoteFailures int - accountURIPrefixes []string - singleDialTimeout time.Duration - slowRemoteTimeout time.Duration - perspective string - rir string - isReservedIPFunc func(netip.Addr) error + log blog.Logger + dnsClient bdns.Client + issuerDomain string + httpPort int + httpsPort int + tlsPort int + userAgent string + clk clock.Clock + remoteVAs []RemoteVA + maxRemoteFailures int + accountURIPrefixes []string + singleDialTimeout time.Duration + slowRemoteTimeout time.Duration + perspective string + rir string + isReservedIPFunc func(netip.Addr) error + allowRestrictedAddrs bool metrics *vaMetrics } @@ -225,6 +226,7 @@ func NewValidationAuthorityImpl( rir string, reservedIPChecker func(netip.Addr) error, slowRemoteTimeout time.Duration, + allowRestrictedAddrs bool, ) (*ValidationAuthorityImpl, error) { if len(accountURIPrefixes) == 0 { @@ -258,10 +260,11 @@ func NewValidationAuthorityImpl( // before timing out. This timeout ignores the base RPC timeout and is strictly // used for the DialContext operations that take place during an // HTTP-01 challenge validation. - singleDialTimeout: 10 * time.Second, - perspective: perspective, - rir: rir, - isReservedIPFunc: reservedIPChecker, + singleDialTimeout: 10 * time.Second, + perspective: perspective, + rir: rir, + isReservedIPFunc: reservedIPChecker, + allowRestrictedAddrs: allowRestrictedAddrs, } return va, nil diff --git a/va/va_test.go b/va/va_test.go index 196ca97dc01..273c563e289 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -116,7 +116,7 @@ func isNonLoopbackReservedIP(ip netip.Addr) error { // // If remoteVAs is nil, this builds a VA that acts like a remote (and does not // perform multi-perspective validation). Otherwise it acts like a primary. -func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNSClientOverride bdns.Client) (*ValidationAuthorityImpl, *blog.Mock) { +func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, fakeDNSClient bdns.Client) (*ValidationAuthorityImpl, *blog.Mock) { features.Reset() fc := clock.NewFake() @@ -133,8 +133,13 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS perspective = "example perspective " + core.RandomString(4) } + dnsClient := fakeDNSClient + if dnsClient == nil { + dnsClient = &bdns.MockClient{} + } + va, err := NewValidationAuthorityImpl( - &bdns.MockClient{Log: logger}, + dnsClient, remoteVAs, userAgent, "letsencrypt.org", @@ -146,15 +151,12 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS "", isNonLoopbackReservedIP, time.Second, + true, ) if err != nil { panic(fmt.Sprintf("Failed to create validation authority: %v", err)) } - if mockDNSClientOverride != nil { - va.dnsClient = mockDNSClientOverride - } - // Adjusting industry regulated ACME challenge port settings is fine during // testing if srv != nil { @@ -166,8 +168,8 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS return va, logger } -func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride bdns.Client, perspective, rir string) RemoteClients { - rva, _ := setup(srv, userAgent, nil, mockDNSClientOverride) +func setupRemote(srv *httptest.Server, userAgent string, fakeDNSClient bdns.Client, perspective, rir string) RemoteClients { + rva, _ := setup(srv, userAgent, nil, fakeDNSClient) rva.perspective = perspective rva.rir = rir @@ -222,9 +224,9 @@ func setupRemotes(confs []remoteConf, srv *httptest.Server) []RemoteVA { return remoteVAs } -func setupWithRemotes(srv *httptest.Server, userAgent string, remotes []remoteConf, mockDNSClientOverride bdns.Client) (*ValidationAuthorityImpl, *blog.Mock) { +func setupWithRemotes(srv *httptest.Server, userAgent string, remotes []remoteConf, fakeDNSClient bdns.Client) (*ValidationAuthorityImpl, *blog.Mock) { remoteVAs := setupRemotes(remotes, srv) - return setup(srv, userAgent, remoteVAs, mockDNSClientOverride) + return setup(srv, userAgent, remoteVAs, fakeDNSClient) } type multiSrv struct { @@ -313,7 +315,7 @@ func TestNewValidationAuthorityImplWithDuplicateRemotes(t *testing.T) { } _, err := NewValidationAuthorityImpl( - &bdns.MockClient{Log: blog.NewMock()}, + &bdns.MockClient{}, remoteVAs, "user agent 1.0", "letsencrypt.org", @@ -325,6 +327,7 @@ func TestNewValidationAuthorityImplWithDuplicateRemotes(t *testing.T) { "", isNonLoopbackReservedIP, time.Second, + true, ) test.AssertError(t, err, "NewValidationAuthorityImpl allowed duplicate remote perspectives") test.AssertContains(t, err.Error(), "duplicate remote VA perspective \"dadaist\"") @@ -334,19 +337,19 @@ func TestPerformValidationWithMismatchedRemoteVAPerspectives(t *testing.T) { t.Parallel() mismatched1 := RemoteVA{ - RemoteClients: setupRemote(nil, "", nil, "dadaist", arin), + RemoteClients: setupRemote(nil, "", &txtFakeDNS{}, "dadaist", arin), Perspective: "baroque", RIR: arin, } mismatched2 := RemoteVA{ - RemoteClients: setupRemote(nil, "", nil, "impressionist", ripe), + RemoteClients: setupRemote(nil, "", &txtFakeDNS{}, "impressionist", ripe), Perspective: "minimalist", RIR: ripe, } remoteVAs := setupRemotes([]remoteConf{{rir: ripe}}, nil) remoteVAs = append(remoteVAs, mismatched1, mismatched2) - va, mockLog := setup(nil, "", remoteVAs, nil) + va, mockLog := setup(nil, "", remoteVAs, &txtFakeDNS{}) req := createValidationRequest(identifier.NewDNS("good-dns01.com"), core.ChallengeTypeDNS01) res, _ := va.DoDCV(context.Background(), req) test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") @@ -357,19 +360,19 @@ func TestPerformValidationWithMismatchedRemoteVARIRs(t *testing.T) { t.Parallel() mismatched1 := RemoteVA{ - RemoteClients: setupRemote(nil, "", nil, "dadaist", arin), + RemoteClients: setupRemote(nil, "", &txtFakeDNS{}, "dadaist", arin), Perspective: "dadaist", RIR: ripe, } mismatched2 := RemoteVA{ - RemoteClients: setupRemote(nil, "", nil, "impressionist", ripe), + RemoteClients: setupRemote(nil, "", &txtFakeDNS{}, "impressionist", ripe), Perspective: "impressionist", RIR: arin, } remoteVAs := setupRemotes([]remoteConf{{rir: ripe}}, nil) remoteVAs = append(remoteVAs, mismatched1, mismatched2) - va, mockLog := setup(nil, "", remoteVAs, nil) + va, mockLog := setup(nil, "", remoteVAs, &txtFakeDNS{}) req := createValidationRequest(identifier.NewDNS("good-dns01.com"), core.ChallengeTypeDNS01) res, _ := va.DoDCV(context.Background(), req) test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") @@ -387,7 +390,7 @@ func TestValidateMalformedChallenge(t *testing.T) { func TestPerformValidationInvalid(t *testing.T) { t.Parallel() - va, _ := setup(nil, "", nil, nil) + va, _ := setup(nil, "", nil, &txtFakeDNS{}) req := createValidationRequest(identifier.NewDNS("foo.com"), core.ChallengeTypeDNS01) res, _ := va.DoDCV(context.Background(), req) @@ -404,7 +407,7 @@ func TestPerformValidationInvalid(t *testing.T) { func TestInternalErrorLogged(t *testing.T) { t.Parallel() - va, mockLog := setup(nil, "", nil, nil) + va, mockLog := setup(nil, "", nil, &ipFakeDNS{}) ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() @@ -419,7 +422,7 @@ func TestInternalErrorLogged(t *testing.T) { func TestPerformValidationValid(t *testing.T) { t.Parallel() - va, mockLog := setup(nil, "", nil, nil) + va, mockLog := setup(nil, "", nil, &txtFakeDNS{}) // create a challenge with well known token req := createValidationRequest(identifier.NewDNS("good-dns01.com"), core.ChallengeTypeDNS01) @@ -447,7 +450,7 @@ func TestPerformValidationValid(t *testing.T) { func TestPerformValidationWildcard(t *testing.T) { t.Parallel() - va, mockLog := setup(nil, "", nil, nil) + va, mockLog := setup(nil, "", nil, &txtFakeDNS{}) // create a challenge with well known token req := createValidationRequest(identifier.NewDNS("*.good-dns01.com"), core.ChallengeTypeDNS01) @@ -503,9 +506,9 @@ func TestMultiVA(t *testing.T) { // With local and all remote VAs working there should be no problem. Name: "Local and remote VAs OK", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, }, PrimaryUA: pass, }, @@ -513,9 +516,9 @@ func TestMultiVA(t *testing.T) { // If the local VA fails everything should fail Name: "Local VA bad, remote VAs OK", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, }, PrimaryUA: fail, ExpectedProbType: string(probs.UnauthorizedProblem), @@ -524,8 +527,8 @@ func TestMultiVA(t *testing.T) { // If one out of three remote VAs fails with an internal err it should succeed Name: "Local VA ok, 1/3 remote VA internal err", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, {ua: pass, rir: apnic, impl: brokenVA}, }, PrimaryUA: pass, @@ -534,7 +537,7 @@ func TestMultiVA(t *testing.T) { // If two out of three remote VAs fail with an internal err it should fail Name: "Local VA ok, 2/3 remote VAs internal err", Remotes: []remoteConf{ - {ua: pass, rir: arin}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, {ua: pass, rir: ripe, impl: brokenVA}, {ua: pass, rir: apnic, impl: brokenVA}, }, @@ -547,10 +550,10 @@ func TestMultiVA(t *testing.T) { // If one out of five remote VAs fail with an internal err it should succeed Name: "Local VA ok, 1/5 remote VAs internal err", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, - {ua: pass, rir: lacnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, + {ua: pass, rir: lacnic, dns: &ipFakeDNS{}}, {ua: pass, rir: afrinic, impl: brokenVA}, }, PrimaryUA: pass, @@ -559,9 +562,9 @@ func TestMultiVA(t *testing.T) { // If two out of five remote VAs fail with an internal err it should fail Name: "Local VA ok, 2/5 remote VAs internal err", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, {ua: pass, rir: arin, impl: brokenVA}, {ua: pass, rir: ripe, impl: brokenVA}, }, @@ -574,10 +577,10 @@ func TestMultiVA(t *testing.T) { // If two out of six remote VAs fail with an internal err it should succeed Name: "Local VA ok, 2/6 remote VAs internal err", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, - {ua: pass, rir: lacnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, + {ua: pass, rir: lacnic, dns: &ipFakeDNS{}}, {ua: pass, rir: afrinic, impl: brokenVA}, {ua: pass, rir: arin, impl: brokenVA}, }, @@ -587,9 +590,9 @@ func TestMultiVA(t *testing.T) { // If three out of six remote VAs fail with an internal err it should fail Name: "Local VA ok, 4/6 remote VAs internal err", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, {ua: pass, rir: lacnic, impl: brokenVA}, {ua: pass, rir: afrinic, impl: brokenVA}, {ua: pass, rir: arin, impl: brokenVA}, @@ -603,9 +606,9 @@ func TestMultiVA(t *testing.T) { // With only one working remote VA there should be a validation failure Name: "Local VA and one remote VA OK", Remotes: []remoteConf{ - {ua: pass, rir: arin}, - {ua: fail, rir: ripe}, - {ua: fail, rir: apnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: fail, rir: ripe, dns: &ipFakeDNS{}}, + {ua: fail, rir: apnic, dns: &ipFakeDNS{}}, }, PrimaryUA: pass, ExpectedProbType: string(probs.UnauthorizedProblem), @@ -615,9 +618,9 @@ func TestMultiVA(t *testing.T) { // If one remote VA cancels, it should succeed Name: "Local VA and one remote VA OK, one cancelled VA", Remotes: []remoteConf{ - {ua: pass, rir: arin}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, {ua: pass, rir: ripe, impl: cancelledVA}, - {ua: pass, rir: apnic}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, }, PrimaryUA: pass, }, @@ -637,9 +640,9 @@ func TestMultiVA(t *testing.T) { // With the local and remote VAs seeing diff problems, we expect a problem. Name: "Local and remote VA differential", Remotes: []remoteConf{ - {ua: fail, rir: arin}, - {ua: fail, rir: ripe}, - {ua: fail, rir: apnic}, + {ua: fail, rir: arin, dns: &ipFakeDNS{}}, + {ua: fail, rir: ripe, dns: &ipFakeDNS{}}, + {ua: fail, rir: apnic, dns: &ipFakeDNS{}}, }, PrimaryUA: pass, ExpectedProbType: string(probs.UnauthorizedProblem), @@ -656,7 +659,7 @@ func TestMultiVA(t *testing.T) { defer ms.Close() // Configure a primary VA with testcase remote VAs. - localVA, mockLog := setupWithRemotes(ms.Server, tc.PrimaryUA, tc.Remotes, nil) + localVA, mockLog := setupWithRemotes(ms.Server, tc.PrimaryUA, tc.Remotes, &ipFakeDNS{}) // Perform all validations res, _ := localVA.DoDCV(ctx, req) @@ -707,15 +710,15 @@ func TestMultiVALogging(t *testing.T) { t.Parallel() remoteConfs := []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, + {ua: pass, rir: arin, dns: &ipFakeDNS{}}, + {ua: pass, rir: ripe, dns: &ipFakeDNS{}}, + {ua: pass, rir: apnic, dns: &ipFakeDNS{}}, } ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) defer ms.Close() - va, _ := setupWithRemotes(ms.Server, pass, remoteConfs, nil) + va, _ := setupWithRemotes(ms.Server, pass, remoteConfs, &ipFakeDNS{}) req := createValidationRequest(identifier.NewDNS("letsencrypt.org"), core.ChallengeTypeHTTP01) res, err := va.DoDCV(ctx, req) test.Assert(t, res.Problem == nil, fmt.Sprintf("validation failed with: %#v", res.Problem)) From fac947a642ee1baafe9df792d7dc37aba7645e4f Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Mon, 8 Dec 2025 16:27:55 -0800 Subject: [PATCH 2/5] Fix unittest race --- bdns/dns_test.go | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/bdns/dns_test.go b/bdns/dns_test.go index bf1d02ee3e4..96f58db49c5 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -791,7 +791,6 @@ func TestRetryMetrics(t *testing.T) { testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig) dr := testClient.(*impl) - dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -800,7 +799,15 @@ func TestRetryMetrics(t *testing.T) { 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) } + test.AssertMetricWithLabelsEquals( + t, dr.timeoutCounter, prometheus.Labels{ + "qtype": "TXT", + "result": "canceled", + "resolver": "127.0.0.1", + }, 1) + testClient = New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig) + dr = testClient.(*impl) dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} ctx, cancel = context.WithTimeout(context.Background(), -10*time.Hour) defer cancel() @@ -809,28 +816,12 @@ func TestRetryMetrics(t *testing.T) { err.Error() != "DNS problem: query timed out looking up TXT for example.com" { t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) } - - dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} - ctx, deadlineCancel := context.WithTimeout(context.Background(), -10*time.Hour) - deadlineCancel() - _, _, 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) - } - - test.AssertMetricWithLabelsEquals( - t, dr.timeoutCounter, prometheus.Labels{ - "qtype": "TXT", - "result": "canceled", - "resolver": "127.0.0.1", - }, 1) test.AssertMetricWithLabelsEquals( t, dr.timeoutCounter, prometheus.Labels{ "qtype": "TXT", "result": "deadline exceeded", "resolver": "127.0.0.1", - }, 2) + }, 1) } type testTimeoutError bool From f02b35cdb559ec985db477723634214839b7f79e Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Wed, 10 Dec 2025 17:08:20 -0800 Subject: [PATCH 3/5] Review comments --- bdns/dns.go | 132 ++++++++++++++++++++++------------------------- bdns/dns_test.go | 21 +++----- va/caa.go | 8 ++- va/dns.go | 10 ++-- 4 files changed, 78 insertions(+), 93 deletions(-) diff --git a/bdns/dns.go b/bdns/dns.go index 36adbbee0bc..07d97d2f361 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -168,22 +168,23 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) ( // present. req.SetEdns0(4096, false) - var resp *dns.Msg - servers, err := c.servers.Addrs() if err != nil { return nil, "", fmt.Errorf("failed to list DNS servers: %w", err) } // Prepare to increment a latency metric no matter whether we succeed or fail. - // The deferred function closes over result, chosenServerIP, and tries, which + // The deferred function closes over resp, chosenServerIP, and tries, which // are all modified in the loop below. start := c.clk.Now() qtypeStr := dns.TypeToString[qtype] - result := "failed" - chosenServerIP := "" - tries := 0 + var ( + resp *dns.Msg + chosenServerIP string + tries int + ) defer func() { + result := "failed" if resp != nil { result = dns.RcodeToString[resp.Rcode] } @@ -195,12 +196,6 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) ( }).Observe(c.clk.Since(start).Seconds()) }() - type dnsRes struct { - resp *dns.Msg - err error - } - ch := make(chan dnsRes, 1) - for i := range c.maxTries { tries = i + 1 chosenServer := servers[i%len(servers)] @@ -212,64 +207,61 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) ( // and ensures that chosenServer can't be a bare port, e.g. ":1337" chosenServerIP, _, err = net.SplitHostPort(chosenServer) if err != nil { - return nil, "", err + return nil, chosenServer, err } - go func() { - resp, rtt, err := c.exchanger.Exchange(req, chosenServer) - result := "failed" - if resp != nil { - result = dns.RcodeToString[resp.Rcode] - } - if err != nil { - c.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]", chosenServer, hostname, qtypeStr, err) - } - c.queryTime.With(prometheus.Labels{ - "qtype": qtypeStr, - "result": result, - "resolver": chosenServerIP, - }).Observe(rtt.Seconds()) - ch <- dnsRes{resp: resp, err: err} - }() - select { - case <-ctx.Done(): - switch ctx.Err() { - case context.DeadlineExceeded: - result = "deadline exceeded" - case context.Canceled: - result = "canceled" - default: - result = "unknown" - } - c.timeoutCounter.With(prometheus.Labels{ - "qtype": qtypeStr, - "result": result, - "resolver": chosenServerIP, - "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), - }).Inc() - return nil, "", ctx.Err() - case r := <-ch: - if r.err != nil { - // Check if the error is a timeout error, which we want to retry. - // Network errors that can timeout implement the net.Error interface. - var netErr net.Error - isRetryable := errors.As(r.err, &netErr) && netErr.Timeout() - hasRetriesLeft := tries < c.maxTries - if isRetryable && hasRetriesLeft { - continue - } else if isRetryable && !hasRetriesLeft { - c.timeoutCounter.With(prometheus.Labels{ - "qtype": qtypeStr, - "result": "out of retries", - "resolver": chosenServerIP, - "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), - }).Inc() - } + // Do a bare assignment (not :=) to populate the `resp` used by the defer above. + var rtt time.Duration + resp, rtt, err = c.exchanger.ExchangeContext(ctx, req, chosenServer) + + // Do some metrics handling before we do error handling. + result := "failed" + if resp != nil { + result = dns.RcodeToString[resp.Rcode] + } + c.queryTime.With(prometheus.Labels{ + "qtype": qtypeStr, + "result": result, + "resolver": chosenServerIP, + }).Observe(rtt.Seconds()) + + if err != nil { + c.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]", chosenServer, hostname, qtypeStr, err) + + // Check if the error is a timeout error, which we want to retry. + // Network errors that can timeout implement the net.Error interface. + var netErr net.Error + isRetryable := errors.As(err, &netErr) && netErr.Timeout() && !errors.Is(err, context.DeadlineExceeded) + hasRetriesLeft := tries < c.maxTries + if isRetryable && hasRetriesLeft { + continue + } else if isRetryable && !hasRetriesLeft { + c.timeoutCounter.With(prometheus.Labels{ + "qtype": qtypeStr, + "result": "out of retries", + "resolver": chosenServerIP, + "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), + }).Inc() + } else if errors.Is(err, context.DeadlineExceeded) { + c.timeoutCounter.With(prometheus.Labels{ + "qtype": qtypeStr, + "result": "deadline exceeded", + "resolver": chosenServerIP, + "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), + }).Inc() + } else if errors.Is(err, context.Canceled) { + c.timeoutCounter.With(prometheus.Labels{ + "qtype": qtypeStr, + "result": "canceled", + "resolver": chosenServerIP, + "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), + }).Inc() } - // This is either a success or a non-retryable error; return either way. - return r.resp, chosenServer, r.err + return nil, chosenServer, err } + + return resp, chosenServer, nil } // It's impossible to get past the bottom of the loop: on the last attempt @@ -286,7 +278,7 @@ func (c *impl) LookupA(ctx context.Context, hostname string) (*Result[*dns.A], s return nil, resolver, err } - return resultFromMsg[*dns.A](resp), resolver, wrapErr(dns.TypeA, hostname, resp, err) + return resultFromMsg[*dns.A](resp), resolver, nil } // LookupAAAA sends a DNS query to find all AAAA records associated with the @@ -339,7 +331,7 @@ func (c *impl) LookupTXT(ctx context.Context, hostname string) (*Result[*dns.TXT // exchanger represents an underlying DNS client. This interface exists solely // so that its implementation can be swapped out in unit tests. type exchanger interface { - Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) + ExchangeContext(ctx context.Context, m *dns.Msg, a string) (*dns.Msg, time.Duration, error) } // dohExchanger implements the exchanger interface. It routes all of its DNS @@ -351,8 +343,8 @@ type dohExchanger struct { userAgent string } -// Exchange sends a DoH query to the provided DoH server and returns the response. -func (d *dohExchanger) Exchange(query *dns.Msg, server string) (*dns.Msg, time.Duration, error) { +// ExchangeContext sends a DoH query to the provided DoH server and returns the response. +func (d *dohExchanger) ExchangeContext(ctx context.Context, query *dns.Msg, server string) (*dns.Msg, time.Duration, error) { q, err := query.Pack() if err != nil { return nil, 0, err @@ -360,7 +352,7 @@ func (d *dohExchanger) Exchange(query *dns.Msg, server string) (*dns.Msg, time.D // The default Unbound URL template url := fmt.Sprintf("https://%s/dns-query", server) - req, err := http.NewRequest("POST", url, strings.NewReader(string(q))) + req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(q))) if err != nil { return nil, 0, err } diff --git a/bdns/dns_test.go b/bdns/dns_test.go index 96f58db49c5..0bbc0bcf3ba 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -608,7 +608,7 @@ type testExchanger struct { var errTooManyRequests = errors.New("too many requests") -func (te *testExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { +func (te *testExchanger) ExchangeContext(_ context.Context, m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { te.Lock() defer te.Unlock() msg := &dns.Msg{ @@ -785,16 +785,13 @@ func TestRetry(t *testing.T) { } func TestRetryMetrics(t *testing.T) { - isTimeoutErr := &url.Error{Op: "read", Err: testTimeoutError(true)} staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr}) test.AssertNotError(t, err, "Got error creating StaticProvider") testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig) dr := testClient.(*impl) - dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} - ctx, cancel := context.WithCancel(context.Background()) - cancel() - _, _, err = dr.LookupTXT(ctx, "example.com") + dr.exchanger = &testExchanger{errs: []error{context.Canceled}} + _, _, err = dr.LookupTXT(t.Context(), "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) @@ -808,10 +805,8 @@ func TestRetryMetrics(t *testing.T) { testClient = New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig) dr = testClient.(*impl) - dr.exchanger = &testExchanger{errs: []error{isTimeoutErr, isTimeoutErr, nil}} - ctx, cancel = context.WithTimeout(context.Background(), -10*time.Hour) - defer cancel() - _, _, err = dr.LookupTXT(ctx, "example.com") + dr.exchanger = &testExchanger{errs: []error{context.DeadlineExceeded}} + _, _, err = dr.LookupTXT(t.Context(), "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) @@ -839,9 +834,9 @@ type rotateFailureExchanger struct { brokenAddresses map[string]bool } -// Exchange for rotateFailureExchanger tracks the `a` argument in `lookups` and +// ExchangeContext for rotateFailureExchanger tracks the `a` argument in `lookups` and // if present in `brokenAddresses`, returns a timeout error. -func (e *rotateFailureExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { +func (e *rotateFailureExchanger) ExchangeContext(_ context.Context, m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { e.Lock() defer e.Unlock() @@ -922,7 +917,7 @@ type dohAlwaysRetryExchanger struct { err error } -func (dohE *dohAlwaysRetryExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { +func (dohE *dohAlwaysRetryExchanger) ExchangeContext(_ context.Context, m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { dohE.Lock() defer dohE.Unlock() diff --git a/va/caa.go b/va/caa.go index c0d32e34fad..d8a4d3c3559 100644 --- a/va/caa.go +++ b/va/caa.go @@ -274,12 +274,10 @@ func (va *ValidationAuthorityImpl) getCAA(ctx context.Context, hostname string) // validates them. If the identifier argument's value has a wildcard prefix then // the prefix is stripped and validation will be performed against the base // domain, honouring any issueWild CAA records encountered as appropriate. -// checkCAARecords returns four values: the first is a string indicating at +// checkCAARecords returns three values: the first is a string indicating at // which name (i.e. FQDN or parent thereof) CAA records were found, if any. The -// second is a bool indicating whether issuance for the identifier is valid. The -// unmodified *dns.CAA records that were processed/filtered are returned as the -// third argument. Any errors encountered are returned as the fourth return -// value (or nil). +// second is a bool indicating whether issuance for the identifier is valid. Any +// errors encountered are returned as the last return value (or nil). func (va *ValidationAuthorityImpl) checkCAARecords( ctx context.Context, ident identifier.ACMEIdentifier, diff --git a/va/dns.go b/va/dns.go index 3ae2a1d68bd..c4dc6a2f044 100644 --- a/va/dns.go +++ b/va/dns.go @@ -21,11 +21,11 @@ import ( "github.com/letsencrypt/boulder/identifier" ) -// getAddr will query for all A/AAAA records associated with hostname and return -// all valid addresses resolved. This is the same choice made by the Go internal -// resolution library used by net/http. If there is an error resolving the -// hostname, or if no usable IP addresses are available then a berrors.DNSError -// instance is returned with a nil netip.Addr slice. +// getAddr queries for all A/AAAA records associated with hostname, and returns +// all valid addresses resolved and the addresses of all resolvers used. If +// there is an error resolving the hostname, or if no usable IP addresses are +// available then a berrors.DNSError instance is returned with a nil netip.Addr +// slice. func (va ValidationAuthorityImpl) getAddrs(ctx context.Context, hostname string) ([]netip.Addr, []string, error) { // Kick off both the A and AAAA lookups in parallel. var wg sync.WaitGroup From 12cf297a00f86ee098b8aff156da6c5483fff87c Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Thu, 11 Dec 2025 14:34:17 -0800 Subject: [PATCH 4/5] Check for ctx.Err rather than context.DeadlineExceeded --- bdns/dns.go | 6 +++--- bdns/dns_test.go | 24 +++++++++++++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/bdns/dns.go b/bdns/dns.go index 07d97d2f361..ea91a5c4349 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -228,10 +228,10 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) ( if err != nil { c.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]", chosenServer, hostname, qtypeStr, err) - // Check if the error is a timeout error, which we want to retry. - // Network errors that can timeout implement the net.Error interface. + // Check if the error is a network timeout, rather than a local context + // timeout. If it is, retry instead of giving up. var netErr net.Error - isRetryable := errors.As(err, &netErr) && netErr.Timeout() && !errors.Is(err, context.DeadlineExceeded) + isRetryable := ctx.Err() == nil && errors.As(err, &netErr) && netErr.Timeout() hasRetriesLeft := tries < c.maxTries if isRetryable && hasRetriesLeft { continue diff --git a/bdns/dns_test.go b/bdns/dns_test.go index 0bbc0bcf3ba..b383c5623f2 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -608,7 +608,11 @@ type testExchanger struct { var errTooManyRequests = errors.New("too many requests") -func (te *testExchanger) ExchangeContext(_ context.Context, m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { +func (te *testExchanger) ExchangeContext(ctx context.Context, m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { + if ctx.Err() != nil { + return nil, 0, ctx.Err() + } + te.Lock() defer te.Unlock() msg := &dns.Msg{ @@ -788,10 +792,16 @@ func TestRetryMetrics(t *testing.T) { staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr}) test.AssertNotError(t, err, "Got error creating StaticProvider") + // This lookup should not be retried, because the error comes from the + // context itself being cancelled. It should never see the error in the + // testExchanger, because the fake exchanger (like the real http package) + // checks for cancellation before doing any work. testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig) dr := testClient.(*impl) - dr.exchanger = &testExchanger{errs: []error{context.Canceled}} - _, _, err = dr.LookupTXT(t.Context(), "example.com") + dr.exchanger = &testExchanger{errs: []error{errors.New("oops")}} + ctx, cancel := context.WithCancel(t.Context()) + cancel() + _, _, 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) @@ -803,10 +813,14 @@ func TestRetryMetrics(t *testing.T) { "resolver": "127.0.0.1", }, 1) + // Same as above, except rather than cancelling the context ourselves, we + // let the go runtime cancel it as a result of a deadline in the past. testClient = New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig) dr = testClient.(*impl) - dr.exchanger = &testExchanger{errs: []error{context.DeadlineExceeded}} - _, _, err = dr.LookupTXT(t.Context(), "example.com") + dr.exchanger = &testExchanger{errs: []error{errors.New("oops")}} + ctx, cancel = context.WithTimeout(t.Context(), -10*time.Hour) + defer cancel() + _, _, 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) From ff5beaf2a737a4c75f208a76b43cd17201ae5b1e Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Fri, 12 Dec 2025 12:58:54 -0800 Subject: [PATCH 5/5] Restore logging of dig-like string for CAA --- va/caa.go | 26 +++++++++++++++++--------- va/caa_test.go | 40 ++++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/va/caa.go b/va/caa.go index d8a4d3c3559..8bd2f186014 100644 --- a/va/caa.go +++ b/va/caa.go @@ -130,13 +130,13 @@ func (va *ValidationAuthorityImpl) checkCAA( return errors.New("expected validationMethod or accountURIID not provided to checkCAA") } - foundAt, valid, err := va.checkCAARecords(ctx, ident, params) + foundAt, valid, response, err := va.checkCAARecords(ctx, ident, params) if err != nil { return berrors.DNSError("%s", err) } - va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %d, Challenge: %s, Valid for issuance: %t, Found at: %q]", - ident.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt) + va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %d, Challenge: %s, Valid for issuance: %t, Found at: %q] Response=%q", + ident.Value, foundAt != "", params.accountURIID, params.validationMethod, valid, foundAt, response) if !valid { return berrors.CAAError("CAA record for %s prevents issuance", foundAt) } @@ -153,6 +153,7 @@ type caaResult struct { issue []*dns.CAA issuewild []*dns.CAA criticalUnknown bool + dig string resolver string err error } @@ -219,6 +220,7 @@ func (va *ValidationAuthorityImpl) parallelCAALookup(ctx context.Context, name s if r.err != nil { return } + r.dig = records.String() if len(records.Final) > 0 { r.present = true } @@ -274,14 +276,16 @@ func (va *ValidationAuthorityImpl) getCAA(ctx context.Context, hostname string) // validates them. If the identifier argument's value has a wildcard prefix then // the prefix is stripped and validation will be performed against the base // domain, honouring any issueWild CAA records encountered as appropriate. -// checkCAARecords returns three values: the first is a string indicating at +// checkCAARecords returns four values: the first is a string indicating at // which name (i.e. FQDN or parent thereof) CAA records were found, if any. The -// second is a bool indicating whether issuance for the identifier is valid. Any -// errors encountered are returned as the last return value (or nil). +// second is a bool indicating whether issuance for the identifier is valid. The +// unmodified *dns.CAA records that were processed/filtered are returned as the +// third argument. Any errors encountered are returned as the fourth return +// value (or nil). func (va *ValidationAuthorityImpl) checkCAARecords( ctx context.Context, ident identifier.ACMEIdentifier, - params *caaParams) (string, bool, error) { + params *caaParams) (string, bool, string, error) { hostname := strings.ToLower(ident.Value) // If this is a wildcard name, remove the prefix var wildcard bool @@ -291,10 +295,14 @@ func (va *ValidationAuthorityImpl) checkCAARecords( } caaSet, err := va.getCAA(ctx, hostname) if err != nil { - return "", false, err + return "", false, "", err + } + raw := "" + if caaSet != nil { + raw = caaSet.dig } valid, foundAt := va.validateCAA(caaSet, wildcard, params) - return foundAt, valid, nil + return foundAt, valid, raw, nil } // validateCAA checks a provided *caaResult. When the wildcard argument is true diff --git a/va/caa_test.go b/va/caa_test.go index 619cd5797c4..419c18ddfe0 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -414,7 +414,7 @@ func TestCAAChecking(t *testing.T) { defer mockLog.Clear() t.Run(caaTest.Name, func(t *testing.T) { ident := identifier.NewDNS(caaTest.Domain) - foundAt, valid, err := va.checkCAARecords(ctx, ident, params) + foundAt, valid, _, err := va.checkCAARecords(ctx, ident, params) if err != nil { t.Errorf("checkCAARecords error for %s: %s", caaTest.Domain, err) } @@ -442,55 +442,55 @@ func TestCAALogging(t *testing.T) { Domain: "reserved.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"reserved.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"reserved.com\"] Response=\" MsgHdr\"", }, { Domain: "reserved.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeDNS01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: dns-01, Valid for issuance: false, Found at: \"reserved.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for reserved.com, [Present: true, Account ID: 12345, Challenge: dns-01, Valid for issuance: false, Found at: \"reserved.com\"] Response=\" MsgHdr\"", }, { Domain: "mixedcase.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for mixedcase.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"mixedcase.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for mixedcase.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"mixedcase.com\"] Response=\" MsgHdr\"", }, { Domain: "critical.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for critical.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"critical.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for critical.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"critical.com\"] Response=\" MsgHdr\"", }, { Domain: "present.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"] Response=\" MsgHdr\"", }, { Domain: "not.here.but.still.present.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for not.here.but.still.present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for not.here.but.still.present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present.com\"] Response=\" MsgHdr\"", }, { Domain: "multi-crit-present.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for multi-crit-present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"multi-crit-present.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for multi-crit-present.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"multi-crit-present.com\"] Response=\" MsgHdr\"", }, { Domain: "present-with-parameter.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present-with-parameter.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present-with-parameter.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for present-with-parameter.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: true, Found at: \"present-with-parameter.com\"] Response=\" MsgHdr\"", }, { Domain: "satisfiable-wildcard-override.com", AccountURIID: 12345, ChallengeType: core.ChallengeTypeHTTP01, - ExpectedLogline: "INFO: [AUDIT] Checked CAA records for satisfiable-wildcard-override.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"satisfiable-wildcard-override.com\"]", + ExpectedLogline: "INFO: [AUDIT] Checked CAA records for satisfiable-wildcard-override.com, [Present: true, Account ID: 12345, Challenge: http-01, Valid for issuance: false, Found at: \"satisfiable-wildcard-override.com\"] Response=\" MsgHdr\"", }, } @@ -1210,9 +1210,9 @@ func TestSelectCAA(t *testing.T) { // A slice of empty caaResults should return nil, "", nil r = []caaResult{ - {"", false, nil, nil, false, "", nil}, - {"", false, nil, nil, false, "", nil}, - {"", false, nil, nil, false, "", nil}, + {"", false, nil, nil, false, "", "", nil}, + {"", false, nil, nil, false, "", "", nil}, + {"", false, nil, nil, false, "", "", nil}, } s, err = selectCAA(r) test.Assert(t, s == nil, "set is not nil") @@ -1221,8 +1221,8 @@ func TestSelectCAA(t *testing.T) { // A slice of caaResults containing an error followed by a CAA // record should return the error r = []caaResult{ - {"foo.com", false, nil, nil, false, "", errors.New("oops")}, - {"com", true, []*dns.CAA{&expected}, nil, false, "foo", nil}, + {"foo.com", false, nil, nil, false, "", "", errors.New("oops")}, + {"com", true, []*dns.CAA{&expected}, nil, false, "dig", "res", nil}, } s, err = selectCAA(r) test.Assert(t, s == nil, "set is not nil") @@ -1232,8 +1232,8 @@ func TestSelectCAA(t *testing.T) { // A slice of caaResults containing a good record that precedes an // error, should return that good record, not the error r = []caaResult{ - {"foo.com", true, []*dns.CAA{&expected}, nil, false, "foo", nil}, - {"com", false, nil, nil, false, "", errors.New("")}, + {"foo.com", true, []*dns.CAA{&expected}, nil, false, "dig", "res", nil}, + {"com", false, nil, nil, false, "", "", errors.New("")}, } s, err = selectCAA(r) test.AssertEquals(t, len(s.issue), 1) @@ -1243,9 +1243,9 @@ func TestSelectCAA(t *testing.T) { // A slice of caaResults containing multiple CAA records should // return the first non-empty CAA record r = []caaResult{ - {"bar.foo.com", false, []*dns.CAA{}, []*dns.CAA{}, false, "", nil}, - {"foo.com", true, []*dns.CAA{&expected}, nil, false, "foo", nil}, - {"com", true, []*dns.CAA{&expected}, nil, false, "bar", nil}, + {"bar.foo.com", false, []*dns.CAA{}, []*dns.CAA{}, false, "", "", nil}, + {"foo.com", true, []*dns.CAA{&expected}, nil, false, "dig", "res", nil}, + {"com", true, []*dns.CAA{&expected}, nil, false, "dig", "res", nil}, } s, err = selectCAA(r) test.AssertEquals(t, len(s.issue), 1)