dns: add ACME challenge tracker and propagation check

Exact appends and deletes; values go 10m after last use or on Close.
Limits per node, one lock for provider writes. Waits for every
authoritative NS and holds a new value until the CA cache has expired.

Updates #2137

flakehashes.json

@@ -1,6 +1,6 @@
 {
   "vendor": {
-    "goModSum": "sha256-oSB3v9cn+i6QxfOQGzsjCFjVgSOTrpp9mjR7mJmzQYs=",
+    "goModSum": "sha256-X7Yx0jOestuniDSZ6Aup4MpL6ys5IdF0AOu9rS4Bzyo=",
     "sri": "sha256-KpiRQbHBAPp2BB+NodeSih2/WIlg52dfoOpYxbrVmBI="
   }
 }

go.mod

@@ -29,6 +29,7 @@ require (
 	github.com/libdns/libdns v1.1.1
 	github.com/libdns/powerdns v0.1.4
 	github.com/libdns/rfc2136 v1.0.1
+	github.com/miekg/dns v1.1.73
 	github.com/oapi-codegen/runtime v1.7.0
 	github.com/oauth2-proxy/mockoidc v0.0.0-20240214162133-caebfff84d25
 	github.com/ory/dockertest/v3 v3.12.0
@@ -178,7 +179,6 @@ require (
 	github.com/mattn/go-runewidth v0.0.31 // indirect
 	github.com/mdlayher/netlink v1.11.2 // indirect
 	github.com/mdlayher/socket v0.7.0 // indirect
-	github.com/miekg/dns v1.1.73 // indirect
 	github.com/mitchellh/go-ps v1.0.0 // indirect
 	github.com/mittwald/go-powerdns v0.6.6 // indirect
 	github.com/moby/docker-image-spec v1.3.1 // indirect

hscontrol/dns/challenge.go

@@ -0,0 +1,302 @@
+package dns
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+	"sync"
+	"time"
+
+	"github.com/libdns/libdns"
+	"github.com/rs/zerolog/log"
+)
+
+var (
+	ErrTooManyChallenges = errors.New("too many pending ACME challenges for this node")
+	ErrChallengeProvider = errors.New("DNS provider failed")
+	ErrEmptyChallenge    = errors.New("empty ACME challenge value")
+	ErrChallengesClosed  = errors.New("ACME challenges are shutting down")
+)
+
+const (
+	// ChallengeTTL is the TTL of every challenge record headscale writes.
+	ChallengeTTL = time.Minute
+	// challengeLifetime outlives both authorizations of a wildcard order
+	// and the CA's validation retries. A retry of the same order resends
+	// the same value, which restarts the clock.
+	challengeLifetime = 10 * time.Minute
+	// maxChallengesPerNode covers a wildcard order (two values) racing a
+	// plain order. It also bounds how many records one node can make
+	// headscale write. The limit is per node, not per name: a node renames
+	// itself by changing its hostname, and every new name would come with
+	// fresh slots.
+	maxChallengesPerNode = 4
+	providerTimeout      = 30 * time.Second
+	// caLookupDelay is how long after Present returns the CA may still
+	// make its first lookup of the value.
+	caLookupDelay = 5 * time.Second
+)
+
+// Challenges publishes ACME DNS-01 TXT values through a [Provider] and
+// removes them again. Clients never ask for removal, so every value is
+// deleted [challengeLifetime], plus the propagation timeout, after it was
+// last published, or on [Challenges.Close]. Nothing is persisted: a crash,
+// or a failed delete, leaves a few random TXT values behind.
+type Challenges struct {
+	provider Provider
+	zone     string
+	lifetime time.Duration
+	wait     func(ctx context.Context, fqdn, value string) error
+
+	// mu guards the fields below and is held across provider calls.
+	// Providers that read-modify-write a record set lose a value when two
+	// changes to one name overlap, and the changes can come from different
+	// nodes: one renames away while its values are live, another takes the
+	// name. Issuance is rare, so one lock for every name is enough.
+	mu     sync.Mutex
+	values map[challengeKey]*challengeValue
+	// waiting counts each node's Present calls waiting for propagation, so
+	// duplicate requests cannot pile up DNS queries.
+	waiting map[uint64]int
+	// closed refuses new values once Close has run. Noise connections
+	// outlive the HTTP server's shutdown, so Present still gets called.
+	closed bool
+}
+
+type challengeKey struct{ fqdn, value string }
+
+type challengeValue struct {
+	// owner is the node the value counts against.
+	owner uint64
+	// record is what the provider returned from AppendRecords; deleting
+	// exactly it matters for providers that match on TTL or record ID.
+	record libdns.Record
+	timer  *time.Timer
+	// expires moves on every use instead of resetting timer: a timer that
+	// already fired, blocked on the lock behind that use, must not delete
+	// the value.
+	expires time.Time
+	// shownAt is when Present returned, or will return, for this value.
+	shownAt time.Time
+}
+
+// NewChallenges returns a [Challenges] writing into zone. Present waits up
+// to propagationTimeout for the zone's nameservers, found through
+// resolvers, to serve a value.
+func NewChallenges(
+	provider Provider,
+	zone string,
+	resolvers []string,
+	propagationTimeout time.Duration,
+) *Challenges {
+	return &Challenges{
+		provider: provider,
+		zone:     fqdn(zone),
+		// A value must outlive its own wait and hold, or it is deleted
+		// while Present still waits on it, and its hold with it.
+		lifetime: challengeLifetime + propagationTimeout,
+		wait:     newTXTChecker(resolvers, propagationTimeout).wait,
+		values:   make(map[challengeKey]*challengeValue),
+		waiting:  make(map[uint64]int),
+	}
+}
+
+// Present publishes value as a TXT record at name for the node owner and
+// returns once the zone's nameservers serve it. Values are added next to
+// existing ones, never replacing them, and publishing a value again is a
+// no-op.
+func (c *Challenges) Present(ctx context.Context, owner uint64, name, value string) error {
+	if value == "" {
+		return ErrEmptyChallenge
+	}
+
+	k := challengeKey{fqdn: fqdn(name), value: value}
+
+	err := c.append(ctx, owner, k)
+	if err != nil {
+		return err
+	}
+
+	err = c.wait(ctx, k.fqdn, value)
+	if err == nil {
+		select {
+		case <-ctx.Done():
+			err = ctx.Err()
+		case <-time.After(time.Until(c.show(k))):
+		}
+	}
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	c.waiting[owner]--
+	if c.waiting[owner] == 0 {
+		delete(c.waiting, owner)
+	}
+
+	// A failed wait keeps the value until its lifetime ends: deleting it
+	// at once would let a client that cancels requests in a loop make
+	// headscale call the provider without bound. A success restarts the
+	// lifetime, which is the CA's window to validate.
+	if v, ok := c.values[k]; ok && err == nil {
+		v.expires = time.Now().Add(c.lifetime)
+	}
+
+	return err
+}
+
+// append publishes k for owner unless it already is, and counts the caller
+// as waiting.
+func (c *Challenges) append(ctx context.Context, owner uint64, k challengeKey) error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	if c.closed {
+		return ErrChallengesClosed
+	}
+
+	if c.waiting[owner] >= maxChallengesPerNode {
+		return ErrTooManyChallenges
+	}
+
+	// A node that resends a value another node published, on a name it has
+	// taken over, keeps that value alive, so the value moves to its count.
+	v, ok := c.values[k]
+	if (!ok || v.owner != owner) && c.count(owner) >= maxChallengesPerNode {
+		return ErrTooManyChallenges
+	}
+
+	if ok {
+		v.owner = owner
+		v.expires = time.Now().Add(c.lifetime)
+		c.waiting[owner]++
+
+		return nil
+	}
+
+	record := libdns.TXT{
+		Name: libdns.RelativeName(k.fqdn, c.zone),
+		TTL:  ChallengeTTL,
+		Text: k.value,
+	}
+
+	// The call must finish even if the client goes away, or a record
+	// could be created that nothing tracks and deletes.
+	ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), providerTimeout)
+	defer cancel()
+
+	got, err := c.provider.AppendRecords(ctx, c.zone, []libdns.Record{record})
+	if err != nil {
+		return fmt.Errorf("%w: %w", ErrChallengeProvider, err)
+	}
+
+	stored := libdns.Record(record)
+	if len(got) == 1 && got[0].RR().Type == "TXT" && got[0].RR().Data == k.value {
+		stored = got[0]
+	}
+
+	c.values[k] = &challengeValue{
+		owner:   owner,
+		record:  stored,
+		expires: time.Now().Add(c.lifetime),
+		timer:   time.AfterFunc(c.lifetime, func() { c.expire(k) }), //nolint:contextcheck // outlives the request
+	}
+	c.waiting[owner]++
+
+	return nil
+}
+
+// count returns how many values owner has published; c.mu must be held.
+func (c *Challenges) count(owner uint64) int {
+	n := 0
+
+	for _, v := range c.values {
+		if v.owner == owner {
+			n++
+		}
+	}
+
+	return n
+}
+
+// show reserves when Present may return for k. The client asks the CA to
+// validate at once, and the CA's resolvers cache the record set they find
+// for its TTL. A resolver that cached this name for an earlier value, such
+// as the first of a wildcard order or one left by the node that had the
+// name before, would not see a new one, so a new value is held back until
+// that cache has expired. The hold counts from every other value on the name,
+// so a resend cannot cut its own hold short. Another name, such as the
+// node's name before a rename, is cached on its own.
+func (c *Challenges) show(k challengeKey) time.Time {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	at := time.Now()
+
+	for other, v := range c.values {
+		expired := v.shownAt.Add(ChallengeTTL + caLookupDelay)
+		if other.fqdn == k.fqdn && other != k && expired.After(at) {
+			at = expired
+		}
+	}
+
+	if v, ok := c.values[k]; ok {
+		v.shownAt = at
+	}
+
+	return at
+}
+
+// expire removes k once it has gone unused for the lifetime.
+func (c *Challenges) expire(k challengeKey) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	v, ok := c.values[k]
+	if !ok {
+		return
+	}
+
+	if d := time.Until(v.expires); d > 0 {
+		v.timer.Reset(d)
+
+		return
+	}
+
+	c.remove(k)
+}
+
+// remove deletes one tracked value from the provider; c.mu must be held.
+// It never deletes by name alone: providers treat an empty value as
+// "every record here", which would take out the operator's records and
+// the other value of a wildcard order.
+func (c *Challenges) remove(k challengeKey) {
+	v := c.values[k]
+	v.timer.Stop()
+	delete(c.values, k)
+
+	ctx, cancel := context.WithTimeout(context.Background(), providerTimeout)
+	defer cancel()
+
+	_, err := c.provider.DeleteRecords(ctx, c.zone, []libdns.Record{v.record})
+	if err != nil {
+		log.Warn().Err(err).Str("fqdn", k.fqdn).Msg("removing ACME challenge record")
+	}
+}
+
+// Close deletes every pending value and refuses new ones.
+func (c *Challenges) Close() {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	c.closed = true
+
+	for k := range c.values {
+		c.remove(k)
+	}
+}
+
+func fqdn(name string) string {
+	return strings.ToLower(strings.TrimSuffix(name, ".")) + "."
+}

hscontrol/dns/challenge_test.go

@@ -0,0 +1,404 @@
+package dns
+
+import (
+	"context"
+	"errors"
+	"sync/atomic"
+	"testing"
+	"testing/synctest"
+	"time"
+
+	"github.com/juanfont/headscale/hscontrol/dns/dnstest"
+	"github.com/libdns/libdns"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	testZone = "ts.example.com."
+	testName = "_acme-challenge.node.ts.example.com."
+	// renamedName is testName's node after it changed its hostname.
+	renamedName = "_acme-challenge.renamed.ts.example.com."
+	testOwner   = 1
+	// operatorValue is a record headscale did not create and must keep.
+	operatorValue = "operator-owned"
+)
+
+var errAPIDown = errors.New("api down")
+
+func newTestChallenges(p *dnstest.Provider) *Challenges {
+	return NewChallenges(p, "TS.example.com", nil, 0)
+}
+
+func TestChallengesAppendNotReplace(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		p := &dnstest.Provider{}
+		p.Add(testName, operatorValue)
+		c := newTestChallenges(p)
+		ctx := context.Background()
+
+		// A wildcard order sends two values for one name, one after the other.
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-1"))
+		require.NoError(t, c.Present(ctx, testOwner, "_acme-challenge.NODE.ts.example.com", "value-2"))
+		assert.ElementsMatch(t, []string{operatorValue, "value-1", "value-2"}, p.Values(testName))
+
+		// A retry of the same order resends the same value.
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-1"))
+		assert.Len(t, p.Calls(), 2, "resent value must not be appended again")
+
+		for _, call := range p.Calls() {
+			assert.Equal(t, testZone, call.Zone)
+		}
+
+		time.Sleep(challengeLifetime) //nolint:forbidigo // synctest fake clock
+		synctest.Wait()
+
+		assert.Equal(t, []string{operatorValue}, p.Values(testName))
+	})
+}
+
+// TestChallengesResendExtendsLifetime also pins that an expiry timer that
+// fired just before a resend, and then waited for the lock, leaves the
+// refreshed value alone.
+func TestChallengesResendExtendsLifetime(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		p := &dnstest.Provider{}
+		c := newTestChallenges(p)
+		ctx := context.Background()
+
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value"))
+		time.Sleep(challengeLifetime - time.Minute) //nolint:forbidigo // synctest fake clock
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value"))
+
+		c.expire(challengeKey{fqdn: testName, value: "value"})
+		time.Sleep(2 * time.Minute) //nolint:forbidigo // synctest fake clock
+		synctest.Wait()
+		assert.Equal(t, []string{"value"}, p.Values(testName))
+
+		time.Sleep(challengeLifetime) //nolint:forbidigo // synctest fake clock
+		synctest.Wait()
+		assert.Empty(t, p.Values(testName))
+	})
+}
+
+func TestChallengesLimitPerNode(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		p := &dnstest.Provider{}
+		c := newTestChallenges(p)
+		ctx := context.Background()
+
+		for i := range maxChallengesPerNode {
+			require.NoError(t, c.Present(ctx, testOwner, testName, "value-"+string(rune('a'+i))))
+		}
+
+		err := c.Present(ctx, testOwner, testName, "one-too-many")
+		require.ErrorIs(t, err, ErrTooManyChallenges)
+
+		// Renaming gives the node a new name, not new slots.
+		err = c.Present(ctx, testOwner, renamedName, "value")
+		require.ErrorIs(t, err, ErrTooManyChallenges)
+		assert.Len(t, p.Calls(), maxChallengesPerNode)
+
+		// Other nodes have their own limit.
+		require.NoError(t, c.Present(ctx, testOwner+1, "_acme-challenge.other.ts.example.com", "value"))
+
+		// A node that took the name over and resends a value there takes
+		// over its slot.
+		require.NoError(t, c.Present(ctx, testOwner+1, testName, "value-a"))
+		require.NoError(t, c.Present(ctx, testOwner, renamedName, "value"))
+
+		c.Close()
+	})
+}
+
+// TestChallengesHoldsNewValueForCACache pins that a second value on a name
+// is answered only once the CA's cache of the first has expired: the CA
+// validates the second value of a wildcard order seconds after the first.
+func TestChallengesHoldsNewValueForCACache(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		c := newTestChallenges(&dnstest.Provider{})
+		ctx := context.Background()
+		start := time.Now()
+
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-1"))
+		assert.Equal(t, start, time.Now(), "first value is not held")
+
+		require.NoError(t, c.Present(ctx, testOwner, renamedName, "value-2"))
+		assert.Equal(t, start, time.Now(), "other names, such as after a rename, are not held")
+
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-2"))
+		assert.Equal(t, start.Add(ChallengeTTL+caLookupDelay), time.Now())
+
+		shown := time.Now()
+
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-2"))
+		assert.Equal(t, shown, time.Now(), "a resent value is not held")
+
+		require.NoError(t, c.Present(ctx, testOwner+1, testName, "value-3"))
+		assert.Equal(t, shown.Add(ChallengeTTL+caLookupDelay), time.Now(), "a node that took the name over is held too")
+
+		ctx, cancel := context.WithCancel(ctx)
+		cancel()
+		require.ErrorIs(t, c.Present(ctx, testOwner, testName, "value-4"), context.Canceled)
+
+		c.Close()
+	})
+}
+
+// TestChallengesResendKeepsHold pins that resending a value while its hold
+// is pending does not cut the hold short.
+func TestChallengesResendKeepsHold(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		c := newTestChallenges(&dnstest.Provider{})
+		ctx := context.Background()
+		start := time.Now()
+
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-1"))
+
+		go func() { _ = c.Present(ctx, testOwner, testName, "value-2") }()
+
+		time.Sleep(time.Second) //nolint:forbidigo // synctest fake clock
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-2"))
+		assert.Equal(t, start.Add(ChallengeTTL+caLookupDelay), time.Now())
+
+		c.Close()
+	})
+}
+
+// TestChallengesFailedWaitKeepsValue pins that a client that went away
+// still gets its value appended and tracked, and that a failed or
+// cancelled wait leaves the value for its lifetime: deleting at once would
+// let a client that cancels in a loop drive unbounded provider calls.
+func TestChallengesFailedWaitKeepsValue(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		p := &dnstest.Provider{}
+		c := newTestChallenges(p)
+		c.wait = func(ctx context.Context, _, _ string) error { return ctx.Err() }
+
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+
+		require.ErrorIs(t, c.Present(ctx, testOwner, testName, "value"), context.Canceled)
+		assert.Equal(t, []string{"value"}, p.Values(testName), "append must finish for a gone client")
+
+		// A retry of the same value is not appended again.
+		require.NoError(t, c.Present(context.Background(), testOwner, testName, "value"))
+		assert.Len(t, p.Calls(), 1)
+
+		time.Sleep(challengeLifetime) //nolint:forbidigo // synctest fake clock
+		synctest.Wait()
+		assert.Empty(t, p.Values(testName))
+	})
+}
+
+// TestChallengesLimitsWaiters pins that duplicate requests for one name
+// cannot pile up propagation waits.
+func TestChallengesLimitsWaiters(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		p := &dnstest.Provider{}
+		c := newTestChallenges(p)
+
+		release := make(chan struct{})
+		c.wait = func(context.Context, string, string) error {
+			<-release
+
+			return nil
+		}
+
+		for range maxChallengesPerNode {
+			go func() { _ = c.Present(context.Background(), testOwner, testName, "value") }()
+		}
+
+		synctest.Wait()
+		require.ErrorIs(t, c.Present(context.Background(), testOwner, testName, "value"), ErrTooManyChallenges)
+
+		close(release)
+		synctest.Wait()
+
+		c.wait = func(context.Context, string, string) error { return nil }
+		require.NoError(t, c.Present(context.Background(), testOwner, testName, "value"))
+		c.Close()
+	})
+}
+
+func TestChallengesProviderError(t *testing.T) {
+	p := &dnstest.Provider{}
+	p.SetError(errAPIDown)
+	c := newTestChallenges(p)
+
+	err := c.Present(context.Background(), testOwner, testName, "value")
+	require.ErrorIs(t, err, ErrChallengeProvider)
+
+	p.SetError(nil)
+	c.Close()
+	assert.Len(t, p.Calls(), 1, "failed append must not be tracked for deletion")
+}
+
+func TestChallengesClose(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		p := &dnstest.Provider{}
+		p.Add(testName, operatorValue)
+		c := newTestChallenges(p)
+		ctx := context.Background()
+
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-1"))
+		require.NoError(t, c.Present(ctx, testOwner, testName, "value-2"))
+		c.Close()
+
+		assert.Equal(t, []string{operatorValue}, p.Values(testName))
+
+		for _, call := range p.Calls() {
+			assert.NotEmpty(t, call.Value, "a delete without value wipes the record set")
+		}
+	})
+}
+
+// appendGate holds AppendRecords until release is closed.
+type appendGate struct {
+	*dnstest.Provider
+
+	entered, release chan struct{}
+}
+
+func (p appendGate) AppendRecords(
+	ctx context.Context,
+	zone string,
+	recs []libdns.Record,
+) ([]libdns.Record, error) {
+	close(p.entered)
+	<-p.release
+
+	return p.Provider.AppendRecords(ctx, zone, recs)
+}
+
+// TestChallengesCloseRefusesNewValues pins that Close removes an append
+// already under way and refuses later ones: Noise connections outlive
+// the HTTP server's shutdown, so Present can still be called.
+func TestChallengesCloseRefusesNewValues(t *testing.T) {
+	p := appendGate{Provider: &dnstest.Provider{}, entered: make(chan struct{}), release: make(chan struct{})}
+	c := NewChallenges(p, testZone, nil, 0)
+
+	presented := make(chan error)
+
+	go func() { presented <- c.Present(context.Background(), testOwner, testName, "in-flight") }()
+
+	<-p.entered
+
+	closed := make(chan struct{})
+
+	go func() {
+		c.Close()
+		close(closed)
+	}()
+
+	close(p.release)
+	<-closed
+	require.NoError(t, <-presented)
+	assert.Empty(t, p.Values(testName), "append in flight during Close must be removed")
+
+	err := c.Present(context.Background(), testOwner+1, testName, "late")
+	require.ErrorIs(t, err, ErrChallengesClosed)
+	assert.Len(t, p.Calls(), 2, "only the in-flight append and its delete")
+}
+
+func TestChallengesEmptyValue(t *testing.T) {
+	p := &dnstest.Provider{}
+	c := newTestChallenges(p)
+
+	require.ErrorIs(t, c.Present(context.Background(), testOwner, testName, ""), ErrEmptyChallenge)
+	assert.Empty(t, p.Calls())
+}
+
+// serialProvider records whether two provider calls overlapped. Providers
+// that read a record set and, one API round trip later, write back what
+// they computed from it, as PowerDNS does, lose one of two overlapping
+// changes to a name.
+type serialProvider struct {
+	*dnstest.Provider
+
+	busy, overlapped atomic.Bool
+	// entered gets a value as each call starts.
+	entered chan struct{}
+}
+
+func (p *serialProvider) call() func() {
+	if !p.busy.CompareAndSwap(false, true) {
+		p.overlapped.Store(true)
+	}
+
+	p.entered <- struct{}{}
+
+	time.Sleep(20 * time.Millisecond) //nolint:forbidigo // the API round trip
+
+	return func() { p.busy.Store(false) }
+}
+
+func (p *serialProvider) AppendRecords(
+	ctx context.Context,
+	zone string,
+	recs []libdns.Record,
+) ([]libdns.Record, error) {
+	defer p.call()()
+
+	return p.Provider.AppendRecords(ctx, zone, recs)
+}
+
+func (p *serialProvider) DeleteRecords(
+	ctx context.Context,
+	zone string,
+	recs []libdns.Record,
+) ([]libdns.Record, error) {
+	defer p.call()()
+
+	return p.Provider.DeleteRecords(ctx, zone, recs)
+}
+
+// TestChallengesNameTakenOver pins that a node that took over a name
+// cannot interleave its provider writes with the removal of a value the
+// node that had the name left there, whichever starts first. It runs on
+// the real clock: synctest cannot advance time while a goroutine waits on
+// a mutex.
+func TestChallengesNameTakenOver(t *testing.T) {
+	p := &serialProvider{Provider: &dnstest.Provider{}, entered: make(chan struct{}, 8)}
+	c := NewChallenges(p, testZone, nil, 0)
+	ctx := context.Background()
+
+	for i, name := range []string{testName, renamedName} {
+		require.NoError(t, c.Present(ctx, testOwner, name, "old"))
+		<-p.entered
+
+		// testOwner renamed away, and "old" expires while the node that
+		// took the name publishes there. It was shown long ago, so nothing
+		// is held.
+		old := challengeKey{fqdn: name, value: "old"}
+
+		c.mu.Lock()
+		c.values[old].expires = time.Time{}
+		c.values[old].shownAt = time.Time{}
+		c.mu.Unlock()
+
+		first := func() { c.expire(old) }
+		second := func() { assert.NoError(t, c.Present(ctx, testOwner+1, name, "new")) }
+
+		if i == 1 {
+			first, second = second, first
+		}
+
+		done := make(chan struct{})
+
+		go func() {
+			first()
+			close(done)
+		}()
+
+		<-p.entered
+		second()
+		<-done
+		<-p.entered
+
+		assert.Equal(t, []string{"new"}, p.Values(name))
+	}
+
+	assert.False(t, p.overlapped.Load(), "provider calls overlapped")
+	c.Close()
+}

