-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Refactor DNS check #116
Merged
Merged
Refactor DNS check #116
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The discussion in a previous thread still applies to the current commit. I'm commenting again so it appears here. [ cc: @janeczku ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation domain (fqdn) and domain are owned by the same DNS authority, hence they share the same set of nameservers. The reason we are using the domain to lookup the authoritative NS is that we expect the domain to exist, while the fqdn record may not yet have propagated in the DNS system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is legal to delegate the validation domain to a different set of nameservers, in which case
lookupNameservers(toFqdn(domain))
andlookupNameservers(toFqdn(fqdn))
will have different results.It is also legal to complete a DNS-01 challenge by publishing a TXT record for
fqdn
even if the public authoritative nameservers have no records at all fordomain
(e.g. split horizon DNS), as I mentioned in a second thread on the previous PR. It's not correct to requiredomain
to exist.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree. The ACME spec could not be more clearer:
Where does it tell you to delegate the validation domain to another zone?
Also, doing so would undermine the whole objective of the DNS challenge, which is to establish proof of ownership of the certificate domain by the client.
If the validation domain is by itself a root domain (delegated to a different zone), then by provisioning a TXT record under it, you do not prove authority over the (parent) certificate domain, which may be owned by a third party. (e.g.
_acme-challenge.co.uk.
-> you, butco.uk.
->ns1.nic.uk
)Lastly, why are we even having this academic discussion? Lego solves DNS challenge by provisioning a TXT record under the certificate domain. No zone delegation is happening, which is why we don't need to handle such case in the DNS check (irrespective of whether that would be technically legal).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not require the certificate domain to exist. E.g, if the certificate domain www.example.com has no records (NXDOMAIN) the DNS check will still pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. You don't provision records on the domain name being validated, you provision records for the validation domain name. The DNS-01 section doesn't concern the domain name at all – only the validation domain name,
fqdn
in this context. DNS-01 doesn't make any requirements of the domain name, only of the validation domain name.Where does it tell you I can't delegate it?
The DNS-01 challenge does not require asserting control over the certificate domain. If that were the goal, it could require the TXT record to be published on the certificate domain instead. (It's already specified to ignore non-matching responses, so there's no conflict even if you've already got TXT records on that name.) Why use a separate validation domain name at all?
There are legitimate use cases for delegating
_acme-challenge
– as I mentioned in the previous thread, the situation where it is impractical to modifybar.com
from a cron job because of grumpy change control officers (and the compliance reasons behind their grumpiness). It is desirable in such situations to delegate_acme-challenge.foo.bar.com
out of thebar.com
zone.DNS-01 does not concern itself with the certificate domain name, it concerns itself with the validation domain name. If you can control the validation domain name, regardless of which zone or nameservers you use to do it, that is 100% sufficient to satisfy a DNS-01 challenge. Delegation is a part of DNS, and I maintain that it is useful to delegate ACME challenges.
As for
_acme-challenge.co.uk
, please see the complex history of underscores in DNS. (Or just try registering that domain name.) It is not an accident that_acme-challenge
contains an underscore.It provisions a TXT record under the validation domain.
RFC2136_ZONE=_acme-challenge.foo.bar.com lego --dns=rfc2136
can easily create records in a delegated zone, and of course there's--dns=manual
.We're having this discussion because delegation is a situation where this check would fail while
boulder
would succeed, and I consider that a bug in this check.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A TXT record can not co-exist with a NS record for the same name if the name is a subdomain and not the root domain. Hence the spec describing you to create a TXT record under the validation domain by implication means that you should not create a NS record there which would be necessary for your scenario of delegation.
But again, this discussion is out of scope for this PR as Lego's DNS providers don't delegate the validation domain anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can and does.(Edit: Apologies, I read your statement wrong. You're correct that it can't exist as a sibling to theIN NS
delegation in the parent zone, but as I illustrate, that's not at issue.)Scenario: issuing a certificate for
is-delegated.willglynn.com
.willglynn.com IN NS ns1.willglynn.com
, which haswillglynn.com IN SOA ns1.willglynn.com
. Within thewillglynn.com
zone,is-delegated.willglynn.com
does not exist, but_acme-challenge.is-delegated.willglynn.com IN NS ns1.lerfjhax.com
.ns1.lerfjhax.com
has a zone_acme-challenge.is-delegated.willglynn.com IN SOA ns1.lerfjhax.com
. Within that zone,_acme-challenge.is-delegated.willglynn.com IN NS ns1.lerfjhax.com
, agreeing with the delegation, and there exists an_acme-challenge.is-delegated.willglynn.com IN TXT
record as well.Asking
lookupNameservers(toFqdn(domain))
about the validation domain returns a referral, sincens1.willglynn.com
knows it is not an authority for the validation domain:Asking the nameserver which is actually an authority for the validation domain name – i.e.
lookupNameservers(toFqdn(fqdn))
– returns the TXT record:And yes, this works:
…unless you delegate the validation domain ahead of time and only need
lego
to create the TXT record in the delegated zone. As I said,RFC2136_ZONE=_acme-challenge.foo.bar.com lego --dns=rfc2136
can do that right now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, thats a valid (if edgy) scenario.