Skip to content
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

support for dns-account-01 #7303

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

orangepizza
Copy link
Contributor

integration not yet written : as it adds change to PA config to add a challenge type: and need certbot side change for write integration test: but unittest wise it mostly works:

currently WFE doesn't send full accountURL into VA, so it tries all accountURL prefix to generate possible challenge subdomain
from looking at test config files second listing is for v2 address so I used it to returned if both subdomain is empty: but it's just guesswork, but there could be better heuristic or edit RFC unto VA to send full URL, but it looks too much passing there for tiny amount of v1 account out there

core/objects.go Outdated Show resolved Hide resolved
policy/pa_test.go Show resolved Hide resolved
va/dns.go Outdated Show resolved Hide resolved
va/dns.go Outdated Show resolved Hide resolved
va/dns.go Show resolved Hide resolved
va/va.go Outdated Show resolved Hide resolved
@orangepizza
Copy link
Contributor Author

now it edits config-next to add a new challenge type github bot should mention: but it look it crashed and fail instead

this PR appears to contain configuration changes. Please ensure that a corresponding deployment ticket has been filed with the new configuration values.

but I don't think I can create such ticket as outsider: may I ask adding one by employee?

@sheurich
Copy link
Contributor

sheurich commented Feb 8, 2024

integration not yet written : as it adds change to PA config to add a challenge type: and need certbot side change for write integration test: but unittest wise it mostly works:

I added an initial integration test for dns-account-01 in 34df64d which can be cherry-picked. It is currently passing.

@sheurich
Copy link
Contributor

sheurich commented Feb 8, 2024

Instructions and results are #7319 (comment)

@orangepizza
Copy link
Contributor Author

@sheurich cherry picked it, and made int only run at config_next (because it's only enabled there

@sheurich
Copy link
Contributor

I think we'll need to provide a DNS-ACCOUNT-01 challenge in addition to DNS-01 for wildcard domains.

Here is a change to do so:

diff --git a/policy/pa.go b/policy/pa.go
index 992b95d31..81f4735d4 100644
--- a/policy/pa.go
+++ b/policy/pa.go
@@ -527,8 +527,10 @@ func (pa *AuthorityImpl) challengeTypesFor(identifier identifier.ACMEIdentifier
)
                                "Challenges requested for wildcard identifier but DNS based " +
                                        "challenge type is not enabled")
                }
-               // Only provide a DNS-01-Wildcard challenge
-               challenges = []core.AcmeChallenge{core.ChallengeTypeDNS01}
+               // Only provide DNS-based challenges
+               challenges = []core.AcmeChallenge{
+                       core.ChallengeTypeDNS01, core.ChallengeTypeDNSAccount01,
+               }
        } else {
                // Otherwise we collect up challenges based on what is enabled.
                if pa.ChallengeTypeEnabled(core.ChallengeTypeHTTP01) {

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.

None yet

3 participants