hscontrol/dns/dnstest/provider.go

@@ -0,0 +1,150 @@
+// Package dnstest provides an in-memory DNS provider for tests.
+package dnstest
+
+import (
+	"context"
+	"slices"
+	"sync"
+
+	"github.com/libdns/libdns"
+)
+
+// Call is one provider call, one entry per record.
+type Call struct {
+	Method string
+	Zone   string
+	Name   string
+	Value  string
+}
+
+// Provider keeps TXT records in memory and records every call. The zero
+// value is ready to use and safe for concurrent use.
+type Provider struct {
+	mu      sync.Mutex
+	records map[string][]string
+	calls   []Call
+	err     error
+}
+
+// SetError makes every following call fail with err; nil clears it.
+func (p *Provider) SetError(err error) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	p.err = err
+}
+
+// AppendRecords fails for a done context, as a provider calling an API
+// would, so tests catch a caller that lets a request's end abandon a
+// write.
+func (p *Provider) AppendRecords(
+	ctx context.Context,
+	zone string,
+	recs []libdns.Record,
+) ([]libdns.Record, error) {
+	err := ctx.Err()
+	if err != nil {
+		return nil, err
+	}
+
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	out := make([]libdns.Record, 0, len(recs))
+
+	for _, rec := range recs {
+		rr := rec.RR()
+		name := libdns.AbsoluteName(rr.Name, zone)
+		p.calls = append(p.calls, Call{Method: "append", Zone: zone, Name: name, Value: rr.Data})
+
+		if p.err != nil {
+			return out, p.err
+		}
+
+		if p.records == nil {
+			p.records = make(map[string][]string)
+		}
+
+		p.records[name] = append(p.records[name], rr.Data)
+		out = append(out, libdns.TXT{Name: rr.Name, TTL: rr.TTL, Text: rr.Data, ProviderData: name})
+	}
+
+	return out, nil
+}
+
+// DeleteRecords follows the libdns contract: an empty value deletes every
+// value at the name. Tests that keep an unrelated value at the same name
+// therefore catch a caller that would wipe a whole record set. Like
+// AppendRecords, it fails for a done context.
+func (p *Provider) DeleteRecords(
+	ctx context.Context,
+	zone string,
+	recs []libdns.Record,
+) ([]libdns.Record, error) {
+	err := ctx.Err()
+	if err != nil {
+		return nil, err
+	}
+
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	var out []libdns.Record
+
+	for _, rec := range recs {
+		rr := rec.RR()
+		name := libdns.AbsoluteName(rr.Name, zone)
+		p.calls = append(p.calls, Call{Method: "delete", Zone: zone, Name: name, Value: rr.Data})
+
+		if p.err != nil {
+			return out, p.err
+		}
+
+		if rr.Data == "" {
+			delete(p.records, name)
+
+			out = append(out, rec)
+
+			continue
+		}
+
+		i := slices.Index(p.records[name], rr.Data)
+		if i < 0 {
+			continue
+		}
+
+		p.records[name] = slices.Delete(p.records[name], i, i+1)
+
+		out = append(out, rec)
+	}
+
+	return out, nil
+}
+
+// Add stores a value directly, as if the operator had created it.
+func (p *Provider) Add(name, value string) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	if p.records == nil {
+		p.records = make(map[string][]string)
+	}
+
+	p.records[name] = append(p.records[name], value)
+}
+
+// Values returns the TXT values stored at the fully qualified name.
+func (p *Provider) Values(name string) []string {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	return slices.Clone(p.records[name])
+}
+
+// Calls returns every call made so far.
+func (p *Provider) Calls() []Call {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	return slices.Clone(p.calls)
+}

