Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from

Conversation

sheurich
Copy link
Owner

@sheurich sheurich commented Apr 28, 2025

No description provided.

beautifulentropy and others added 7 commits April 28, 2025 11:14
Break validation of length and content of expected User-Agents out into
two assertion functions. Make it so that DOH and MPICFullResults can be
deprecated in either order.

Fixes letsencrypt#8145
- Add `ChallengeTypeDNSAccount01` constant, `IsValid` update, and `RecordsSane` logic in `core/objects.go`
- Add `DNSAccountChallenge01` function and handling in `core/challenges.go`
- Add `DNSAccount01Enabled` feature flag in `features/features.go`
- Add tests for the new challenge type in `core/core_test.go` and `core/objects_test.go`
- Update vendor dependency `github.com/eggsampler/acme/v3` to support `dns-account-01`

Implements core components for draft-ietf-acme-dns-account-label-00
- Modify PA to offer dns-account-01 when feature flag is enabled
- Add tests for dns-account-01 challenge type in PA
- Update challenge type config handling

Enables dns-account-01 to be offered as a valid challenge type
- Add accountURI field to AuthzMeta in VA protocol
- Update RA protocol to pass account URI
- Update SA model to store account URI for challenges

Required for passing account URI from WFE to VA for validation
Extracts the common DNS TXT record lookup, comparison, and
result/error handling logic from `validateDNS01` into a new
unexported helper function `validateDNS`.

This change aims to reduce code duplication in preparation for
simplifying the upcoming `validateDNSAccount01` function, which
shares most of this core validation flow.

`validateDNS01` is updated to calculate its specific inputs
(digest, query domain) and then call the new helper function.
There are no intended functional changes to `validateDNS01` itself.
Adds validation logic for the ACME `dns-account-01` challenge type.

Introduces the `validateDNSAccount01` function in `va/dns.go` which
implements the account-specific DNS label construction (based on the
Account URI) and validation flow, utilizing the shared `validateDNS`
function. Unauthorized errors returned during validation are
enriched with the Account URI for context.

Adds a new test file `va/dns_account_test.go` with unit tests for the
`validateDNSAccount01` function, mirroring existing dns-01 test
scenarios and checking specific dns-account-01 behavior.

Updates `bdns/mocks.go` with necessary entries to support the new
dns-account-01 test cases.

Ref: draft-ietf-acme-dns-account-label-00
Passes the account URI to the VA's validateChallenge function
and adds a case to route dns-account-01 challenges (when enabled)
to a new validateDNSAccount01 function.

Updates the caller in DoDCV and adjusts tests.
@sheurich sheurich requested a review from Copilot April 28, 2025 21:13
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the dns-account-01 challenge type by adding the necessary protocol, model, and validation logic to support account-specific DNS validations. Key changes include:

  • Propagating and validating an additional accountURI parameter in VA and RA components via updated gRPC protos and Go code.
  • Implementing and testing the validateDNSAccount01 logic in the Validation Authority, including constructing an account-specific DNS query domain.
  • Updating configuration, models, and DNS mocks to support the new challenge type.

Reviewed Changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
va/va.go Modified validateChallenge routing and added accountURI parameter for dns-account-01.
va/proto/va.proto & va.pb.go Updated proto definitions and generated files to include the accountURI field.
va/dns.go Added validateDNSAccount01 with account-specific DNS prefix calculation and error enrichment.
va/dns_test.go & dns_account_test.go Renamed and added tests covering various failure and success cases for dns-account-01.
policy/pa.go & pa_test.go Updated challenge offering to include dns-account-01 when the feature flag is enabled.
features/features.go Added DNSAccount01Enabled flag to control exposure of the new challenge type.
core/objects.go, challenges.go, core_test.go Extended challenge type definitions and test coverage for dns-account-01.
cmd/config.go Updated configuration validation to allow dns-account-01 in supported challenge types.
bdns/mocks.go Modified DNS TXT record mocks to handle account-specific query domains for dns-account-01.
Files not reviewed (1)
  • go.mod: Language not supported

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mrge found 4 issues across 22 files. View them in mrge.io

@sheurich sheurich force-pushed the feat-dns-account-01-mod-val branch from 2ea07e3 to bb574de Compare April 28, 2025 21:25
@sheurich sheurich closed this Apr 28, 2025
@sheurich sheurich deleted the feat-dns-account-01-mod-val branch April 28, 2025 21:28
@sheurich sheurich restored the feat-dns-account-01-mod-val branch April 28, 2025 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants