diff --git a/pkg/certmanager/provision_worker.go b/pkg/certmanager/provision_worker.go index edcf61008..7f0e78c7c 100644 --- a/pkg/certmanager/provision_worker.go +++ b/pkg/certmanager/provision_worker.go @@ -348,10 +348,9 @@ func (h *beginChallengeHandler) loadSkipDNSChecks(ctx context.Context, hostname } func (h *beginChallengeHandler) checkCAARecords(ctx context.Context, hostname string) error { - dnsCtx, cancel := context.WithTimeout(ctx, dnsExchangeTimeout) - defer cancel() - - err := h.dnsClient.CheckCAA(dnsCtx, hostname, h.caaIssuerDomain) + // Exchange timeouts are applied per CAA label inside dnsclient.CheckCAA so + // the parent-climb budget is not shared across every lookup. + err := h.dnsClient.CheckCAA(ctx, hostname, h.caaIssuerDomain) if err == nil { return nil } diff --git a/pkg/dnsclient/caa.go b/pkg/dnsclient/caa.go index 580767c6a..c7807bbc7 100644 --- a/pkg/dnsclient/caa.go +++ b/pkg/dnsclient/caa.go @@ -43,7 +43,11 @@ func (c *Client) CheckCAA(ctx context.Context, hostname, permittedIssuer string) msg := &dns.Msg{MsgHeader: dns.MsgHeader{ID: dns.ID(), RecursionDesired: true}} msg.Question = []dns.RR{&dns.CAA{Hdr: dns.Header{Name: fqdn, Class: dns.ClassINET}}} - resp, err := c.query(ctx, msg) + // Each label gets its own exchange budget so a slow empty answer at a + // child name cannot starve the parent lookup that holds the policy. + queryCtx, cancel := c.withExchangeTimeout(ctx) + resp, err := c.query(queryCtx, msg) + cancel() if err != nil { return fmt.Errorf("cannot exchange dns message for caa records: %w", err) } diff --git a/pkg/dnsclient/checks_test.go b/pkg/dnsclient/checks_test.go index 5bdf7a25f..ded61922d 100644 --- a/pkg/dnsclient/checks_test.go +++ b/pkg/dnsclient/checks_test.go @@ -23,6 +23,7 @@ package dnsclient import ( "context" "testing" + "time" "codeberg.org/miekg/dns" "github.com/stretchr/testify/assert" @@ -484,6 +485,43 @@ func TestCheckCAA(t *testing.T) { require.Error(t, err) assert.ErrorIs(t, err, ErrCAADenied) }) + + t.Run("applies exchange timeout per label", func(t *testing.T) { + t.Parallel() + + var deadlines []time.Time + client := &Client{ + ExchangeTimeout: 2 * time.Second, + exchange: func(ctx context.Context, msg *dns.Msg, _ string) (*dns.Msg, error) { + deadline, ok := ctx.Deadline() + require.True(t, ok) + deadlines = append(deadlines, deadline) + + name := msg.Question[0].Header().Name + if name != "example.com." { + return &dns.Msg{MsgHeader: dns.MsgHeader{Rcode: dns.RcodeSuccess}}, nil + } + + caa := caaRecord("issue", "letsencrypt.org", 0) + caa.Hdr.Name = name + + return &dns.Msg{ + MsgHeader: dns.MsgHeader{Rcode: dns.RcodeSuccess}, + Answer: []dns.RR{caa}, + }, nil + }, + } + + err := client.CheckCAA(context.Background(), "trust.example.com", "letsencrypt.org") + + require.NoError(t, err) + require.GreaterOrEqual(t, len(deadlines), 2) + assert.True( + t, + deadlines[1].After(deadlines[0]), + "expected a fresh per-label deadline, got shared climb deadline", + ) + }) } func caaRecord(tag, value string, flag uint8) *dns.CAA { diff --git a/pkg/dnsclient/client.go b/pkg/dnsclient/client.go index 0f90e426e..4e18c76a9 100644 --- a/pkg/dnsclient/client.go +++ b/pkg/dnsclient/client.go @@ -23,16 +23,24 @@ package dnsclient import ( "context" "fmt" + "time" "codeberg.org/miekg/dns" ) +const ( + // DefaultExchangeTimeout is the per-lookup budget for a single DNS + // exchange (UDP, with optional TCP retry on truncation). + DefaultExchangeTimeout = 10 * time.Second +) + type ( // Client performs DNS lookups used to verify domain ownership and // certificate prerequisites. Client struct { - ResolverAddr string - exchange exchangeFunc + ResolverAddr string + ExchangeTimeout time.Duration + exchange exchangeFunc } exchangeFunc func(ctx context.Context, msg *dns.Msg, network string) (*dns.Msg, error) @@ -40,7 +48,20 @@ type ( // NewClient returns a client that resolves names through resolverAddr. func NewClient(resolverAddr string) *Client { - return &Client{ResolverAddr: resolverAddr} + return &Client{ + ResolverAddr: resolverAddr, + ExchangeTimeout: DefaultExchangeTimeout, + } +} + +// withExchangeTimeout returns a child context limited to ExchangeTimeout for a +// single DNS lookup. A zero or negative timeout leaves ctx unchanged. +func (c *Client) withExchangeTimeout(ctx context.Context) (context.Context, context.CancelFunc) { + if c.ExchangeTimeout <= 0 { + return ctx, func() {} + } + + return context.WithTimeout(ctx, c.ExchangeTimeout) } func (c *Client) exchangeUDP(ctx context.Context, msg *dns.Msg) (*dns.Msg, error) {