hscontrol/dns/propagation.go

@@ -0,0 +1,180 @@
+package dns
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net"
+	"slices"
+	"strings"
+	"sync"
+	"sync/atomic"
+	"time"
+
+	"github.com/rs/zerolog/log"
+)
+
+var ErrNotPropagated = errors.New("TXT record not visible on all authoritative nameservers")
+
+const propagationInterval = 2 * time.Second
+
+// txtChecker waits until every authoritative nameserver of a zone serves
+// a TXT value. The client asks the CA to validate as soon as set-dns
+// returns, so answering early fails the challenge and costs the node a
+// failed authorization at the CA.
+type txtChecker struct {
+	resolvers []string
+	timeout   time.Duration
+	interval  time.Duration
+	// port the authoritative nameservers are queried on.
+	port string
+}
+
+func newTXTChecker(resolvers []string, timeout time.Duration) txtChecker {
+	return txtChecker{
+		resolvers: resolvers,
+		timeout:   timeout,
+		interval:  propagationInterval,
+		port:      "53",
+	}
+}
+
+// wait returns nil once each nameserver of the zone holding fqdn answers
+// it with value.
+// A timeout of zero disables the check.
+func (c txtChecker) wait(ctx context.Context, fqdn, value string) error {
+	if c.timeout <= 0 {
+		return nil
+	}
+
+	start := time.Now()
+
+	ctx, cancel := context.WithTimeout(ctx, c.timeout)
+	defer cancel()
+
+	recursive := resolverAt(c.resolvers...)
+
+	nameservers, err := nameserversOf(ctx, recursive, fqdn)
+	if err != nil {
+		return fmt.Errorf("looking up nameservers of %s: %w", fqdn, c.via(err))
+	}
+
+	// A nameserver counts as done when any of its addresses has the value:
+	// the host may lack a route to one address family.
+	pending := make(map[string][]string, len(nameservers))
+	for _, ns := range nameservers {
+		addrs, err := recursive.LookupHost(ctx, ns.Host)
+		if err != nil {
+			return fmt.Errorf("resolving nameserver %s: %w", ns.Host, c.via(err))
+		}
+
+		pending[ns.Host] = addrs
+	}
+
+	for {
+		// Every address is asked at once, so a silent one delays a round by
+		// at most one query.
+		var (
+			mu    sync.Mutex
+			wg    sync.WaitGroup
+			found = make(map[string]string)
+		)
+
+		for host, addrs := range pending {
+			for _, addr := range addrs {
+				wg.Go(func() {
+					// Bound each query: the resolver's own timeout and retries
+					// come from /etc/resolv.conf and make one silent address
+					// cost tens of seconds.
+					qctx, cancel := context.WithTimeout(ctx, c.interval)
+					defer cancel()
+
+					txts, _ := resolverAt(net.JoinHostPort(addr, c.port)).LookupTXT(qctx, fqdn)
+					if slices.Contains(txts, value) {
+						mu.Lock()
+						found[host] = addr
+						mu.Unlock()
+					}
+				})
+			}
+		}
+
+		wg.Wait()
+
+		for host, addr := range found {
+			log.Debug().
+				Str("fqdn", fqdn).
+				Str("nameserver", host).
+				Str("addr", addr).
+				Dur("elapsed", time.Since(start)).
+				Msg("ACME challenge visible on nameserver")
+			delete(pending, host)
+		}
+
+		if len(pending) == 0 {
+			return nil
+		}
+
+		select {
+		case <-ctx.Done():
+			hosts := make([]string, 0, len(pending))
+			for host := range pending {
+				hosts = append(hosts, host)
+			}
+
+			slices.Sort(hosts)
+
+			return fmt.Errorf("%w: %s waiting for %s", ErrNotPropagated, fqdn, strings.Join(hosts, ", "))
+		case <-time.After(c.interval):
+		}
+	}
+}
+
+// nameserversOf returns the nameservers of the closest enclosing name of
+// name that has any, which is the zone holding it.
+func nameserversOf(ctx context.Context, r *net.Resolver, name string) ([]*net.NS, error) {
+	for {
+		nameservers, err := r.LookupNS(ctx, name)
+
+		_, parent, _ := strings.Cut(name, ".")
+		if !isNotFound(err) || parent == "" {
+			return nameservers, err
+		}
+
+		name = parent
+	}
+}
+
+func isNotFound(err error) bool {
+	dnsErr, ok := errors.AsType[*net.DNSError](err)
+
+	return ok && dnsErr.IsNotFound
+}
+
+// via names the resolvers a lookup went to: Go reports the server from
+// /etc/resolv.conf, which the custom dialer never contacts.
+func (c txtChecker) via(err error) error {
+	if dnsErr, ok := errors.AsType[*net.DNSError](err); ok {
+		dnsErr.Server = strings.Join(c.resolvers, ", ")
+	}
+
+	return err
+}
+
+// resolverAt returns a resolver that sends queries to addrs instead of the
+// servers in /etc/resolv.conf. Each dial takes the next address, so the
+// resolver's retries move on to another server when one does not answer.
+func resolverAt(addrs ...string) *net.Resolver {
+	var next atomic.Uint32
+
+	return &net.Resolver{
+		PreferGo: true,
+		Dial: func(ctx context.Context, network, _ string) (net.Conn, error) {
+			var d net.Dialer
+
+			addr := addrs[int(next.Add(1)-1)%len(addrs)]
+
+			return d.DialContext(ctx, network, addr)
+		},
+	}
+}

