Allow DNS chain

Signed-off-by: Bryan Frimin <bryan@probo.com>
2026-07-27 08:30:13 +02:00
parent 4e1ecd41c2
commit 427365b1f2
3 changed files with 315 additions and 34 deletions
--- 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))
--- 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()

--- 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)

-	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}}}
+	current := owner
+	visited := map[string]struct{}{owner: {}}
+
+	for range maxCNAMELookups {
+		queryCtx, cancel := c.withExchangeTimeout(ctx)
+		resp, err := c.queryCNAME(queryCtx, current)
+
+		cancel()

-	resp, err := c.query(ctx, msg)
 		if err != nil {
 			return err
 		}

-	if len(resp.Answer) == 0 {
-		return fmt.Errorf("no cname records found for domain %q", hostname)
+		edges, err := cnameEdges(resp)
+		if err != nil {
+			return err
 		}

-	if len(resp.Answer) > 1 {
-		return fmt.Errorf("multiple cname records found for domain %q", hostname)
-	}
-
-	resolvedRecord, ok := resp.Answer[0].(*dns.CNAME)
+		next, ok := edges[current]
 		if !ok {
-		return fmt.Errorf("first answer is not a cname record for domain %q", hostname)
+			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: name, Class: dns.ClassINET}}}
+
+	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)
+	}
+
+	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),
+		)
+	}
+
+	for _, rr := range resp.Answer {
+		cname, ok := rr.(*dns.CNAME)
+		if !ok {
+			continue
 		}

-	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, ".")
 }