Skip to content

Commit 3922886

Browse files
authored
Fix wildcard authorization reuse with DNS-Account-01 (#8504)
Update ra.NewOrder to allow reuse of authorizations with a dns-account-01 challenge for wildcard identifiers. Add two new tests showing that the RA accepts or rejects reuse of such an authorization, depending on the state of the DNSAccount01Enabled feature flag. Fixes #8505
1 parent 0303e88 commit 3922886

File tree

3 files changed

+225
-14
lines changed

3 files changed

+225
-14
lines changed

ra/ra.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2243,14 +2243,18 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
22432243
authzAge = (profile.pendingAuthzLifetime - authz.Expires.Sub(ra.clk.Now())).Seconds()
22442244
}
22452245

2246-
// If the identifier is a wildcard DNS name, it must have exactly one
2247-
// DNS-01 type challenge. The PA guarantees this at order creation time,
2248-
// but we verify again to be safe.
2249-
if ident.Type == identifier.TypeDNS && strings.HasPrefix(ident.Value, "*.") &&
2250-
(len(authz.Challenges) != 1 || authz.Challenges[0].Type != core.ChallengeTypeDNS01) {
2251-
return nil, berrors.InternalServerError(
2252-
"SA.GetAuthorizations returned a DNS wildcard authz (%s) with invalid challenge(s)",
2253-
authz.ID)
2246+
// If the identifier is a wildcard DNS name, all challenges must be
2247+
// DNS-01 or DNS-Account-01. The PA guarantees this at order creation
2248+
// time, but we verify again to be safe.
2249+
if ident.Type == identifier.TypeDNS && strings.HasPrefix(ident.Value, "*.") {
2250+
for _, chall := range authz.Challenges {
2251+
if chall.Type != core.ChallengeTypeDNS01 && !(features.Get().DNSAccount01Enabled && chall.Type == core.ChallengeTypeDNSAccount01) {
2252+
return nil, berrors.InternalServerError(
2253+
"SA.GetAuthorizations returned a DNS wildcard authz (%s) with invalid challenge(s)",
2254+
authz.ID,
2255+
)
2256+
}
2257+
}
22542258
}
22552259

22562260
// If we reached this point then the existing authz was acceptable for

ra/ra_test.go

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2119,11 +2119,110 @@ func TestNewOrderAuthzReuseSafety(t *testing.T) {
21192119

21202120
// Create an order for that request
21212121
_, err := ra.NewOrder(ctx, orderReq)
2122-
// It should fail
2122+
// It should fail because HTTP-01 is not a valid challenge type for wildcards
21232123
test.AssertError(t, err, "Added an initial order for regA with invalid challenge(s)")
21242124
test.AssertContains(t, err.Error(), "SA.GetAuthorizations returned a DNS wildcard authz (1) with invalid challenge(s)")
21252125
}
21262126

2127+
// TestNewOrderAuthzReuseDNSAccount01 checks that the RA correctly allows reuse
2128+
// of a wildcard authorization with a DNS-Account-01 challenge.
2129+
func TestNewOrderAuthzReuseDNSAccount01(t *testing.T) {
2130+
_, _, ra, _, _, registration, cleanUp := initAuthorities(t)
2131+
defer cleanUp()
2132+
2133+
features.Set(features.Config{DNSAccount01Enabled: true})
2134+
defer features.Reset()
2135+
2136+
ctx := context.Background()
2137+
idents := identifier.ACMEIdentifiers{identifier.NewDNS("*.zombo.com")}
2138+
2139+
// Use a mock SA that returns a pending authz with both DNS-01 and
2140+
// DNS-Account-01 challenges for wildcard. This tests that pending wildcard
2141+
// authorizations with 2 challenges can be reused.
2142+
expires := time.Now().Add(24 * time.Hour)
2143+
ra.SA = &mockSAWithAuthzs{
2144+
authzs: []*core.Authorization{
2145+
{
2146+
ID: "1",
2147+
Identifier: identifier.NewDNS("*.zombo.com"),
2148+
RegistrationID: registration.Id,
2149+
Status: "pending",
2150+
Expires: &expires,
2151+
Challenges: []core.Challenge{
2152+
{
2153+
Type: core.ChallengeTypeDNS01,
2154+
Status: core.StatusPending,
2155+
Token: core.NewToken(),
2156+
},
2157+
{
2158+
Type: core.ChallengeTypeDNSAccount01,
2159+
Status: core.StatusPending,
2160+
Token: core.NewToken(),
2161+
},
2162+
},
2163+
},
2164+
},
2165+
}
2166+
2167+
orderReq := &rapb.NewOrderRequest{
2168+
RegistrationID: registration.Id,
2169+
Identifiers: idents.ToProtoSlice(),
2170+
}
2171+
2172+
// Create an order for the wildcard domain. NewOrder should recognize that
2173+
// the existing valid authorization with DNS-Account-01 challenge can be
2174+
// reused for this wildcard domain request.
2175+
order, err := ra.NewOrder(ctx, orderReq)
2176+
test.AssertNotError(t, err, "NewOrder failed to reuse wildcard authz with DNS-Account-01")
2177+
// The order should contain exactly one authorization (the reused one)
2178+
test.AssertEquals(t, len(order.V2Authorizations), 1)
2179+
// The authorization ID should match the mock authz we provided (ID "1")
2180+
test.AssertEquals(t, order.V2Authorizations[0], int64(1))
2181+
}
2182+
2183+
// TestNewOrderAuthzReuseDNSAccount01Disabled checks that the RA rejects
2184+
// wildcard authorization reuse with DNS-Account-01 when the feature is disabled.
2185+
func TestNewOrderAuthzReuseDNSAccount01Disabled(t *testing.T) {
2186+
_, _, ra, _, _, registration, cleanUp := initAuthorities(t)
2187+
defer cleanUp()
2188+
2189+
// Feature flag is NOT set - DNSAccount01Enabled defaults to false
2190+
2191+
ctx := context.Background()
2192+
idents := identifier.ACMEIdentifiers{identifier.NewDNS("*.zombo.com")}
2193+
2194+
// Use a mock SA that returns a DNS-Account-01 authz for wildcard
2195+
expires := time.Now().Add(24 * time.Hour)
2196+
ra.SA = &mockSAWithAuthzs{
2197+
authzs: []*core.Authorization{
2198+
{
2199+
ID: "1",
2200+
Identifier: identifier.NewDNS("*.zombo.com"),
2201+
RegistrationID: registration.Id,
2202+
Status: "valid",
2203+
Expires: &expires,
2204+
Challenges: []core.Challenge{
2205+
{
2206+
Type: core.ChallengeTypeDNSAccount01,
2207+
Status: core.StatusValid,
2208+
Token: core.NewToken(),
2209+
},
2210+
},
2211+
},
2212+
},
2213+
}
2214+
2215+
orderReq := &rapb.NewOrderRequest{
2216+
RegistrationID: registration.Id,
2217+
Identifiers: idents.ToProtoSlice(),
2218+
}
2219+
2220+
// NewOrder should reject the DNS-Account-01 authz when feature is disabled
2221+
_, err := ra.NewOrder(ctx, orderReq)
2222+
test.AssertError(t, err, "NewOrder should reject DNS-Account-01 when feature disabled")
2223+
test.AssertContains(t, err.Error(), "SA.GetAuthorizations returned a DNS wildcard authz (1) with invalid challenge(s)")
2224+
}
2225+
21272226
func TestNewOrderWildcard(t *testing.T) {
21282227
_, _, ra, _, _, registration, cleanUp := initAuthorities(t)
21292228
defer cleanUp()

test/integration/dns_account_01_test.go

Lines changed: 113 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ func TestDNSAccount01HappyPath(t *testing.T) {
4747
t.Fatalf("adding DNS response: %s", err)
4848
}
4949
t.Cleanup(func() {
50-
_, _ = testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
50+
_, err := testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
51+
if err != nil {
52+
t.Fatal(err)
53+
}
5154
})
5255

5356
chal, err = c.Client.UpdateChallenge(c.Account, chal)
@@ -103,7 +106,10 @@ func TestDNSAccount01WrongTXTRecord(t *testing.T) {
103106
t.Fatalf("adding DNS response: %s", err)
104107
}
105108
t.Cleanup(func() {
106-
_, _ = testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
109+
_, err := testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
110+
if err != nil {
111+
t.Fatal(err)
112+
}
107113
})
108114

109115
_, err = c.Client.UpdateChallenge(c.Account, chal)
@@ -212,7 +218,10 @@ func TestDNSAccount01MultipleTXTRecordsNoneMatch(t *testing.T) {
212218
t.Fatalf("adding DNS response: %s", err)
213219
}
214220
t.Cleanup(func() {
215-
_, _ = testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
221+
_, err := testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
222+
if err != nil {
223+
t.Fatal(err)
224+
}
216225
})
217226

218227
_, err = c.Client.UpdateChallenge(c.Account, chal)
@@ -276,7 +285,10 @@ func TestDNSAccount01MultipleTXTRecordsOneMatches(t *testing.T) {
276285
t.Fatalf("adding DNS response: %s", err)
277286
}
278287
t.Cleanup(func() {
279-
_, _ = testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
288+
_, err := testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
289+
if err != nil {
290+
t.Fatal(err)
291+
}
280292
})
281293

282294
chal, err = c.Client.UpdateChallenge(c.Account, chal)
@@ -342,7 +354,10 @@ func TestDNSAccount01WildcardDomain(t *testing.T) {
342354
t.Fatalf("adding DNS response: %s", err)
343355
}
344356
t.Cleanup(func() {
345-
_, _ = testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
357+
_, err := testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
358+
if err != nil {
359+
t.Fatal(err)
360+
}
346361
})
347362

348363
chal, err = c.Client.UpdateChallenge(c.Account, chal)
@@ -361,3 +376,96 @@ func TestDNSAccount01WildcardDomain(t *testing.T) {
361376
}
362377
}
363378
}
379+
380+
func TestDNSAccount01WildcardAuthorizationReuse(t *testing.T) {
381+
t.Parallel()
382+
383+
if os.Getenv("BOULDER_CONFIG_DIR") == "test/config" {
384+
t.Skip("Test requires dns-account-01 to be enabled")
385+
}
386+
387+
// Use same domain for both orders to trigger authorization reuse
388+
domain := random_domain()
389+
wildcardDomain := fmt.Sprintf("*.%s", domain)
390+
391+
c, err := makeClient()
392+
if err != nil {
393+
t.Fatalf("creating client: %s", err)
394+
}
395+
396+
idents := []acme.Identifier{{Type: "dns", Value: wildcardDomain}}
397+
398+
// First order: Create and complete DNS-Account-01 challenge
399+
order1, err := c.Client.NewOrder(c.Account, idents)
400+
if err != nil {
401+
t.Fatalf("creating first order: %s", err)
402+
}
403+
404+
authzURL := order1.Authorizations[0]
405+
auth1, err := c.Client.FetchAuthorization(c.Account, authzURL)
406+
if err != nil {
407+
t.Fatalf("fetching first authorization: %s", err)
408+
}
409+
410+
chal, ok := auth1.ChallengeMap[acme.ChallengeTypeDNSAccount01]
411+
if !ok {
412+
t.Fatal("dns-account-01 challenge not offered by server")
413+
}
414+
415+
_, err = testSrvClient.AddDNSAccount01Response(c.Account.URL, domain, chal.KeyAuthorization)
416+
if err != nil {
417+
t.Fatalf("adding DNS response: %s", err)
418+
}
419+
t.Cleanup(func() {
420+
_, err := testSrvClient.RemoveDNSAccount01Response(c.Account.URL, domain)
421+
if err != nil {
422+
t.Fatal(err)
423+
}
424+
})
425+
426+
chal, err = c.Client.UpdateChallenge(c.Account, chal)
427+
if err != nil {
428+
t.Fatalf("updating challenge: %s", err)
429+
}
430+
431+
// Wait for authorization to become valid
432+
auth1, err = c.Client.FetchAuthorization(c.Account, authzURL)
433+
if err != nil {
434+
t.Fatalf("fetching first authorization after challenge update: %s", err)
435+
}
436+
437+
if auth1.Status != "valid" {
438+
t.Fatalf("expected first authorization status to be 'valid', got '%s'", auth1.Status)
439+
}
440+
441+
// Second order: Should reuse the existing authorization
442+
order2, err := c.Client.NewOrder(c.Account, idents)
443+
if err != nil {
444+
t.Fatalf("creating second order: %s", err)
445+
}
446+
447+
if len(order2.Authorizations) != 1 {
448+
t.Fatalf("expected 1 authorization in second order, got %d", len(order2.Authorizations))
449+
}
450+
451+
authzURL2 := order2.Authorizations[0]
452+
auth2, err := c.Client.FetchAuthorization(c.Account, authzURL2)
453+
if err != nil {
454+
t.Fatalf("fetching second authorization: %s", err)
455+
}
456+
457+
// Verify reuse occurred: same authorization URL
458+
if authzURL != authzURL2 {
459+
t.Errorf("expected same authorization URL, got different: %s != %s", authzURL, authzURL2)
460+
}
461+
462+
// Verify authorization is already valid (no re-validation needed)
463+
if auth2.Status != "valid" {
464+
t.Errorf("expected reused authorization status to be 'valid', got '%s'", auth2.Status)
465+
}
466+
467+
// Verify authorization still has DNS-Account-01 challenge
468+
if _, ok := auth2.ChallengeMap[acme.ChallengeTypeDNSAccount01]; !ok {
469+
t.Error("expected reused authorization to have dns-account-01 challenge")
470+
}
471+
}

0 commit comments

Comments
 (0)