Repository: certbot/certbot
Pull request: Exposes environment variable to let hooks scripts know when the last challenge is handled
Fixes #5484 

This PRs makes Certbot expose two new environment variables in the auth and cleanup hooks of the `manual` plugin:
* `CERTBOT_REMAINING_CHALLENGES` contains the number of challenges that remain after the current one (so it equals to 0 when the script is called for the last challenge)
* `CERTBOT_ALL_DOMAINS` contains a comma-separated list of all domains concerned by a challenge for the current certificate

With these variables, an hook script can know when it is run for the last time, and then trigger appropriate finalizers for all challenges that have been executed. This will be particularly useful for certificates with a lot of domains validated with DNS-01 challenges: instead of waiting on each hook execution to check that the relevant DNS TXT entry has been inserted, these waits can be avoided thanks to the latest hook verifying all domains in one run.

Problem statement:
Feature request: Add option to know what is the last domain passed to dns hook script
Hi,

certbot doesn't provide an official hook to work with the API of my DNS provider so I'm using my own script. Well, we know every time we create/modify a record on our DNS provider it needs to propagate to all the authoritative DNS servers, in my case it takes 1 minute and 30 seconds approx., right now, as far as I know, there is no option to know what is the last domain that is being issued so to be bullet proof, every time the script creates the _acme_challenge TXT record, I wait 1:30 minutes, well I use the API to know when the propagation is finished but as I said, it takes 1:30 minutes. If I want to issue a certificate with 100 domains it will take approx. 2 hours and 30 minutes to complete the request. If I had a way to know what is the last domain I could write all the TXT records and when the last domain is created, I could add the check to know when the propagation has finished (or just a simple sleep...) so it takes only 1:30 minutes instead of 2 hours and 30 minutes.

It would be great if certbot provided a variable like `CERTBOT_LAST_DOMAIN=true` or something like that so the script could have the logic to sleep the right amount of time to let finish the DNS  propagation or to perform the needed checks only when there are no more domains to add to our DNS provider.

As a workaround I pass the last domain to my script:

`./certbot-auto certonly --manual --preferred-challenges=dns --manual-auth-hook "/path/to/hook/dnsapi.sh yyy.domain.tld" --manual-cleanup-hook /path/to/hook/dnsapi-cleanup.sh -d zzz.domain.tld,aaa.domain.tld,yyy.domain.tld`

and it works perfectly but would be great to have this feature added directly to certbot.

Thank you in advance.

Cheers,
sahsanu

Hints from issue discussion:
Sounds like a question for @bmw
We're currently rethinking a bit about how our plugins work, but if we don't end up making major changes there, I think can and should do something like this.
That sounds good, we'll appreciate it.

Thank you very much. 

-----Original Message-----
From: Brad Warren <notifications@github.com>
To: certbot/certbot <certbot@noreply.github.com>
Cc: sahsanu <raul@sahsanu.com>, Author <author@noreply.github.com>
Sent: jue., 18 abr. 2019 20:43
Subject: Re: [certbot/certbot] Feature request: Add option to know what is the last domain passed to dns hook script (#5484)

We're currently rethinking a bit about how our plugins work, but if we don't end up making major changes there, I think can and should do something like this.

-- 
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/certbot/certbot/issues/5484#issuecomment-484635238
We'd accept a PR for this which adds an environment variable like `CERTBOT_REMAINING_CHALLENGES` that is set for both `--manual-auth-hook` and `--manual-cleanup-hook`. The value would be the number of additional times Certbot will call the hook for the current certificate. When the hook is called for the last challenge, `CERTBOT_REMAINING_CHALLENGES` would be 0.

This was discussed in https://github.com/certbot/certbot/pull/6607.
@bmw @sahsanu  I would love `CERTBOT_REMAINING_CHALLENGES`, this would speed up cert generation for us. Currently with manual-auth-hook for 28 subdomain wildcard SAN, 1 root domain wildcard SAN, 1 root domain, we are at 45 - 50 mins to generate a cert.

A implementation like this will greatly help us. Can you please consider opening this issue?

https://community.letsencrypt.org/t/parallel-run-of-certbot-manual-auth-hook-manual-cleanup-hook-for-validation-of-multidomain-cert/115352
@sahsanu in the meantime you can slightly improve the time it takes for resolution with a similar implementation, based on your DNS provider, this might be helpful or not.

```
timeout 120 sh << EOF
NAME_SERVERS="$(dig +short -t NS ${DOMAIN} | tr '\n' ' ')"
echo "NAME_SERVERS=\${NAME_SERVERS}"
for NS in \$NAME_SERVERS; do
  echo "NS=\${NS}"
  until dig +short -t TXT ${ACME_CHALLENGE_DOMAIN} @\${NS} | grep -q -- "${CERTBOT_VALIDATION}"; do
    echo "$(date -Is) waiting for DNS resolution of ${ACME_CHALLENGE_DOMAIN} to show ${CERTBOT_VALIDATION} on \${NS}"
    sleep 1
  done
  echo "resolved on \${NS}"
done
echo "resolved on all name servers: \${NAME_SERVERS}"
EOF

echo "$(date -Is) sleeping for 15s for DNS propogation."
sleep 15
```

Commit message: Less variables and less lines

Patch to review:
diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py
index 3204fe1dab2..1804e7d7e49 100644
--- a/certbot/certbot/_internal/plugins/manual.py
+++ b/certbot/certbot/_internal/plugins/manual.py
@@ -109,14 +109,13 @@ def get_chall_pref(self, domain):
 
     def perform(self, achalls):  # pylint: disable=missing-function-docstring
         self._verify_ip_logging_ok()
-        if self.conf('auth-hook'):
-            perform_achall = self._perform_achall_with_script
-        else:
-            perform_achall = self._perform_achall_manually
 
         responses = []
         for achall in achalls:
-            perform_achall(achall)
+            if self.conf('auth-hook'):
+                self._perform_achall_with_script(achall, achalls)
+            else:
+                self._perform_achall_manually(achall)
             responses.append(achall.response(achall.account_key))
         return responses
 
@@ -134,9 +133,11 @@ def _verify_ip_logging_ok(self):
             else:
                 raise errors.PluginError('Must agree to IP logging to proceed')
 
-    def _perform_achall_with_script(self, achall):
+    def _perform_achall_with_script(self, achall, achalls):
         env = dict(CERTBOT_DOMAIN=achall.domain,
-                   CERTBOT_VALIDATION=achall.validation(achall.account_key))
+                   CERTBOT_VALIDATION=achall.validation(achall.account_key),
+                   CERTBOT_ALL_DOMAINS=','.join([one_achall.domain for one_achall in achalls]),
+                   CERTBOT_REMAINING_CHALLENGES=str(len(achalls) - achalls.index(achall) - 1))
         if isinstance(achall.chall, challenges.HTTP01):
             env['CERTBOT_TOKEN'] = achall.chall.encode('token')
         else:
