feat: Add core definitions for dns-account-01 #8140

sheurich · 2025-04-23T20:37:05Z

Summary

This PR introduces the foundational components required to support the dns-account-01 challenge type, as specified in draft-ietf-acme-dns-account-label-00.

Updated Scope (per review feedback): This PR now focuses only on core definitions and SA support. PA/VA/RA logic moved to PR #8149.

Changes

Core Definitions & Logic:

core/objects.go: Added ChallengeTypeDNSAccount01 constant and updated validation methods
core/challenges.go: Added DNSAccountChallenge01 constructor and factory support

Storage Authority (SA) Support:

sa/model.go: Added dns-account-01 to challenge type mappings

Testing:

core/*_test.go: Basic definition and validation tests
sa/sa_test.go: Database round-trip tests for dns-account-01 challenges

Dependencies:

Updated github.com/eggsampler/acme/v3 to release version v3.6.2

Next Steps: PR #8149 will add the PA/VA/RA validation logic and feature flags.

aarongable

Generally looks good. A few comments but nothing big or structural in the code itself.

That said, I do have one big/overarching comment:

I think it's an antipattern to introduce a feature flag but not immediately turn that feature flag on in the config-next integration tests configuration files. It means that we're not actually testing that flag in a meaningful way, and we could easily miss that step in a later PR (e.g. I don't see the flag being enabled in #8149 either).

But in this case, turning on the feature flag would immediately cause things to blow up, because the SA doesn't know how to convert a dns-account-01 challenge into a database entry.

This suggests to me that we're going about things in slightly the wrong order. In my opinion, the correct order to land these changes would be:

core definitions
SA/database support and WFE/rendering support, neither of which needs to be gated behind a flag
PA, VA, and RA support, plus the feature flag, and new integration tests

aarongable · 2025-05-01T23:16:36Z

core/objects.go

@@ -238,6 +239,16 @@ func (ch Challenge) RecordsSane() bool {
 			return false
 		}
 		return true
+	case ChallengeTypeDNSAccount01:


Rather than duplicating code in this block, I'd suggest simply changing line 232 to say

case ChallengeTypeDNS01, ChallengeTypeDNSAccount01:

aarongable · 2025-05-01T23:19:44Z

policy/pa.go

@@ -535,16 +536,28 @@ func (pa *AuthorityImpl) ChallengeTypesFor(ident identifier.ACMEIdentifier) ([]c
 	// stating that ACME HTTP-01 and TLS-ALPN-01 are not suitable for validating
 	// Wildcard Domains.
 	if ident.Type == identifier.TypeDNS && strings.HasPrefix(ident.Value, "*.") {
-		return []core.AcmeChallenge{core.ChallengeTypeDNS01}, nil
+		challenges := []core.AcmeChallenge{core.ChallengeTypeDNS01}


The comment above this block also needs to be updated to mention dns-account-01.

aarongable · 2025-05-01T23:21:56Z

features/features.go

@@ -89,6 +89,11 @@ type Config struct {
 	// StoreARIReplacesInOrders causes the SA to store and retrieve the optional
 	// ARI replaces field in the orders table.
 	StoreARIReplacesInOrders bool
+
+	// DNSAccount01Enabled enables or disables support for the dns-account-01


This comment should indicate which services respect this flag (i.e. which services it needs to be set in to be fully successful -- don't want to enable it on the RA but not the VA and end up in a broken state).

aarongable · 2025-05-01T23:24:04Z

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/config v1.27.43
 	github.com/aws/aws-sdk-go-v2/service/s3 v1.65.3
 	github.com/aws/smithy-go v1.22.0
-	github.com/eggsampler/acme/v3 v3.6.2-0.20250208073118-0466a0230941
+	github.com/eggsampler/acme/v3 v3.6.2


While I appreciate this bump to an actually-released version, it doesn't seem to actually affect any of our vendored code. That suggests to me that it isn't actually necessary for the rest of this PR to work, and I think it should be separated out.

If you update this PR to be only the core definitions (i.e. separate the PA + feature code into a later PR, as I suggest in the top-level comment) then I think including this here will be fine.

aarongable · 2025-05-01T23:25:26Z

policy/pa_test.go

@@ -401,6 +402,9 @@ func TestChallengeTypesFor(t *testing.T) {
 	t.Parallel()
 	pa := paImpl(t)

+	features.Set(features.Config{DNSAccount01Enabled: true})


Ideally this test would run with this flag both enabled and disabled, to prove that the flag works correctly.

sheurich · 2025-05-02T19:06:12Z

Thanks @aarongable. I made the suggested changes and split the feature-flag + PA changes to a future PR.

sheurich · 2025-05-05T19:28:34Z

core definitions

SA/database support and WFE/rendering support, neither of which needs to be gated behind a flag

PA, VA, and RA support, plus the feature flag, and new integration tests

It seems that no changes are required for WFE rendering.

Given that the SA changes are minor (limited to updating the challenge type map), it makes sense to bundle the first two items into this PR. By including the necessary SA changes here, the remaining items—PA, VA, RA support, feature flag implementation, and integration tests—can be moved to #8149.

core/objects.go

aarongable · 2025-05-09T18:50:31Z

sa/model.go

The changes to this file should be accompanied by changes to either model_test.go or sa_test.go showing that we can successfully round-trip a dns-account-01 challenge into and back out of the database. This should probably be in the form of something similar to the existing TestGetAuthorization2 test, but with the test setup creating an authz with the new dns-account-01 challenge set.

sheurich · 2025-05-28T18:29:18Z

@aarongable everything is up-to-date and ready for a final review.

sheurich · 2025-06-19T17:18:26Z

@aarongable Thanks for the detailed review! I've addressed all the feedback from your reviews:

✅ All Review Comments Resolved:

Code duplication in core/objects.go - Fixed with single case statement
SA database testing - Added comprehensive round-trip tests in sa/sa_test.go
Code formatting - Switch cases now on single lines as suggested
PA/feature flag separation - Moved to PR feat: Support for dns-account-01 Challenge #8149 as recommended

✅ Architectural Changes Implemented:
Following your guidance, this PR now contains only core definitions + SA support:

Core type definitions (core/objects.go, core/challenges.go)
SA model mappings (sa/model.go)
Associated tests with full coverage

The PA, VA, RA support + feature flags are in the follow-up PR #8149.

✅ Dependency Update:
Per your comment: "If you update this PR to be only the core definitions... then I think including this here will be fine" - the acme/v3 bump is included as conditionally approved.

This should be ready for final approval! 🚀

- Add `ChallengeTypeDNSAccount01` constant, `IsValid` update, and `RecordsSane` logic in `core/objects.go` - Add `DNSAccountChallenge01` function and handling in `core/challenges.go` - Add tests for the new challenge type in `core/core_test.go` and `core/objects_test.go` Implements core components for draft-ietf-acme-dns-account-label-00

- Add dns-account-01 challenge type to challTypeToUint map - Add dns-account-01 challenge type to uintToChallType map

…tions2 Extends the existing TestGetValidAuthorizations2 function to verify that authorizations with dns-account-01 challenges can be properly stored in and retrieved from the database.

sheurich requested a review from a team as a code owner April 23, 2025 20:37

sheurich requested a review from aarongable April 23, 2025 20:37

sheurich force-pushed the feat-dns-account-01-core branch 2 times, most recently from bd2501a to 217b809 Compare April 25, 2025 23:04

sheurich changed the title ~~feat: Add core definitions for dns-account-01 challenge type~~ feat: Add core definitions and PA offering for dns-account-01 Apr 25, 2025

sheurich force-pushed the feat-dns-account-01-core branch from 217b809 to e8b0275 Compare April 28, 2025 20:58

This was referenced Apr 28, 2025

feat(va, ra, sa): Implement dns-account-01 validation logic and protocol updates sheurich/boulder#4

Closed

feat: Support for dns-account-01 Challenge #8149

Open

sheurich force-pushed the feat-dns-account-01-core branch from e8b0275 to 5b582d2 Compare May 1, 2025 16:43

aarongable requested changes May 1, 2025

View reviewed changes

sheurich force-pushed the feat-dns-account-01-core branch from 5b582d2 to a813c3a Compare May 2, 2025 18:50

sheurich changed the title ~~feat: Add core definitions and PA offering for dns-account-01~~ feat: Add core definitions for dns-account-01 May 2, 2025

sheurich requested a review from aarongable May 2, 2025 19:52

sheurich force-pushed the feat-dns-account-01-core branch 4 times, most recently from 7e79102 to 70d50ce Compare May 8, 2025 19:35

aarongable reviewed May 9, 2025

View reviewed changes

aarongable requested review from a team and beautifulentropy and removed request for a team May 9, 2025 18:50

sheurich force-pushed the feat-dns-account-01-core branch 2 times, most recently from c00aeee to afc4313 Compare May 12, 2025 16:19

sheurich force-pushed the feat-dns-account-01-core branch 2 times, most recently from 4aafca3 to afd88db Compare May 28, 2025 18:11

sheurich requested a review from aarongable May 28, 2025 18:25

sheurich force-pushed the feat-dns-account-01-core branch from afd88db to 69ae9c0 Compare June 3, 2025 16:58

sheurich force-pushed the feat-dns-account-01-core branch from 69ae9c0 to e8ee04a Compare June 6, 2025 18:36

sheurich force-pushed the feat-dns-account-01-core branch from e8ee04a to 1c345a9 Compare June 17, 2025 18:38

sheurich force-pushed the feat-dns-account-01-core branch from d65b13d to 135b84a Compare June 19, 2025 19:16

sheurich mentioned this pull request Jun 20, 2025

feat: Support for dns-account-01 Challenge Type sheurich/boulder#8

Closed

sheurich force-pushed the feat-dns-account-01-core branch from 135b84a to 32de32b Compare June 24, 2025 17:48

sheurich added 4 commits June 24, 2025 14:17

chore: Update vendor dependency github.com/eggsampler/acme/v3 v3.6.2

3f0ddb9

feat: Add dns-account-01 challenge type to SA model

60e6787

- Add dns-account-01 challenge type to challTypeToUint map - Add dns-account-01 challenge type to uintToChallType map

test: Add dns-account-01 challenge test case to TestGetValidAuthoriza…

6726be6

…tions2 Extends the existing TestGetValidAuthorizations2 function to verify that authorizations with dns-account-01 challenges can be properly stored in and retrieved from the database.

sheurich force-pushed the feat-dns-account-01-core branch from 32de32b to 6726be6 Compare June 24, 2025 18:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: Add core definitions for dns-account-01 #8140

feat: Add core definitions for dns-account-01 #8140

Uh oh!

sheurich commented Apr 23, 2025 •

edited

Loading

Uh oh!

aarongable left a comment

Uh oh!

aarongable May 1, 2025

Uh oh!

aarongable May 1, 2025

Uh oh!

aarongable May 1, 2025

Uh oh!

aarongable May 1, 2025 •

edited

Loading

Uh oh!

aarongable May 1, 2025

Uh oh!

sheurich commented May 2, 2025 •

edited

Loading

Uh oh!

sheurich commented May 5, 2025

Uh oh!

Uh oh!

aarongable May 9, 2025

Uh oh!

sheurich commented May 28, 2025

Uh oh!

sheurich commented Jun 19, 2025

Uh oh!

Uh oh!

Uh oh!

feat: Add core definitions for dns-account-01 #8140

Are you sure you want to change the base?

feat: Add core definitions for dns-account-01 #8140

Uh oh!

Conversation

sheurich commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Core Definitions & Logic:

Storage Authority (SA) Support:

Testing:

Dependencies:

Uh oh!

aarongable left a comment

Choose a reason for hiding this comment

Uh oh!

aarongable May 1, 2025

Choose a reason for hiding this comment

Uh oh!

aarongable May 1, 2025

Choose a reason for hiding this comment

Uh oh!

aarongable May 1, 2025

Choose a reason for hiding this comment

Uh oh!

aarongable May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aarongable May 1, 2025

Choose a reason for hiding this comment

Uh oh!

sheurich commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sheurich commented May 5, 2025

Uh oh!

Uh oh!

aarongable May 9, 2025

Choose a reason for hiding this comment

Uh oh!

sheurich commented May 28, 2025

Uh oh!

sheurich commented Jun 19, 2025

Uh oh!

Uh oh!

sheurich commented Apr 23, 2025 •

edited

Loading

aarongable May 1, 2025 •

edited

Loading

sheurich commented May 2, 2025 •

edited

Loading