diff --git a/certbot/src/certbot/_internal/plugins/manual.py b/certbot/src/certbot/_internal/plugins/manual.py index 950eddc63..35abc6b51 100644 --- a/certbot/src/certbot/_internal/plugins/manual.py +++ b/certbot/src/certbot/_internal/plugins/manual.py @@ -110,13 +110,15 @@ permitted by DNS standards.) help='Path or command to execute for the authentication script') add('cleanup-hook', help='Path or command to execute for the cleanup script') + add('setup-hook', + help='Path or command to execute just this once. Only used for dns-persist challenges') def prepare(self) -> None: # pylint: disable=missing-function-docstring self._validate_hooks() def _validate_hooks(self) -> None: if self.config.validate_hooks: - for name in ('auth-hook', 'cleanup-hook'): + for name in ('auth-hook', 'cleanup-hook', 'setup-hook'): hook = self.conf(name) if hook is not None: hook_prefix = self.option_name(name)[:-len('-hook')] @@ -168,22 +170,54 @@ permitted by DNS standards.) # pylint: disable=unused-argument,missing-function-docstring return [challenges.HTTP01, challenges.DNS01, challenges.DNSPersist01] + def _check_hook_achall_match(self, achalls: list[achallenges.AnnotatedChallenge]) -> None: + """Checks whether the user has provided the appropriate hook type for the given list of + challenges, and emits a helpful warning if not. Raises an exception if we're in + non-interactive mode, have at least 1 non-DNS-PERSIST-01 challenge, and no auth hook + was provided.""" + num_dns_persist_achalls = sum(1 for achall in achalls \ + if isinstance(achall.chall, challenges.DNSPersist01)) + num_other_achalls = len(achalls) - num_dns_persist_achalls + auth_hook_provided = self.conf('auth-hook') + setup_hook_provided = self.conf('setup-hook') + if num_other_achalls > 0 and self.config.noninteractive_mode and not auth_hook_provided: + raise errors.PluginError( + 'An authentication script must be provided with --{0} when using the manual ' + 'plugin non-interactively on HTTP-01 or DNS-01 challenges.'.format( + self.option_name('auth-hook'))) + if num_dns_persist_achalls == 0 and setup_hook_provided: + msg = ('A setup script was provided with --{0}, but because no DNS-PERSIST-01 ' + 'challenges are being attempted, it will be ignored.').format( + self.option_name('setup-hook')) + if not auth_hook_provided: + msg += ' Did you mean to use --{0} instead?'.format( + self.option_name('auth-hook')) + logger.warning(msg) + if num_other_achalls == 0 and auth_hook_provided: + msg = ('An authentication script was provided with --{0}, but because only ' + 'DNS-PERSIST-01 challenges are being attempted, it will be ignored.').format( + self.option_name('auth-hook')) + if not setup_hook_provided: + msg += ' Did you mean to use --{0} instead?'.format( + self.option_name('setup-hook')) + logger.warning(msg) + + def _achall_has_hook(self, achall: achallenges.AnnotatedChallenge) -> bool: + if isinstance(achall.chall, challenges.DNSPersist01): + return self.conf('setup-hook') + else: + return self.conf('auth-hook') + def perform(self, achalls: list[achallenges.AnnotatedChallenge] ) -> list[challenges.ChallengeResponse]: # pylint: disable=missing-function-docstring + self._check_hook_achall_match(achalls) responses = [] last_dns_achall = 0 for i, achall in enumerate(achalls): - # only dns-persist-01 challenges should be both non-interactive and have no auth hook - if not isinstance(achall.chall, challenges.DNSPersist01) \ - and self.config.noninteractive_mode and not self.conf('auth-hook'): - raise errors.PluginError( - 'An authentication script must be provided with --{0} when ' - 'using the manual plugin non-interactively.'.format( - self.option_name('auth-hook'))) if isinstance(achall.chall, (challenges.DNS01, challenges.DNSPersist01)): last_dns_achall = i for i, achall in enumerate(achalls): - if self.conf('auth-hook'): + if self._achall_has_hook(achall): self._perform_achall_with_script(achall, achalls) else: self._perform_achall_manually(achall, i == last_dns_achall) @@ -222,7 +256,11 @@ permitted by DNS standards.) else: os.environ.pop('CERTBOT_TOKEN', None) os.environ.update(env) - _, out = self._execute_hook('auth-hook', identifier_value) + if isinstance(achall.chall, challenges.DNSPersist01): + hook_name = 'setup-hook' + else: + hook_name = 'auth-hook' + _, out = self._execute_hook(hook_name, identifier_value) env['CERTBOT_AUTH_OUTPUT'] = out.strip() self.env[achall] = env @@ -265,6 +303,8 @@ permitted by DNS standards.) def cleanup(self, achalls: Iterable[achallenges.AnnotatedChallenge]) -> None: # pylint: disable=missing-function-docstring if self.conf('cleanup-hook'): for achall in achalls: + if isinstance(achall.chall, challenges.DNSPersist01): + continue env = self.env.pop(achall) if 'CERTBOT_TOKEN' not in env: os.environ.pop('CERTBOT_TOKEN', None) diff --git a/certbot/src/certbot/_internal/storage.py b/certbot/src/certbot/_internal/storage.py index a7f19d0dc..02465ff2d 100644 --- a/certbot/src/certbot/_internal/storage.py +++ b/certbot/src/certbot/_internal/storage.py @@ -280,6 +280,12 @@ def _relevant(namespaces: Iterable[str], option: str) -> bool: """ from certbot._internal import renewal + # an awkward special case: future renewals shouldn't depend on whether the user set + # --manual-setup-hook now, since it's meant to represent a (potentially) one-off script. + # if a user wants it to run again in the future, they must set it explicitly via CLI + if option == "manual_setup_hook": + return False + return (option in renewal.CONFIG_ITEMS or any(option.startswith(namespace) for namespace in namespaces)) diff --git a/certbot/src/certbot/_internal/tests/plugins/manual_test.py b/certbot/src/certbot/_internal/tests/plugins/manual_test.py index 9ab78da14..b7b0f3704 100644 --- a/certbot/src/certbot/_internal/tests/plugins/manual_test.py +++ b/certbot/src/certbot/_internal/tests/plugins/manual_test.py @@ -49,8 +49,8 @@ class AuthenticatorTest(test_util.TempDirTestCase): # initialization. self.config = mock.MagicMock( http01_port=0, manual_auth_hook=None, manual_cleanup_hook=None, - noninteractive_mode=False, validate_hooks=False, - config_dir=os.path.join(self.tempdir, "config_dir"), + manual_setup_hook=None, noninteractive_mode=False, + validate_hooks=False, config_dir=os.path.join(self.tempdir, "config_dir"), work_dir=os.path.join(self.tempdir, "work_dir"), backup_dir=os.path.join(self.tempdir, "backup_dir"), temp_checkpoint_dir=os.path.join( @@ -83,49 +83,55 @@ class AuthenticatorTest(test_util.TempDirTestCase): [challenges.HTTP01, challenges.DNS01, challenges.DNSPersist01] def test_script_perform(self): - self.config.manual_auth_hook = ( - '{0} -c "' - 'from certbot.compat import os;' - 'print(os.environ.get(\'CERTBOT_DOMAIN\'));' - 'print(os.environ.get(\'CERTBOT_IDENTIFIER\'));' - 'print(os.environ.get(\'CERTBOT_TOKEN\', \'notoken\'));' - 'print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\'));' - 'print(os.environ.get(\'CERTBOT_ALL_DOMAINS\'));' - 'print(os.environ.get(\'CERTBOT_ALL_IDENTIFIERS\'));' - 'print(os.environ.get(\'CERTBOT_REMAINING_CHALLENGES\'));"' - .format(sys.executable)) - dns_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}\n{6}'.format( + script_template = textwrap.dedent(""" + {0} -c "from certbot.compat import os + print(os.environ.get(\'CERTBOT_DOMAIN\')) + print(os.environ.get(\'CERTBOT_IDENTIFIER\')) + print(os.environ.get(\'CERTBOT_TOKEN\', \'notoken\')) + print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\')) + print(os.environ.get(\'CERTBOT_ALL_DOMAINS\')) + print(os.environ.get(\'CERTBOT_ALL_IDENTIFIERS\')) + print(os.environ.get(\'CERTBOT_REMAINING_CHALLENGES\')) + print('{1}')" + """) + self.config.manual_auth_hook = script_template.format(sys.executable, "auth_hook") + self.config.manual_setup_hook = script_template.format(sys.executable, "setup_hook") + dns_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}\n{6}\n{7}'.format( self.dns_achall.identifier.value, self.dns_achall.identifier.value, 'notoken', self.dns_achall.validation(self.dns_achall.account_key), ','.join(achall.identifier.value for achall in self.achalls), ','.join(achall.identifier.value 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}\n{5}\n{6}'.format( + len(self.achalls) - self.achalls.index(self.dns_achall) - 1, + "auth_hook") + http_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}\n{6}\n{7}'.format( self.http_achall.identifier.value, self.http_achall.identifier.value, self.http_achall.chall.encode('token'), self.http_achall.validation(self.http_achall.account_key), ','.join(achall.identifier.value for achall in self.achalls), ','.join(achall.identifier.value for achall in self.achalls), - len(self.achalls) - self.achalls.index(self.http_achall) - 1) - dns_persist_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}\n{6}'.format( + len(self.achalls) - self.achalls.index(self.http_achall) - 1, + "auth_hook") + dns_persist_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}\n{6}\n{7}'.format( self.dns_persist_achall.identifier.value, self.dns_persist_achall.identifier.value, 'notoken', self.dns_persist_achall.get_validation_rdata(False), ','.join(achall.identifier.value for achall in self.achalls), ','.join(achall.identifier.value for achall in self.achalls), - len(self.achalls) - self.achalls.index(self.dns_persist_achall) - 1) - dns_persist_wildcard_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}\n{6}'.format( + len(self.achalls) - self.achalls.index(self.dns_persist_achall) - 1, + "setup_hook") + dns_persist_wildcard_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}\n{6}\n{7}'.format( self.dns_persist_achall_wildcard.identifier.value, self.dns_persist_achall_wildcard.identifier.value, 'notoken', self.dns_persist_achall_wildcard.get_validation_rdata(True), ','.join(achall.identifier.value for achall in self.achalls), ','.join(achall.identifier.value for achall in self.achalls), - len(self.achalls) - self.achalls.index(self.dns_persist_achall_wildcard) - 1) + len(self.achalls) - self.achalls.index(self.dns_persist_achall_wildcard) - 1, + "setup_hook") assert self.auth.perform(self.achalls) == self.responses assert self.auth.env[self.dns_achall]['CERTBOT_AUTH_OUTPUT'] == \ @@ -164,6 +170,8 @@ class AuthenticatorTest(test_util.TempDirTestCase): self.auth.perform(self.achalls) for achall in self.achalls: + if isinstance(achall.chall, challenges.DNSPersist01): + continue self.auth.cleanup([achall]) assert os.environ['CERTBOT_AUTH_OUTPUT'] == 'foo' assert os.environ['CERTBOT_DOMAIN'] == achall.identifier.value @@ -177,7 +185,6 @@ class AuthenticatorTest(test_util.TempDirTestCase): else: assert 'CERTBOT_TOKEN' not in os.environ - def test_auth_hint_hook(self): self.config.manual_auth_hook = '/bin/true' for dns_achall in [acme_util.DNS01_A, acme_util.DNS_PERSIST_01_A]: diff --git a/certbot/src/certbot/_internal/tests/storage_test.py b/certbot/src/certbot/_internal/tests/storage_test.py index da3bd25ce..409e90336 100644 --- a/certbot/src/certbot/_internal/tests/storage_test.py +++ b/certbot/src/certbot/_internal/tests/storage_test.py @@ -13,7 +13,7 @@ import pytest import certbot from certbot import errors -from certbot._internal.storage import ALL_FOUR +from certbot._internal.storage import ALL_FOUR, relevant_values from certbot._internal import san from certbot.compat import filesystem from certbot.compat import os @@ -112,6 +112,13 @@ class RelevantValuesTest(unittest.TestCase): expected_relevant_values = self.values.copy() assert self._call(self.values) == expected_relevant_values + def test_manual_hooks(self): + self.values["manual_setup_hook"] = '"# setup' + self.values["manual_auth_hook"] = '"# auth' + expected_relevant_values = self.values.copy() + del expected_relevant_values["manual_setup_hook"] + assert self._call(self.values) == expected_relevant_values + class BaseRenewableCertTest(test_util.ConfigTestCase): """Base class for setting up Renewable Cert tests.