diff --git a/certbot/tests/plugins/manual_test.py b/certbot/tests/plugins/manual_test.py
index bd11a9538d6..b4e19c420d0 100644
--- a/certbot/tests/plugins/manual_test.py
+++ b/certbot/tests/plugins/manual_test.py
@@ -72,16 +72,23 @@ def test_script_perform(self):
         self.config.manual_public_ip_logging_ok = True
         self.config.manual_auth_hook = (
             '{0} -c "from __future__ import print_function;'
-            'from certbot.compat import os;  print(os.environ.get(\'CERTBOT_DOMAIN\'));'
+            'from certbot.compat import os;'
+            'print(os.environ.get(\'CERTBOT_DOMAIN\'));'
             'print(os.environ.get(\'CERTBOT_TOKEN\', \'notoken\'));'
-            'print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\'));"'
+            'print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\'));'
+            'print(os.environ.get(\'CERTBOT_ALL_DOMAINS\'));'
+            'print(os.environ.get(\'CERTBOT_REMAINING_CHALLENGES\'));"'
             .format(sys.executable))
-        dns_expected = '{0}\n{1}\n{2}'.format(
+        dns_expected = '{0}\n{1}\n{2}\n{3}\n{4}'.format(
             self.dns_achall.domain, 'notoken',
-            self.dns_achall.validation(self.dns_achall.account_key))
-        http_expected = '{0}\n{1}\n{2}'.format(
+            self.dns_achall.validation(self.dns_achall.account_key),
+            ','.join([achall.domain for achall in self.achalls]),
+            len(self.achalls) - self.achalls.index(self.dns_achall) - 1)
+        http_expected = '{0}\n{1}\n{2}\n{3}\n{4}'.format(
             self.http_achall.domain, self.http_achall.chall.encode('token'),
-            self.http_achall.validation(self.http_achall.account_key))
+            self.http_achall.validation(self.http_achall.account_key),
+            ','.join([achall.domain for achall in self.achalls]),
+            len(self.achalls) - self.achalls.index(self.http_achall) - 1)
 
         self.assertEqual(
             self.auth.perform(self.achalls),


Code review comments:
diff_hunk: @@ -134,9 +133,11 @@ def _verify_ip_logging_ok(self):
             else:
                 raise errors.PluginError('Must agree to IP logging to proceed')
 
-    def _perform_achall_with_script(self, achall):
+    def _perform_achall_with_script(self, achall, achalls):
         env = dict(CERTBOT_DOMAIN=achall.domain,
-                   CERTBOT_VALIDATION=achall.validation(achall.account_key))
+                   CERTBOT_VALIDATION=achall.validation(achall.account_key),
+                   CERTBOT_ALL_DOMAINS=','.join([one_achall.domain for one_achall in achalls]),
path: certbot/certbot/_internal/plugins/manual.py
text: @user1:
Python does not really require the `[]` here because `join()` can act on a generator, not just a list. It would work equally well to say

`.join(one_achall.domain for one_achall in achalls)`

since at least Python 2.6 and probably longer ago. :slightly_smiling_face: 

@author:
Thanks, I fixed that.