From 3948f47354773b92438fe683b96fa7ad0a84a41b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 24 Jul 2026 20:28:23 +0000 Subject: [PATCH] Apply CAA exchange timeout per label A single dnsExchangeTimeout around CheckCAA let slow empty answers at child names consume the budget before parent policy was queried. Give each label its own exchange timeout inside the climb instead. Signed-off-by: Cursor Agent Co-authored-by: Bryan FRIMIN --- pkg/certmanager/provision_worker.go | 7 +++--- pkg/dnsclient/caa.go | 6 ++++- pkg/dnsclient/checks_test.go | 38 +++++++++++++++++++++++++++++ pkg/dnsclient/client.go | 27 +++++++++++++++++--- 4 files changed, 70 insertions(+), 8 deletions(-) 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) {