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

Feature Request: dns-01 should account for wildcard domains #151

Closed
willseward opened this issue Mar 13, 2016 · 13 comments
Closed

Feature Request: dns-01 should account for wildcard domains #151

willseward opened this issue Mar 13, 2016 · 13 comments

Comments

@willseward
Copy link
Contributor

Assuming someone had the domain example.com and server1.example.com were a CNAME for example.com, the dns-01 certificate request would fail. This is the case for topologies using L7 load balancing.

After skimming the ACME protocol, I don't see any prohibition for looking for server1.example.com zone first, then after failing to find it, looking for example.com, and so on... Correct me if I'm wrong.

@willseward
Copy link
Contributor Author

Something like this: 5785077

I'm not a go programmer, so I'm hesitant to put in a PR

@xenolf
Copy link
Member

xenolf commented Mar 14, 2016

Hey @willseward! I'm pretty sure we support CNAMES when checking for a live record. The support was added in #116. We are checking if the DNS record we request a cert for is in fact a CNAME and if it is, we use the delegated record for our logic. (see here)

If I misunderstood the problem you are describing, please correct me 😟

@willseward
Copy link
Contributor Author

That commit may solve the problem on other providers, but the DO provider is still broken with respect to this issue. If you don't have the dns setup as a zone in DO, it will return a 404 and fail. It would be an easy fix to expand the Present method to this Present(domain, authZone, token, keyAuth string), so that DO would know where to place the TXT record.

@xenolf
Copy link
Member

xenolf commented Mar 15, 2016

Oh I'm sorry - I didn't understand this as a DO problem at all as you weren't mentioning it. So if I'm getting it straight now, the issue is that the DO API returns a 404 if the validation domain is not a zone but merely belongs to a wildcard CNAME?

Changing Present would not help with this I'm afraid as the code I linked to actually runs after the provider added the TXT record to validate it.

Are setups like *.example.com CNAME to examplestatic.com also feasible? Because simply walking up the domain tree wouldn't catch this.

@willseward
Copy link
Contributor Author

As long as example.com is the authoritative zone for *.example.com, placing a TXT record in example.com should validate successfully. If the CNAME for *.example.com points to examplestatic.com, I don't think that makes much of a difference.

I made some changes to the dns-01 test so that the provider will create a TXT record for the validation domain, but adds the record to the authoritative zone (hence the authZone in my previous comment) and everything seems to works fine.

Like this:

zone, zerr := findZoneByFqdn(toFqdn(domain), recursiveNameserver)
if zerr != nil {
    log.Printf("Zone for '%s' could not be determined", domain)
    return zerr
}
log.Printf("Zone for domain is '%s'", zone)

dnsZone := unFqdn(zone)

// Generate the Key Authorization for the challenge
keyAuth, err := getKeyAuthorization(chlng.Token, s.jws.privKey)
if err != nil {
    return err
}

err = s.provider.Present(domain, dnsZone, chlng.Token, keyAuth)
if err != nil {
    return fmt.Errorf("Error presenting token %s", err)
}
defer func() {
    err := s.provider.CleanUp(domain, dnsZone, chlng.Token, keyAuth)
    if err != nil {
        log.Printf("Error cleaning up %s %v ", domain, err)
    }
}()

Again, excuse my go...

@janeczku
Copy link
Contributor

@xenolf It's a bug in the DO provider. Any certificate request for a subdomain will fail because the Provide() method tries to create the validation record in the non-existent zone subdomain.example.com instead of in example.com.
See https://github.com/xenolf/lego/blob/master/providers/dns/digitalocean/digitalocean.go#L53
/cc @mholt

@mholt
Copy link
Contributor

mholt commented Mar 16, 2016

Oops. I won't have time to fix that in the near future. Looks like @willseward probably has a good start on it.

@willseward
Copy link
Contributor Author

I'd be more than happy to fix it

Wills Ward

On Mar 16, 2016, at 11:22 AM, Matt Holt notifications@github.com wrote:

Oops. I won't have time to fix that in the near future. Looks like @willseward probably has a good start on it.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@willseward
Copy link
Contributor Author

#159

I might have refactored more than I should have. Let me know what you think.

@xenolf
Copy link
Member

xenolf commented Mar 18, 2016

Alright :)

As the scope of this problem is only the DO provider, we should fix it in there.
@willseward If you want to tackle this then please open a PR with your changes and we go from there.

@xenolf
Copy link
Member

xenolf commented Mar 18, 2016

Oh wow, what timing :)

@willseward
Copy link
Contributor Author

Well, I can submit a PR with just DO changes if necessary.

@xenolf
Copy link
Member

xenolf commented Apr 11, 2016

This was fixed by #176

@xenolf xenolf closed this as completed Apr 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants