diff --git a/pkg/certmanager/provision_worker.go b/pkg/certmanager/provision_worker.go index a04eace43..8652e3183 100644 --- a/pkg/certmanager/provision_worker.go +++ b/pkg/certmanager/provision_worker.go @@ -42,7 +42,6 @@ const ( // exponent at 5 (LEAST(ssl_retry_count, 5)), so normal retries only reach // exponents 0–2 before FAILED. maxProvisioningRetries = 3 - dnsExchangeTimeout = 10 * time.Second processTickTimeout = 90 * time.Second persistFailureTimeout = 15 * time.Second @@ -206,11 +205,10 @@ func (h *beginChallengeHandler) Process(ctx context.Context, certificate coredat dnsCtx, dnsSpan := h.tracer.Start(ctx, "certmanager.dns_check") dnsStarted := time.Now() - cnameCtx, cnameCancel := context.WithTimeout(dnsCtx, dnsExchangeTimeout) - err := h.dnsClient.CheckCNAME(cnameCtx, certificate.Hostname, h.cnameTarget) - - cnameCancel() - + // Exchange timeouts are applied per chain hop inside + // dnsclient.CheckCNAME so the alias walk does not share one budget + // across every lookup. + err := h.dnsClient.CheckCNAME(dnsCtx, certificate.Hostname, h.cnameTarget) if err != nil { h.acmeService.metrics.observeStep(provisionPhaseDNSCheck, provisionResultDNSError, dnsStarted) h.recordSpanError(dnsSpan, err, classifyProvisioningError(err)) diff --git a/pkg/dnsclient/checks_test.go b/pkg/dnsclient/checks_test.go index 9a656e2e8..3ca7afcfd 100644 --- a/pkg/dnsclient/checks_test.go +++ b/pkg/dnsclient/checks_test.go @@ -22,6 +22,7 @@ package dnsclient import ( "context" + "strconv" "testing" "time" @@ -50,6 +51,204 @@ func TestCheckCNAME(t *testing.T) { require.NoError(t, err) }) + t.Run("accepts alias chain reaching the target", func(t *testing.T) { + t.Parallel() + + chain := map[string]string{ + "trust.example.com.": "cname.eu.console.getprobo.com.", + "cname.eu.console.getprobo.com.": "custom.getprobo.com.", + } + + var queried []string + + client := &Client{ + exchange: func(_ context.Context, msg *dns.Msg, _ string) (*dns.Msg, error) { + name := msg.Question[0].Header().Name + queried = append(queried, name) + + target, ok := chain[name] + if !ok { + return &dns.Msg{}, nil + } + + cname := &dns.CNAME{Hdr: dns.Header{Name: name}} + cname.Target = target + + return &dns.Msg{Answer: []dns.RR{cname}}, nil + }, + } + + err := client.CheckCNAME(context.Background(), "trust.example.com", "custom.getprobo.com") + + require.NoError(t, err) + assert.Equal( + t, + []string{"trust.example.com.", "cname.eu.console.getprobo.com."}, + queried, + ) + }) + + t.Run("accepts chain returned in a single answer", func(t *testing.T) { + t.Parallel() + + var queried []string + + client := &Client{ + exchange: func(_ context.Context, msg *dns.Msg, _ string) (*dns.Msg, error) { + queried = append(queried, msg.Question[0].Header().Name) + + first := &dns.CNAME{Hdr: dns.Header{Name: "trust.example.com."}} + first.Target = "cname.eu.console.getprobo.com." + + second := &dns.CNAME{Hdr: dns.Header{Name: "cname.eu.console.getprobo.com."}} + second.Target = "custom.getprobo.com." + + third := &dns.CNAME{Hdr: dns.Header{Name: "custom.getprobo.com."}} + third.Target = "lb.example-cloud.com." + + return &dns.Msg{Answer: []dns.RR{first, second, third}}, nil + }, + } + + err := client.CheckCNAME(context.Background(), "trust.example.com", "custom.getprobo.com") + + require.NoError(t, err) + assert.Equal(t, []string{"trust.example.com."}, queried) + }) + + t.Run("rejects chain ending before the target", func(t *testing.T) { + t.Parallel() + + client := &Client{ + exchange: func(_ context.Context, msg *dns.Msg, _ string) (*dns.Msg, error) { + name := msg.Question[0].Header().Name + if name != "trust.example.com." { + return &dns.Msg{}, nil + } + + cname := &dns.CNAME{Hdr: dns.Header{Name: name}} + cname.Target = "other.example.net." + + return &dns.Msg{Answer: []dns.RR{cname}}, nil + }, + } + + err := client.CheckCNAME(context.Background(), "trust.example.com", "custom.getprobo.com") + + require.Error(t, err) + assert.Contains(t, err.Error(), `stops at "other.example.net"`) + }) + + t.Run("rejects looping chain", func(t *testing.T) { + t.Parallel() + + chain := map[string]string{ + "trust.example.com.": "alias.example.net.", + "alias.example.net.": "trust.example.com.", + } + + client := &Client{ + exchange: func(_ context.Context, msg *dns.Msg, _ string) (*dns.Msg, error) { + name := msg.Question[0].Header().Name + + cname := &dns.CNAME{Hdr: dns.Header{Name: name}} + cname.Target = chain[name] + + return &dns.Msg{Answer: []dns.RR{cname}}, nil + }, + } + + err := client.CheckCNAME(context.Background(), "trust.example.com", "custom.getprobo.com") + + require.Error(t, err) + assert.Contains(t, err.Error(), "loops back") + }) + + t.Run("rejects chain longer than the lookup budget", func(t *testing.T) { + t.Parallel() + + var queried []string + + client := &Client{ + exchange: func(_ context.Context, msg *dns.Msg, _ string) (*dns.Msg, error) { + name := msg.Question[0].Header().Name + queried = append(queried, name) + + cname := &dns.CNAME{Hdr: dns.Header{Name: name}} + cname.Target = "hop" + strconv.Itoa(len(queried)) + ".example.net." + + return &dns.Msg{Answer: []dns.RR{cname}}, nil + }, + } + + err := client.CheckCNAME(context.Background(), "trust.example.com", "custom.getprobo.com") + + require.Error(t, err) + assert.Contains(t, err.Error(), "does not reach") + assert.Len(t, queried, maxCNAMELookups) + }) + + t.Run("rejects multiple records owned by the same name", func(t *testing.T) { + t.Parallel() + + client := &Client{ + exchange: func(_ context.Context, msg *dns.Msg, _ string) (*dns.Msg, error) { + name := msg.Question[0].Header().Name + + first := &dns.CNAME{Hdr: dns.Header{Name: name}} + first.Target = "custom.getprobo.com." + + second := &dns.CNAME{Hdr: dns.Header{Name: name}} + second.Target = "other.example.net." + + return &dns.Msg{Answer: []dns.RR{first, second}}, nil + }, + } + + err := client.CheckCNAME(context.Background(), "trust.example.com", "custom.getprobo.com") + + require.Error(t, err) + assert.Contains(t, err.Error(), "multiple cname records found") + }) + + t.Run("applies exchange timeout per hop", func(t *testing.T) { + t.Parallel() + + chain := map[string]string{ + "trust.example.com.": "alias.example.net.", + "alias.example.net.": "custom.getprobo.com.", + } + + 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 + + cname := &dns.CNAME{Hdr: dns.Header{Name: name}} + cname.Target = chain[name] + + return &dns.Msg{Answer: []dns.RR{cname}}, nil + }, + } + + err := client.CheckCNAME(context.Background(), "trust.example.com", "custom.getprobo.com") + + require.NoError(t, err) + require.Len(t, deadlines, 2) + assert.True( + t, + deadlines[1].After(deadlines[0]), + "expected a fresh per-hop deadline, got shared chain deadline", + ) + }) + t.Run("rejects apex owned record for subdomain query", func(t *testing.T) { t.Parallel() diff --git a/pkg/dnsclient/cname.go b/pkg/dnsclient/cname.go index 0fc1e5b43..4f3b76c3e 100644 --- a/pkg/dnsclient/cname.go +++ b/pkg/dnsclient/cname.go @@ -28,49 +28,133 @@ import ( "codeberg.org/miekg/dns" ) -// CheckCNAME verifies that hostname has a single CNAME record owned by that -// name and pointing at expectedTarget. +const ( + maxCNAMELookups = 4 +) + func (c *Client) CheckCNAME(ctx context.Context, hostname, expectedTarget string) error { owner := ToFQDN(hostname) target := ToFQDN(expectedTarget) + current := owner + visited := map[string]struct{}{owner: {}} + + for range maxCNAMELookups { + queryCtx, cancel := c.withExchangeTimeout(ctx) + resp, err := c.queryCNAME(queryCtx, current) + + cancel() + + if err != nil { + return err + } + + edges, err := cnameEdges(resp) + if err != nil { + return err + } + + next, ok := edges[current] + if !ok { + return cnameChainStopError(resp, hostname, owner, current, target) + } + + // Resolvers disagree on QTYPE=CNAME: some answer with the first hop + // only, others chase and return every hop, so consume the hops this + // answer already carries before spending another lookup. + for { + if _, seen := visited[next]; seen { + return fmt.Errorf( + "cname chain for domain %q loops back to %q", + hostname, + trimRootDot(next), + ) + } + + visited[next] = struct{}{} + + if EqualNames(next, target) { + return nil + } + + following, ok := edges[next] + if !ok { + break + } + + next = following + } + + current = next + } + + return fmt.Errorf( + "cname chain for domain %q does not reach %q within %d lookups", + hostname, + trimRootDot(target), + maxCNAMELookups, + ) +} + +func (c *Client) queryCNAME(ctx context.Context, name string) (*dns.Msg, error) { msg := &dns.Msg{MsgHeader: dns.MsgHeader{ID: dns.ID(), RecursionDesired: true}} - msg.Question = []dns.RR{&dns.CNAME{Hdr: dns.Header{Name: owner, Class: dns.ClassINET}}} + msg.Question = []dns.RR{&dns.CNAME{Hdr: dns.Header{Name: name, Class: dns.ClassINET}}} - resp, err := c.query(ctx, msg) - if err != nil { - return err + return c.query(ctx, msg) +} + +func cnameEdges(resp *dns.Msg) (map[string]string, error) { + edges := make(map[string]string) + + for _, rr := range resp.Answer { + cname, ok := rr.(*dns.CNAME) + if !ok { + continue + } + + name := ToFQDN(cname.Hdr.Name) + if _, duplicate := edges[name]; duplicate { + return nil, fmt.Errorf("multiple cname records found for domain %q", trimRootDot(name)) + } + + edges[name] = ToFQDN(cname.Target) } - if len(resp.Answer) == 0 { - return fmt.Errorf("no cname records found for domain %q", hostname) + return edges, nil +} + +func cnameChainStopError( + resp *dns.Msg, + hostname string, + owner string, + current string, + target string, +) error { + if !EqualNames(current, owner) { + return fmt.Errorf( + "cname chain for domain %q stops at %q, expected %q", + hostname, + trimRootDot(current), + trimRootDot(target), + ) } - if len(resp.Answer) > 1 { - return fmt.Errorf("multiple cname records found for domain %q", hostname) - } + for _, rr := range resp.Answer { + cname, ok := rr.(*dns.CNAME) + if !ok { + continue + } - resolvedRecord, ok := resp.Answer[0].(*dns.CNAME) - if !ok { - return fmt.Errorf("first answer is not a cname record for domain %q", hostname) - } - - if !EqualNames(resolvedRecord.Hdr.Name, owner) { return fmt.Errorf( "cname owner mismatch: domain %q has record owned by %q", hostname, - strings.TrimSuffix(resolvedRecord.Hdr.Name, "."), + trimRootDot(cname.Hdr.Name), ) } - if !EqualNames(resolvedRecord.Target, target) { - return fmt.Errorf( - "cname target mismatch: domain %q resolves to %q, expected %q", - hostname, - strings.TrimSuffix(resolvedRecord.Target, "."), - strings.TrimSuffix(expectedTarget, "."), - ) - } - - return nil + return fmt.Errorf("no cname records found for domain %q", hostname) +} + +func trimRootDot(name string) string { + return strings.TrimSuffix(name, ".") }