Support for DNS validation

lordelph commented 4 years ago

This adds support for DNS validation, documentation for this has been added to the README. This does not include specific support for any particular DNS provider API, the user must provide that.

Also includes a fix for Client::validate which has an erroneous loop condition and invalid status comparison (also suggested in earlier pull request)

bakkerpeter commented 4 years ago

Thanks for the great addition to this, it looks all good, I have just some small suggestions.

I like the idea of the helper method to test DNS propagation, but I think it would be more declarative if we just pass an array of $authorization objects to it. Also I would like to add some improvements along the way as a domain can have more than one active TXT record.

Like to hear what you think!


/**
* Wait until a set of DNS records return specific TXT record values
*
* @param Authorization[] Set of authorizations
* @param int $maxSeconds to wait
* @return bool true if record found, false otherwise
*/
public static function waitForDNS(array $authorizations, $maxSeconds=60) {

   do {
      foreach($authorizations as $number => $authorization) {
         $record = $authorization->getTxtRecord($challenge);
         $key='_acme-challenge.'.$authorization->getDomain();
         $challenge = $authorization->getDnsChallenge();
         $txtRecord = $authorization->getTxtRecord($challenge);
         $results = dns_get_record($authorization->getDomain(), DNS_TXT);

         foreach ($results as $result) {
           if ($result['txt']==$txtRecord) {
              unset($authorizations[$number]);
           }
         }
      }
//...

Thanks again for sparking the DNS feature.

lordelph commented 4 years ago

Yes, that's a good suggestion, do you want me to update the pull request?

Also, because LE now checks DNS from multiple locations, I've found it's not always good enough to wait for your local DNS resolver to see the changes, and I do an extra sleep afterwards - to ensure others aren't caught out, maybe the helper should include a minimum time and a maximum time?

bakkerpeter commented 4 years ago

Hi @lordelph, I made some changes to the package to implement the DNS feature easier and clearer. Although in the process I caused some conflicts. I will resolve them along with your commits.

lordelph commented 4 years ago

Sounds good. Please note your version of waitForDNS outlined above doesn't check the right domain - in your example it should pass $key to dns_get_record

It might provide some more clarity to provide a $txtDomain = $authorization->getTxtDomain($challenge); method to allow the DNS record domain to be obtained.

bakkerpeter commented 4 years ago

Thanks for pointing that out. I went ahead and created a new branch based on your commits. I have opened a new PR here: https://github.com/afosto/yaac/pull/5 I think we should close this pull request, I'm curious to what you think of the PR.

bakkerpeter commented 4 years ago

Solved in https://github.com/afosto/yaac/pull/5

afosto / yaac

Support for DNS validation #2