feat: Support for dns-account-01 Challenge by sheurich · Pull Request #8149 · letsencrypt/boulder

sheurich · 2025-04-28T21:41:44Z

This pull request introduces support for the dns-account-01 challenge type as specified in draft-ietf-acme-dns-account-label-01 (https://datatracker.ietf.org/doc/draft-ietf-acme-dns-account-label/01/), building upon PR #8140 which introduced the core type definitions.

Core Implementation:

The policy engine in policy/pa.go is updated to offer the dns-account-01 challenge for both standard and wildcard domains.
The main validation authority logic in va/va.go is updated to recognize dns-account-01 challenges and route them to the correct validation routine, passing the necessary account information.
The core validation logic for dns-account-01 is implemented in va/dns.go, which calculates the account-specific DNS label and verifies the corresponding TXT record.

Configuration:

The PAConfig is updated to recognize dns-account-01 as a valid challenge type which can be enabled in the PA config.
A new DNSAccount01Enabled feature flag is introduced in features/features.go. If this flag is not set, then the PA will not offer the new challenge type, and the VA will refuse to validate this challenge type.

Testing:

A new suite of integration tests (test/integration/dns_account_01_test.go) has been added to cover various scenarios, including successful validation, incorrect TXT records, and wildcard domains.
The PA unit tests have been expanded to cover cases where the dns-account-01 feature is both enabled and disabled.
The VA unit tests now include va/dns_account_test.go, specifically targeting the dns-account-01 validation logic.
The mock DNS client (bdns/mocks.go) has been updated to simulate various dns-account-01 responses.
The challenge test server client (test/chall-test-srv-client/client.go) now includes methods for adding and removing dns-account-01 challenge responses.

These changes provide a complete implementation of the dns-account-01 challenge, including the necessary logic, configuration, and comprehensive testing to ensure its correctness and reliability.

sheurich · 2025-05-01T17:21:42Z

va/dns_account_test.go

+
+	// Assert the specific error type
+	prob := detailedError(err)
+	test.AssertEquals(t, prob.Type, probs.ConnectionProblem)


When accountURI is missing in validateDNSAccount01, it triggers an InternalServerError. The test TestDNSAccount01ValidationEmptyAccountURI verifies that the resulting client-facing error is mapped to ConnectionProblem (via the WFE mapping). Is ConnectionProblem the most suitable error type for the client, or should we consider a more specific internal error, such as MissingParameter, which could map to a different client-facing error (e.g., Malformed) if needed in the future? However, since the accountURI originates from the JWS, this error scenario should theoretically never occur.

ConnectionProblem is what we use as a catch-all for all 500s during validation, so it's fine to test for that here. But I think it would be even better to drop the call to detailedError, and instead directly assert that the returned error is of type berrors.ServerInternal.

aarongable

Note to fellow reviewers: when reviewing, ignore the first two commits in this PR. They are copied from #8140.

Similar to that other PR, I mostly have small editorial suggestions, and one big overarching comment. This time I've left that comment on va.proto; take a look.

aarongable · 2025-05-01T23:53:22Z

va/dns.go

+	// Compute the digest of the key authorization file
+	h := sha256.New()
+	h.Write([]byte(keyAuthorization))
+	authorizedKeysDigest := base64.RawURLEncoding.EncodeToString(h.Sum(nil))


This code can also be shared inside the new validateDNS helper method.

va/proto/va.proto

va/dns.go

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

This change consolidates configuration by removing the separate dnsAccountChallengeURIPrefix field and using the first element of the existing AccountURIPrefixes slice for dns-account-01 challenge validation. Changes: - Removed dnsAccountChallengeURIPrefix from va/config/config.go - Updated va/va.go to use accountURIPrefixes[0] for dns-account-01 challenges - Removed the field from NewValidationAuthorityImpl constructors - Updated test configurations to align with simplified logic - Added RegID to initial IsAnyNilOrZero validation check for robustness - Added documentation clarifying AccountURIPrefixes dual usage This addresses startup-time validation concerns since AccountURIPrefixes is already required to have at least one element, eliminating the risk of runtime failures due to missing configuration.

test/config-next/remoteva-a.json

va/va.go

va/dns.go

va/dns_account_test.go

aarongable · 2025-07-31T21:08:45Z

va/dns_account_test.go

+
+	// Assert the specific error type
+	prob := detailedError(err)
+	test.AssertEquals(t, prob.Type, probs.ConnectionProblem)


ConnectionProblem is what we use as a catch-all for all 500s during validation, so it's fine to test for that here. But I think it would be even better to drop the call to detailedError, and instead directly assert that the returned error is of type berrors.ServerInternal.

test/integration/dns_account_01_test.go

… very end of each JSON configuration to match config struct ordering and maintain consistency across service configs

… code

Address pull request review comments: - Remove unnecessary info log in validateDNSAccount01 - Refactor validateDNS to take challengePrefix instead of full domain - Update spec reference to draft-ietf-acme-dns-account-label-01 - Remove unnecessary base32 padding configuration - Fix error message formatting to use %q instead of %s - Remove obsolete log line in validateDNS01 - Consolidate DNS validation logic for better maintainability

Convert individual test functions to table-driven tests and update error messages to follow Go style guide recommendations: - Consolidate separate test functions into single table-driven test - Update error messages to use Go Style Guide pattern - Use strings.Contains for robust error message checking - Fix error type assertion to use berrors.InternalServer directly - Remove unused constants and fix code formatting

Enhance integration tests based on pull request feedback: - Add environment variable checks for better test configuration reliability - Simplify test validation to focus on authorization status changes - Replace complex certificate generation flows with targeted validation - Remove unused crypto imports after test simplification - Change from t.Skip to t.Fatal for clearer error handling

The comment for `validateDNSAccount01` now references CAB/F Ballot SC-84, which allows the use of the `dns-account-01` challenge method. This provides better context for why this challenge type is implemented.

aarongable

Looking great! Just two comments from this last round.

P.S. I'm not sure why the "Check PR" actions are failing. I'll investigate that next week; I don't think it has anything to do with the contents of thie PR.

va/dns.go

va/dns_account_test.go

Was unidiomatic Co-authored-by: Aaron Gable <aaron@aarongable.com>

…r assertions

va/dns_account_test.go

Co-authored-by: Aaron Gable <aaron@aarongable.com>

jsha

Nice!

Integralist · 2025-09-15T15:55:40Z

Amazing to see this merged. Is there a timeline for when this might appear in https://github.com/letsencrypt/pebble ?

EDIT: My bad, looks like this has been in pebble for well over a year letsencrypt/pebble#435

miken32 · 2025-10-06T20:43:48Z

va/dns.go

+	// Calculate the DNS prefix label based on the account URI
+	sha256sum := sha256.Sum256([]byte(accountURI))
+	prefixBytes := sha256sum[0:10] // First 10 bytes
+	prefixLabel := strings.ToLower(base32.StdEncoding.EncodeToString(prefixBytes))


I'm looking into using this validation method, and possibly this is a dumb question (😰 go is not my native language), but should this be removing padding = at the end of the output? Something like:

base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(prefixBytes)

It's a reasonable question! But with 10 bytes of input, base32 will never need padding. This is because base32 uses one character to represent every 5 bits of input. Five bytes of input is 40 bits, which is evenly divisible by 5, so can be represented by eight base32 characters with no padding. Ten bytes of input is the same.

As documented in the Internet Draft, the 10-byte prefix length was chosen specifically because it has this property.

sheurich requested a review from a team as a code owner April 28, 2025 21:41

sheurich requested a review from beautifulentropy April 28, 2025 21:41

sheurich force-pushed the feat-dns-account-01-mod-val branch 2 times, most recently from 1b542cc to 749bbf9 Compare May 1, 2025 16:43

sheurich commented May 1, 2025

View reviewed changes

aarongable mentioned this pull request May 1, 2025

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

Merged

aarongable requested changes May 2, 2025

View reviewed changes

sheurich force-pushed the feat-dns-account-01-mod-val branch 13 times, most recently from 3b5bb07 to 2c19389 Compare May 8, 2025 21:05

sheurich force-pushed the feat-dns-account-01-mod-val branch 2 times, most recently from 3f7cef6 to 2e16e53 Compare May 12, 2025 16:20

sheurich requested a review from aarongable May 28, 2025 21:26

sheurich changed the title ~~feat(va, ra, sa): dns-account-01 validation logic and protocol updates~~ feat: Support for dns-account-01 Challenge Jun 20, 2025

sheurich force-pushed the feat-dns-account-01-mod-val branch 2 times, most recently from eaba1cd to c56430c Compare June 24, 2025 17:49

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 and others added 2 commits July 29, 2025 16:38

Merge branch 'main' into feat-dns-account-01-mod-val

bdb1922

aarongable reviewed Jul 31, 2025

View reviewed changes

sheurich added 6 commits August 1, 2025 18:06

Moved "features" stanza containing "DNSAccount01Enabled": true to the…

8237818

… very end of each JSON configuration to match config struct ordering and maintain consistency across service configs

Rejoined split comment that was accidentally broken by newly-inserted…

fde121f

… code

feat(va): Reference CAB/F ballot in dns-account-01 comment

9345254

The comment for `validateDNSAccount01` now references CAB/F Ballot SC-84, which allows the use of the `dns-account-01` challenge method. This provides better context for why this challenge type is implemented.

sheurich requested a review from aarongable August 1, 2025 22:44

aarongable reviewed Aug 1, 2025

View reviewed changes

va/dns.go Outdated Show resolved Hide resolved

va/dns_account_test.go Outdated Show resolved Hide resolved

sheurich and others added 2 commits August 4, 2025 14:28

Update va/dns.go

8cde75e

Was unidiomatic Co-authored-by: Aaron Gable <aaron@aarongable.com>

refactor(va): update TestDNSAccount01Validation to use error types fo…

8bb4e78

…r assertions

sheurich requested a review from aarongable August 4, 2025 19:05

aarongable previously approved these changes Aug 4, 2025

View reviewed changes

va/dns_account_test.go Outdated Show resolved Hide resolved

Update va/dns_account_test.go

a9fde95

Co-authored-by: Aaron Gable <aaron@aarongable.com>

sheurich dismissed aarongable’s stale review via a9fde95 August 5, 2025 01:42

jprenken approved these changes Aug 7, 2025

View reviewed changes

sheurich requested a review from aarongable August 8, 2025 12:31

Integralist mentioned this pull request Aug 20, 2025

feature request: support for dns-account-01 challenge mholt/acmez#42

Closed

Merge branch 'main' into feat-dns-account-01-mod-val

7bdf48d

aarongable approved these changes Sep 9, 2025

View reviewed changes

jsha approved these changes Sep 10, 2025

View reviewed changes

aarongable merged commit 46013ea into letsencrypt:main Sep 10, 2025
12 checks passed

Integralist mentioned this pull request Sep 15, 2025

DNS Account 01 Challenge Type Support letsencrypt/pebble#516

Closed

sheurich deleted the feat-dns-account-01-mod-val branch September 15, 2025 16:34

sheurich mentioned this pull request Sep 24, 2025

Add dns-account-01 challenge support unixcharles/acme-client#256

Merged

miken32 reviewed Oct 6, 2025

View reviewed changes

zhangyoufu mentioned this pull request Oct 21, 2025

strip _acme-challenge. prefix should be discouraged libdns/acmedns#5

Open

Uh oh!

Conversation

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

Uh oh!

Uh oh!

sheurich May 1, 2025

Choose a reason for hiding this comment

Uh oh!

aarongable Jul 31, 2025

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aarongable Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aarongable left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

jsha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Integralist commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miken32 Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

aarongable Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sheurich commented Apr 28, 2025 •

edited by aarongable

Loading

aarongable left a comment •

edited

Loading

Integralist commented Sep 15, 2025 •

edited

Loading