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 <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
This commit is contained in:
committed by
Bryan Frimin
parent
6a1f6d9273
commit
3948f47354
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user