hscontrol/dns/propagation_test.go

@@ -0,0 +1,154 @@
+package dns
+
+import (
+	"context"
+	"net"
+	"runtime"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	mdns "github.com/miekg/dns"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// fakeNameserver answers authoritatively for testZone: ns1 and ns2 are
+// the zone's nameservers, and TXT answers come from txt.
+type fakeNameserver struct {
+	mu  sync.Mutex
+	txt map[string][]string
+	// hideFor makes the first n TXT queries miss, like a nameserver that
+	// has not caught up yet.
+	hideFor atomic.Int32
+}
+
+func (f *fakeNameserver) set(values ...string) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	f.txt[testName] = values
+}
+
+func (f *fakeNameserver) ServeDNS(w mdns.ResponseWriter, r *mdns.Msg) {
+	m := new(mdns.Msg)
+	m.SetReply(r)
+	m.Authoritative = true
+
+	q := r.Question[0]
+	hdr := mdns.RR_Header{Name: q.Name, Class: mdns.ClassINET, Ttl: 60, Rrtype: q.Qtype}
+
+	switch q.Qtype {
+	case mdns.TypeNS:
+		m.Answer = []mdns.RR{
+			&mdns.NS{Hdr: hdr, Ns: "ns1." + testZone},
+			&mdns.NS{Hdr: hdr, Ns: "ns2." + testZone},
+		}
+	case mdns.TypeA:
+		switch q.Name {
+		case "ns1." + testZone:
+			m.Answer = []mdns.RR{&mdns.A{Hdr: hdr, A: net.IPv4(127, 0, 0, 1)}}
+		case "ns2." + testZone:
+			m.Answer = []mdns.RR{&mdns.A{Hdr: hdr, A: net.IPv4(127, 0, 0, 2)}}
+		}
+	case mdns.TypeTXT:
+		if f.hideFor.Add(-1) >= 0 {
+			break
+		}
+
+		f.mu.Lock()
+		for _, v := range f.txt[q.Name] {
+			m.Answer = append(m.Answer, &mdns.TXT{Hdr: hdr, Txt: []string{v}})
+		}
+		f.mu.Unlock()
+	}
+
+	if len(m.Answer) == 0 {
+		m.Ns = []mdns.RR{&mdns.SOA{
+			Hdr:     mdns.RR_Header{Name: testZone, Rrtype: mdns.TypeSOA, Class: mdns.ClassINET, Ttl: 60},
+			Ns:      "ns1." + testZone,
+			Mbox:    "hostmaster." + testZone,
+			Serial:  1,
+			Refresh: 60, Retry: 60, Expire: 60, Minttl: 60,
+		}}
+	}
+
+	_ = w.WriteMsg(m)
+}
+
+// startNameservers runs ns1 on 127.0.0.1 and ns2 on 127.0.0.2, both on the
+// same port, and returns a checker using ns1 as its recursive resolver.
+func startNameservers(t *testing.T) (txtChecker, *fakeNameserver, *fakeNameserver) {
+	t.Helper()
+
+	if runtime.GOOS != "linux" {
+		t.Skip("needs 127.0.0.2, which only Linux routes by default")
+	}
+
+	ns1 := &fakeNameserver{txt: map[string][]string{}}
+	ns2 := &fakeNameserver{txt: map[string][]string{}}
+
+	var lc net.ListenConfig
+
+	pc1, err := lc.ListenPacket(t.Context(), "udp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	_, port, err := net.SplitHostPort(pc1.LocalAddr().String())
+	require.NoError(t, err)
+
+	pc2, err := lc.ListenPacket(t.Context(), "udp", net.JoinHostPort("127.0.0.2", port))
+	require.NoError(t, err)
+
+	for pc, h := range map[net.PacketConn]mdns.Handler{pc1: ns1, pc2: ns2} {
+		srv := &mdns.Server{PacketConn: pc, Handler: h}
+
+		go func() { _ = srv.ActivateAndServe() }()
+
+		t.Cleanup(func() { _ = srv.Shutdown() })
+	}
+
+	c := newTXTChecker([]string{net.JoinHostPort("127.0.0.1", port)}, 5*time.Second)
+	c.interval = 10 * time.Millisecond
+	c.port = port
+
+	return c, ns1, ns2
+}
+
+func TestTXTCheckerAllNameservers(t *testing.T) {
+	c, ns1, ns2 := startNameservers(t)
+	ctx := context.Background()
+
+	ns1.set("other", "value")
+	ns2.set("value")
+	require.NoError(t, c.wait(ctx, testName, "value"))
+
+	// ns2 never gets the value: waiting for ns1 alone would let the CA
+	// hit ns2 and fail the challenge.
+	ns1.set("value-2")
+
+	c.timeout = 200 * time.Millisecond
+
+	err := c.wait(ctx, testName, "value-2")
+	require.ErrorIs(t, err, ErrNotPropagated)
+	require.ErrorContains(t, err, "ns2."+testZone)
+	assert.NotContains(t, err.Error(), "ns1.")
+}
+
+func TestTXTCheckerWaitsForLaggingNameserver(t *testing.T) {
+	c, ns1, ns2 := startNameservers(t)
+
+	ns1.set("value")
+	ns2.set("value")
+	ns2.hideFor.Store(3)
+
+	require.NoError(t, c.wait(context.Background(), testName, "value"))
+	assert.Negative(t, ns2.hideFor.Load(), "ns2 was asked again after it missed")
+}
+
+func TestTXTCheckerErrorNamesResolvers(t *testing.T) {
+	c := newTXTChecker([]string{"1.1.1.1:53", "8.8.8.8:53"}, time.Second)
+	err := c.via(&net.DNSError{Err: "no such host", Name: "ts.example.com.", Server: "127.0.0.53:53"})
+
+	assert.EqualError(t, err, "lookup ts.example.com. on 1.1.1.1:53, 8.8.8.8:53: no such host")
+}