From 4a61728ff0db8179e060d185b73a8c0d539d4c91 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Thu, 12 Feb 2026 19:13:49 +0100 Subject: [PATCH] fix: deduplicate authz for DNS01 challenge (#2828) --- challenge/resolver/prober.go | 60 +++++++++++++++++++++++--- challenge/resolver/prober_mock_test.go | 55 ++++++++++++++++++----- challenge/resolver/prober_test.go | 48 +++++++++++++++++++-- 3 files changed, 142 insertions(+), 21 deletions(-) diff --git a/challenge/resolver/prober.go b/challenge/resolver/prober.go index aac1016d8..66b12c7a7 100644 --- a/challenge/resolver/prober.go +++ b/challenge/resolver/prober.go @@ -98,11 +98,24 @@ func (p *Prober) Solve(authorizations []acme.Authorization) error { } func sequentialSolve(authSolvers []*selectedAuthSolver, failures obtainError) { + // Some CA are using the same token, + // this can be a problem with the DNS01 challenge when the DNS provider doesn't support duplicate TXT records. + // In the sequential mode, this is not a problem because we can solve the challenges in order. + // But it can reduce the number of call the DNS provider APIs. + uniq := make(map[string]struct{}) + for i, authSolver := range authSolvers { // Submit the challenge domain := challenge.GetTargetedDomain(authSolver.authz) + chlg, _ := challenge.FindChallenge(challenge.DNS01, authSolver.authz) + if solvr, ok := authSolver.solver.(preSolver); ok { + if _, ok := uniq[authSolver.authz.Identifier.Value+chlg.Token]; ok && chlg.Token != "" { + log.Infof("acme: duplicate token for %q (DNS-01); skipping pre-solve.", authSolver.authz.Identifier.Value) + continue + } + err := solvr.PreSolve(authSolver.authz) if err != nil { failures[domain] = err @@ -111,6 +124,8 @@ func sequentialSolve(authSolvers []*selectedAuthSolver, failures obtainError) { continue } + + uniq[authSolver.authz.Identifier.Value+chlg.Token] = struct{}{} } // Solve challenge @@ -123,22 +138,43 @@ func sequentialSolve(authSolvers []*selectedAuthSolver, failures obtainError) { continue } - // Clean challenge - cleanUp(authSolver.solver, authSolver.authz) + if _, ok := uniq[authSolver.authz.Identifier.Value+chlg.Token]; ok || chlg.Token == "" { + // Clean challenge + cleanUp(authSolver.solver, authSolver.authz) - if len(authSolvers)-1 > i { - solvr := authSolver.solver.(sequential) - _, interval := solvr.Sequential() - log.Infof("sequence: wait for %s", interval) - time.Sleep(interval) + if len(authSolvers)-1 > i { + solvr := authSolver.solver.(sequential) + _, interval := solvr.Sequential() + log.Infof("sequence: wait for %s", interval) + time.Sleep(interval) + } + + delete(uniq, authSolver.authz.Identifier.Value+chlg.Token) + } else { + log.Infof("acme: duplicate token for %q (DNS-01); skipping cleanup.", authSolver.authz.Identifier.Value) } } } func parallelSolve(authSolvers []*selectedAuthSolver, failures obtainError) { + // Some CA are using the same token, + // this can be a problem with the DNS01 challenge when the DNS provider doesn't support duplicate TXT records. + uniq := make(map[string]struct{}) + // For all valid preSolvers, first submit the challenges, so they have max time to propagate for _, authSolver := range authSolvers { authz := authSolver.authz + + chlg, err := challenge.FindChallenge(challenge.DNS01, authz) + if err == nil { + if _, ok := uniq[authz.Identifier.Value+chlg.Token]; ok { + log.Infof("acme: duplicate token for %q (DNS-01); skipping pre-solve.", authSolver.authz.Identifier.Value) + continue + } + + uniq[authz.Identifier.Value+chlg.Token] = struct{}{} + } + if solvr, ok := authSolver.solver.(preSolver); ok { err := solvr.PreSolve(authz) if err != nil { @@ -150,6 +186,16 @@ func parallelSolve(authSolvers []*selectedAuthSolver, failures obtainError) { defer func() { // Clean all created TXT records for _, authSolver := range authSolvers { + chlg, err := challenge.FindChallenge(challenge.DNS01, authSolver.authz) + if err == nil { + if _, ok := uniq[authSolver.authz.Identifier.Value+chlg.Token]; ok { + delete(uniq, authSolver.authz.Identifier.Value+chlg.Token) + } else { + log.Infof("acme: duplicate token for %q (DNS-01); skipping cleanup.", authSolver.authz.Identifier.Value) + continue + } + } + cleanUp(authSolver.solver, authSolver.authz) } }() diff --git a/challenge/resolver/prober_mock_test.go b/challenge/resolver/prober_mock_test.go index 5a91fe075..dc7ad8dec 100644 --- a/challenge/resolver/prober_mock_test.go +++ b/challenge/resolver/prober_mock_test.go @@ -1,6 +1,7 @@ package resolver import ( + "fmt" "time" "github.com/go-acme/lego/v4/acme" @@ -11,34 +12,68 @@ type preSolverMock struct { preSolve map[string]error solve map[string]error cleanUp map[string]error + + preSolveCounter int + solveCounter int + cleanUpCounter int } func (s *preSolverMock) PreSolve(authorization acme.Authorization) error { + s.preSolveCounter++ + return s.preSolve[authorization.Identifier.Value] } func (s *preSolverMock) Solve(authorization acme.Authorization) error { + s.solveCounter++ + return s.solve[authorization.Identifier.Value] } func (s *preSolverMock) CleanUp(authorization acme.Authorization) error { + s.cleanUpCounter++ + return s.cleanUp[authorization.Identifier.Value] } +func (s *preSolverMock) String() string { + return fmt.Sprintf("PreSolve: %d, Solve: %d, CleanUp: %d", s.preSolveCounter, s.solveCounter, s.cleanUpCounter) +} + func createStubAuthorizationHTTP01(domain, status string) acme.Authorization { + return createStubAuthorization(domain, status, false, acme.Challenge{ + Type: challenge.HTTP01.String(), + Validated: time.Now(), + }) +} + +func createStubAuthorizationDNS01(domain string, wildcard bool) acme.Authorization { + var chlgs []acme.Challenge + + if wildcard { + chlgs = append(chlgs, acme.Challenge{ + Type: challenge.HTTP01.String(), + Validated: time.Now(), + }) + } + + chlgs = append(chlgs, acme.Challenge{ + Type: challenge.DNS01.String(), + Validated: time.Now(), + }) + + return createStubAuthorization(domain, acme.StatusProcessing, wildcard, chlgs...) +} + +func createStubAuthorization(domain, status string, wildcard bool, chlgs ...acme.Challenge) acme.Authorization { return acme.Authorization{ - Status: status, - Expires: time.Now(), + Wildcard: wildcard, + Status: status, + Expires: time.Now(), Identifier: acme.Identifier{ - Type: challenge.HTTP01.String(), + Type: "dns", Value: domain, }, - Challenges: []acme.Challenge{ - { - Type: challenge.HTTP01.String(), - Validated: time.Now(), - Error: nil, - }, - }, + Challenges: chlgs, } } diff --git a/challenge/resolver/prober_test.go b/challenge/resolver/prober_test.go index 06ff07d2c..829b16883 100644 --- a/challenge/resolver/prober_test.go +++ b/challenge/resolver/prober_test.go @@ -2,19 +2,22 @@ package resolver import ( "errors" + "fmt" "testing" "github.com/go-acme/lego/v4/acme" "github.com/go-acme/lego/v4/challenge" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestProber_Solve(t *testing.T) { testCases := []struct { - desc string - solvers map[challenge.Type]solver - authz []acme.Authorization - expectedError string + desc string + solvers map[challenge.Type]solver + authz []acme.Authorization + expectedError string + expectedCounters map[challenge.Type]string }{ { desc: "success", @@ -30,6 +33,30 @@ func TestProber_Solve(t *testing.T) { createStubAuthorizationHTTP01("example.org", acme.StatusProcessing), createStubAuthorizationHTTP01("example.net", acme.StatusProcessing), }, + expectedCounters: map[challenge.Type]string{ + challenge.HTTP01: "PreSolve: 3, Solve: 3, CleanUp: 3", + }, + }, + { + desc: "DNS-01 deduplicate", + solvers: map[challenge.Type]solver{ + challenge.DNS01: &preSolverMock{ + preSolve: map[string]error{}, + solve: map[string]error{}, + cleanUp: map[string]error{}, + }, + }, + authz: []acme.Authorization{ + createStubAuthorizationDNS01("a.example", false), + createStubAuthorizationDNS01("a.example", true), + createStubAuthorizationDNS01("b.example", false), + createStubAuthorizationDNS01("b.example", true), + createStubAuthorizationDNS01("c.example", true), + createStubAuthorizationDNS01("d.example", false), + }, + expectedCounters: map[challenge.Type]string{ + challenge.DNS01: "PreSolve: 4, Solve: 6, CleanUp: 4", + }, }, { desc: "already valid", @@ -45,6 +72,9 @@ func TestProber_Solve(t *testing.T) { createStubAuthorizationHTTP01("example.org", acme.StatusValid), createStubAuthorizationHTTP01("example.net", acme.StatusValid), }, + expectedCounters: map[challenge.Type]string{ + challenge.HTTP01: "PreSolve: 0, Solve: 0, CleanUp: 0", + }, }, { desc: "when preSolve fail, auth is flagged as error and skipped", @@ -69,6 +99,9 @@ func TestProber_Solve(t *testing.T) { expectedError: `error: one or more domains had a problem: [example.com] preSolve error example.com `, + expectedCounters: map[challenge.Type]string{ + challenge.HTTP01: "PreSolve: 3, Solve: 2, CleanUp: 3", + }, }, { desc: "errors at different stages", @@ -95,6 +128,9 @@ func TestProber_Solve(t *testing.T) { [example.com] preSolve error example.com [example.org] solve error example.org `, + expectedCounters: map[challenge.Type]string{ + challenge.HTTP01: "PreSolve: 3, Solve: 2, CleanUp: 3", + }, }, } @@ -112,6 +148,10 @@ func TestProber_Solve(t *testing.T) { } else { require.NoError(t, err) } + + for n, s := range test.solvers { + assert.Equal(t, test.expectedCounters[n], fmt.Sprintf("%s", s)) + } }) } }