From 83afb58a9a6099ad1e3c54097c2bb509e98f38f8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 1 Feb 2016 17:20:20 -0800 Subject: [PATCH 01/73] Avoid dangerous and mysterious behaviour if someone tries to modify a config --- letsencrypt/configuration.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index afd5edbe4..d6a016cea 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -43,6 +43,12 @@ class NamespaceConfig(object): # Check command line parameters sanity, and error out in case of problem. check_config_sanity(self) + # We're done setting up the attic. Now pull up the ladder after ourselves... + self.__setattr__ = self.__setattr_implementation__ + + def __setattr_implementation__(self, var, value): + return self.namespace.__setattr__(var, value) + def __getattr__(self, name): return getattr(self.namespace, name) From 5337fdec2394a132f5a69d29964229afa543708e Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 1 Feb 2016 17:24:05 -0800 Subject: [PATCH 02/73] Work in progress - make renew verb/main loop --- letsencrypt/cli.py | 78 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 61bc85e72..032248611 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -69,6 +69,7 @@ the cert. Major SUBCOMMANDS are: (default) run Obtain & install a cert in your current webserver certonly Obtain cert, but do not install it (aka "auth") install Install a previously obtained cert in a server + renew Renew previously obtained certs that are near expiry revoke Revoke a previously obtained certificate rollback Rollback server configuration changes made during install config_changes Show changes made to server config during installation @@ -259,7 +260,7 @@ def _treat_as_renewal(config, domains): return _handle_subset_cert_request(config, domains, subset_names_cert) def _handle_identical_cert_request(config, cert): - """Figure out what to do if a cert has the same names as a perviously obtained one + """Figure out what to do if a cert has the same names as a previously obtained one :param storage.RenewableCert cert: @@ -663,6 +664,79 @@ def install(args, config, plugins): le_client.enhance_config(domains, config) +def renew(args, config, plugins): + """Renew previously-obtained certificates.""" + print("Welcome to the renew verb!") + plugins = plugins_disco.PluginsRegistry.find_all() + cli_config = configuration.RenewerConfiguration(config) + configs_dir = cli_config.renewal_configs_dir + for renewal_file in os.listdir(configs_dir): + if not renewal_file.endswith(".conf"): + continue + print("Processing " + renewal_file) + # XXX: does this succeed in making a fully independent config object + # each time? + cli_config = configuration.RenewerConfiguration(config) + full_path = os.path.join(configs_dir, renewal_file) + try: + renewal_candidate = storage.RenewableCert(full_path, cli_config) + except (errors.CertStorageError, IOError): + logger.warning("Renewal configuration file %s is broken. " + "Skipping.", full_path) + continue + print(renewal_candidate.names(), renewal_candidate.should_autorenew()) + print("We should make a decision about whether to renew...!") + if "renewalparams" not in renewal_candidate.configuration: + logger.warning("Renewal configuration file %s lacks " + "renewalparams. Skipping.", full_path) + continue + renewalparams = renewal_candidate.configuration["renewalparams"] + if "authenticator" not in renewalparams: + logger.warning("Renewal configuration file %s does not specify " + "an authenticator. Skipping.", full_path) + continue + # ?? config = configuration.NamespaceConfig(_AttrDict(renewalparams)) + # XXX: also need: webroot_map + # XXX: also need: nginx_ and apache_ items + # string-valued items to add if they're present + for config_item in ["config_dir", "log_dir", "work_dir", "user_agent", + "server", "standalone_supported_challenges"]: + if config_item in renewalparams: + print("setting", config_item, renewalparams[config_item]) + cli_config.namespace.__setattr__(config_item, + renewalparams[config_item]) + # int-valued items to add if they're present + for config_item in ["rsa_key_size", "tls_sni_01_port", "http01_port"]: + if config_item in renewalparams: + try: + value = int(renewalparams[config_item]) + cli_config.namespace.__setattr__(config_item, value) + except ValueError: + logger.warning("Renewal configuration file %s specifies " + "a non-numeric value for %s. Skipping.", + full_path, config_item) + continue + # XXX: what does this do? + zope.component.provideUtility(cli_config) + try: + authenticator = plugins[renewalparams["authenticator"]] + except KeyError: + if "authenticator" in renewal_params: + logger.warning("Renewal configuration file %s specifies an " + "authenticator plugin (%s) that could not be " + "found. Skipping.", full_path, + renewal_params["authenticator"]) + else: + logger.warning("Renewal configuration file %s specifies no " + "authenticator plugin. Skipping.", full_path) + continue + authenticator = authenticator.init(cli_config) + le_client = _init_le_client(args, cli_config, authenticator, + authenticator) + # TODO: How do we handle the separate installer vs. authenticator + # the same as installer issue? + import code; code.interact(local=locals()) + def revoke(args, config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" # For user-agent construction @@ -781,7 +855,7 @@ class HelpfulArgumentParser(object): # Maps verbs/subcommands to the functions that implement them VERBS = {"auth": obtain_cert, "certonly": obtain_cert, "config_changes": config_changes, "everything": run, - "install": install, "plugins": plugins_cmd, + "install": install, "plugins": plugins_cmd, "renew": renew, "revoke": revoke, "rollback": rollback, "run": run} # List of topics for which additional help can be provided From 42cee297b8094e23fd7c690c39ce4c280a243a52 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 1 Feb 2016 17:31:00 -0800 Subject: [PATCH 03/73] Don't parse the cli twice --- letsencrypt/cli.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 032248611..6980cca4c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -664,11 +664,11 @@ def install(args, config, plugins): le_client.enhance_config(domains, config) -def renew(args, config, plugins): +def renew(args, cli_config, plugins): """Renew previously-obtained certificates.""" print("Welcome to the renew verb!") plugins = plugins_disco.PluginsRegistry.find_all() - cli_config = configuration.RenewerConfiguration(config) + # cli_config = configuration.RenewerConfiguration(config) configs_dir = cli_config.renewal_configs_dir for renewal_file in os.listdir(configs_dir): if not renewal_file.endswith(".conf"): @@ -676,10 +676,10 @@ def renew(args, config, plugins): print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? - cli_config = configuration.RenewerConfiguration(config) + config = configuration.RenewerConfiguration(config) full_path = os.path.join(configs_dir, renewal_file) try: - renewal_candidate = storage.RenewableCert(full_path, cli_config) + renewal_candidate = storage.RenewableCert(full_path, config) except (errors.CertStorageError, IOError): logger.warning("Renewal configuration file %s is broken. " "Skipping.", full_path) @@ -703,21 +703,21 @@ def renew(args, config, plugins): "server", "standalone_supported_challenges"]: if config_item in renewalparams: print("setting", config_item, renewalparams[config_item]) - cli_config.namespace.__setattr__(config_item, - renewalparams[config_item]) + config.namespace.__setattr__(config_item, + renewalparams[config_item]) # int-valued items to add if they're present for config_item in ["rsa_key_size", "tls_sni_01_port", "http01_port"]: if config_item in renewalparams: try: value = int(renewalparams[config_item]) - cli_config.namespace.__setattr__(config_item, value) + config.namespace.__setattr__(config_item, value) except ValueError: logger.warning("Renewal configuration file %s specifies " "a non-numeric value for %s. Skipping.", full_path, config_item) continue # XXX: what does this do? - zope.component.provideUtility(cli_config) + zope.component.provideUtility(config) try: authenticator = plugins[renewalparams["authenticator"]] except KeyError: @@ -730,9 +730,8 @@ def renew(args, config, plugins): logger.warning("Renewal configuration file %s specifies no " "authenticator plugin. Skipping.", full_path) continue - authenticator = authenticator.init(cli_config) - le_client = _init_le_client(args, cli_config, authenticator, - authenticator) + authenticator = authenticator.init(config) + le_client = _init_le_client(args, config, authenticator, authenticator) # TODO: How do we handle the separate installer vs. authenticator # the same as installer issue? import code; code.interact(local=locals()) From 1488a3c2b495bd67d287bf99f23ac498d43c6521 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 1 Feb 2016 18:10:59 -0800 Subject: [PATCH 04/73] Work in progress --- letsencrypt/cli.py | 19 +++++++++++-------- letsencrypt/configuration.py | 6 ++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ff2c916e5..602543225 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -672,8 +672,7 @@ def install(args, config, plugins): def renew(args, cli_config, plugins): """Renew previously-obtained certificates.""" print("Welcome to the renew verb!") - plugins = plugins_disco.PluginsRegistry.find_all() - # cli_config = configuration.RenewerConfiguration(config) + cli_config = configuration.RenewerConfiguration(cli_config) configs_dir = cli_config.renewal_configs_dir for renewal_file in os.listdir(configs_dir): if not renewal_file.endswith(".conf"): @@ -681,7 +680,7 @@ def renew(args, cli_config, plugins): print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? - config = configuration.RenewerConfiguration(config) + config = configuration.RenewerConfiguration(cli_config) full_path = os.path.join(configs_dir, renewal_file) try: renewal_candidate = storage.RenewableCert(full_path, config) @@ -702,20 +701,24 @@ def renew(args, cli_config, plugins): continue # ?? config = configuration.NamespaceConfig(_AttrDict(renewalparams)) # XXX: also need: webroot_map - # XXX: also need: nginx_ and apache_ items + # XXX: also need: nginx_, apache_, and plesk_ items # string-valued items to add if they're present for config_item in ["config_dir", "log_dir", "work_dir", "user_agent", "server", "standalone_supported_challenges"]: if config_item in renewalparams: - print("setting", config_item, renewalparams[config_item]) - config.namespace.__setattr__(config_item, - renewalparams[config_item]) + value = renewalparams[config_item] + # Unfortunately, we've lost type information from ConfigObj, + # so we don't know if the original was NoneType or str! + if value == "None": + value = None + print("setting", config_item, value) + config.__setattr__(config_item, value) # int-valued items to add if they're present for config_item in ["rsa_key_size", "tls_sni_01_port", "http01_port"]: if config_item in renewalparams: try: value = int(renewalparams[config_item]) - config.namespace.__setattr__(config_item, value) + config.__setattr__(config_item, value) except ValueError: logger.warning("Renewal configuration file %s specifies " "a non-numeric value for %s. Skipping.", diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index d6a016cea..5e54649e6 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -90,10 +90,16 @@ class RenewerConfiguration(object): def __init__(self, namespace): self.namespace = namespace + # We're done setting up the attic. Now pull up the ladder after ourselves... + self.__setattr__ = self.__setattr_implementation__ def __getattr__(self, name): return getattr(self.namespace, name) + def __setattr_implementation__(self, var, value): + print("in __setattr_implementation__, setting", var, value) + return self.namespace.__setattr__(var, value) + @property def archive_dir(self): # pylint: disable=missing-docstring return os.path.join(self.namespace.config_dir, constants.ARCHIVE_DIR) From 71bd458494d8290c95571a69104cc81f3b9537a0 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 1 Feb 2016 19:18:27 -0800 Subject: [PATCH 05/73] Further work in progress --- letsencrypt/cli.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 602543225..d6b7ab683 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -6,6 +6,7 @@ from __future__ import print_function # (TODO: split this file into main.py and cli.py) import argparse import atexit +import copy import functools import json import logging @@ -388,7 +389,7 @@ def _suggest_donate(): reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) -def _auth_from_domains(le_client, config, domains): +def _auth_from_domains(le_client, config, domains, lineage=None): """Authenticate and enroll certificate.""" # Note: This can raise errors... caught above us though. This is now # a three-way case: reinstall (which results in a no-op here because @@ -397,7 +398,16 @@ def _auth_from_domains(le_client, config, domains): # (which results in treating the request as a renewal), or newcert # (which results in treating the request as a new certificate request). - action, lineage = _treat_as_renewal(config, domains) + # If lineage is specified, use that one instead of looking around for + # a matching one. + if lineage is None: + # This will find a relevant matching lineage that exists + action, lineage = _treat_as_renewal(config, domains) + else: + # Renewal, where we already know the specific lineage we're + # interested in + action = "renew" if lineage.should_autorenew() else "reinstall" + if action == "reinstall": # The lineage already exists; allow the caller to try installing # it without getting a new certificate at all. @@ -674,13 +684,13 @@ def renew(args, cli_config, plugins): print("Welcome to the renew verb!") cli_config = configuration.RenewerConfiguration(cli_config) configs_dir = cli_config.renewal_configs_dir - for renewal_file in os.listdir(configs_dir): + for renewal_file in reversed(os.listdir(configs_dir)): if not renewal_file.endswith(".conf"): continue print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? - config = configuration.RenewerConfiguration(cli_config) + config = configuration.RenewerConfiguration(copy.deepcopy(cli_config)) full_path = os.path.join(configs_dir, renewal_file) try: renewal_candidate = storage.RenewableCert(full_path, config) @@ -704,7 +714,8 @@ def renew(args, cli_config, plugins): # XXX: also need: nginx_, apache_, and plesk_ items # string-valued items to add if they're present for config_item in ["config_dir", "log_dir", "work_dir", "user_agent", - "server", "standalone_supported_challenges"]: + "server", "account", + "standalone_supported_challenges"]: if config_item in renewalparams: value = renewalparams[config_item] # Unfortunately, we've lost type information from ConfigObj, @@ -724,7 +735,7 @@ def renew(args, cli_config, plugins): "a non-numeric value for %s. Skipping.", full_path, config_item) continue - # XXX: what does this do? + # XXX: ensure that each call here replaces the previous one zope.component.provideUtility(config) try: authenticator = plugins[renewalparams["authenticator"]] @@ -739,7 +750,11 @@ def renew(args, cli_config, plugins): "authenticator plugin. Skipping.", full_path) continue authenticator = authenticator.init(config) - le_client = _init_le_client(args, config, authenticator, authenticator) + print(config) + le_client = _init_le_client(config, config, authenticator, authenticator) + print("Trying...") + print(_auth_from_domains(le_client, config, renewal_candidate.names(), + renewal_candidate)) # TODO: How do we handle the separate installer vs. authenticator # the same as installer issue? import code; code.interact(local=locals()) From 7a7cd3d4f7015e13f46bf2c5b69f06a486bfd3bc Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 1 Feb 2016 20:35:43 -0800 Subject: [PATCH 06/73] Work in progress (renewal succeeded) --- letsencrypt/cli.py | 59 ++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ccb295e07..b91f6df28 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -626,7 +626,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo _suggest_donate() -def obtain_cert(args, config, plugins): +def obtain_cert(args, config, plugins, lineage=None): """Implements "certonly": authenticate & obtain cert, but do not install it.""" if args.domains and args.csr is not None: @@ -645,6 +645,7 @@ def obtain_cert(args, config, plugins): # This is a special case; cert and chain are simply saved if args.csr is not None: + assert lineage is None, "Did not expect a CSR with a RenewableCert" certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR( file=args.csr[0], data=args.csr[1], form="der")) cert_path, _, cert_fullchain = le_client.save_certificate( @@ -652,7 +653,7 @@ def obtain_cert(args, config, plugins): _report_new_cert(cert_path, cert_fullchain) else: domains = _find_domains(config, installer) - _auth_from_domains(le_client, config, domains) + _auth_from_domains(le_client, config, domains, lineage) _suggest_donate() @@ -681,7 +682,6 @@ def install(args, config, plugins): def renew(args, cli_config, plugins): """Renew previously-obtained certificates.""" - print("Welcome to the renew verb!") cli_config = configuration.RenewerConfiguration(cli_config) configs_dir = cli_config.renewal_configs_dir for renewal_file in reversed(os.listdir(configs_dir)): @@ -699,7 +699,6 @@ def renew(args, cli_config, plugins): "Skipping.", full_path) continue print(renewal_candidate.names(), renewal_candidate.should_autorenew()) - print("We should make a decision about whether to renew...!") if "renewalparams" not in renewal_candidate.configuration: logger.warning("Renewal configuration file %s lacks " "renewalparams. Skipping.", full_path) @@ -714,7 +713,7 @@ def renew(args, cli_config, plugins): # XXX: also need: nginx_, apache_, and plesk_ items # string-valued items to add if they're present for config_item in ["config_dir", "log_dir", "work_dir", "user_agent", - "server", "account", + "server", "account", "authenticator", "installer", "standalone_supported_challenges"]: if config_item in renewalparams: value = renewalparams[config_item] @@ -722,7 +721,6 @@ def renew(args, cli_config, plugins): # so we don't know if the original was NoneType or str! if value == "None": value = None - print("setting", config_item, value) config.__setattr__(config_item, value) # int-valued items to add if they're present for config_item in ["rsa_key_size", "tls_sni_01_port", "http01_port"]: @@ -737,27 +735,38 @@ def renew(args, cli_config, plugins): continue # XXX: ensure that each call here replaces the previous one zope.component.provideUtility(config) - try: - authenticator = plugins[renewalparams["authenticator"]] - except KeyError: - if "authenticator" in renewal_params: - logger.warning("Renewal configuration file %s specifies an " - "authenticator plugin (%s) that could not be " - "found. Skipping.", full_path, - renewal_params["authenticator"]) - else: - logger.warning("Renewal configuration file %s specifies no " - "authenticator plugin. Skipping.", full_path) - continue - authenticator = authenticator.init(config) + # try: + # authenticator = plugins[renewalparams["authenticator"]] + # if "installer" in renewalparams and renewalparams["installer"] != "None": + # installer = plugins[renewalparams["installer"]] + # except KeyError: + # if "authenticator" in renewal_params: + # logger.warning("Renewal configuration file %s specifies an " + # "authenticator plugin (%s) that could not be " + # "found. Skipping.", full_path, + # renewal_params["authenticator"]) + # else: + # logger.warning("Renewal configuration file %s specifies no " + # "authenticator plugin. Skipping.", full_path) + # continue + #authenticator = authenticator.init(config) + #installer = installer.init(config) print(config) - le_client = _init_le_client(config, config, authenticator, authenticator) + #le_client = _init_le_client(config, config, authenticator, installer) + try: + domains = [le_util.enforce_domain_sanity(x) for x in + renewal_candidate.names()] + except UnicodeError, ValueError: + logger.warning("Renewal configuration file %s references a cert " + "that mentions a domain name that we regarded as " + "invalid. Skipping.", full_path) + continue + + config.__setattr__("domains", domains) + print("Trying...") - print(_auth_from_domains(le_client, config, renewal_candidate.names(), - renewal_candidate)) - # TODO: How do we handle the separate installer vs. authenticator - # the same as installer issue? - import code; code.interact(local=locals()) + print(obtain_cert(config, config, plugins, renewal_candidate)) + def revoke(args, config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" From 273a78a5c63cdb7337c218aeb5fdd8173387b91b Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 15:02:15 -0800 Subject: [PATCH 07/73] use webroot_map if present (it's already a dict when deserialized!) --- letsencrypt/cli.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ef939e426..272f97f83 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -734,7 +734,10 @@ def renew(args, cli_config, plugins): "an authenticator. Skipping.", full_path) continue # ?? config = configuration.NamespaceConfig(_AttrDict(renewalparams)) - # XXX: also need: webroot_map + if "webroot_map" in renewalparams: + config.__setattr__("webroot_map", renewalparams["webroot_map"]) + print ("webroot_map", renewalparams["webroot_map"]) + raw_input() # XXX: also need: nginx_, apache_, and plesk_ items # string-valued items to add if they're present for config_item in ["config_dir", "log_dir", "work_dir", "user_agent", From 2c200b1e4316fd0d1dab7a175eb70b03c34a5b77 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 2 Feb 2016 15:05:08 -0800 Subject: [PATCH 08/73] Stray merge fix --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 2717581a7..e4a72682d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -707,7 +707,7 @@ def install(config, plugins): le_client.enhance_config(domains, config) -def renew(args, cli_config, plugins): +def renew(cli_config, plugins): """Renew previously-obtained certificates.""" cli_config = configuration.RenewerConfiguration(cli_config) configs_dir = cli_config.renewal_configs_dir From 3ea1c499f43f79d66ebd5473793889be3bd633d4 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 15:20:49 -0800 Subject: [PATCH 09/73] Cleanup after removal of "args" --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index b40583b46..788209c54 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -795,7 +795,7 @@ def renew(cli_config, plugins): config.__setattr__("domains", domains) print("Trying...") - print(obtain_cert(config, config, plugins, renewal_candidate)) + print(obtain_cert(config, plugins, renewal_candidate)) def revoke(config, unused_plugins): # TODO: coop with renewal config From 30bdbbfb823777cd5c8f3e51e544add54a7192f6 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 15:49:01 -0800 Subject: [PATCH 10/73] It's an error to use -d with renew now --- letsencrypt/cli.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 788209c54..12430ae57 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -710,6 +710,14 @@ def install(config, plugins): def renew(cli_config, plugins): """Renew previously-obtained certificates.""" cli_config = configuration.RenewerConfiguration(cli_config) + if cli_config.domains != []: + raise errors.Error("Currently, the renew verb is only capable of " + "renewing all installed certificates that are due " + "to be renewed; individual domains cannot be " + "specified with this action. If you would like to " + "renew specific certificates, use the certonly " + "command. The renew verb may provide other options " + "for selecting certificates to renew in the future.") configs_dir = cli_config.renewal_configs_dir for renewal_file in reversed(os.listdir(configs_dir)): if not renewal_file.endswith(".conf"): From 9e36c5b36d9e12d63359b1eac4120fbc5cea2765 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 16:03:48 -0800 Subject: [PATCH 11/73] Default deploy_before_expiry is now 99 years --- letsencrypt/constants.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index a1dccd1ea..402f5e9a1 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -37,7 +37,9 @@ STAGING_URI = "https://acme-staging.api.letsencrypt.org/directory" RENEWER_DEFAULTS = dict( renewer_enabled="yes", renew_before_expiry="30 days", - deploy_before_expiry="20 days", + # This value should ensure that there is never a deployment delay by + # default. + deploy_before_expiry="99 years", ) """Defaults for renewer script.""" From 9fde7fe476052aef75bd1ce104bca241f1b2cc68 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 2 Feb 2016 16:52:12 -0800 Subject: [PATCH 12/73] don't ask for donations if renewing --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 12430ae57..6c209f05f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -383,7 +383,7 @@ def _report_new_cert(cert_path, fullchain_path): def _suggest_donation_if_appropriate(config): """Potentially suggest a donation to support Let's Encrypt.""" - if not config.staging: # --dry-run implies --staging + if not config.staging or config.verb == "renew": # --dry-run implies --staging reporter_util = zope.component.getUtility(interfaces.IReporter) msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" From b97ddb92f06f5a8c1dc4fa321466ff38862ca138 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 2 Feb 2016 17:27:40 -0800 Subject: [PATCH 13/73] fix bool --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 6c209f05f..1d1fb03ea 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -383,7 +383,7 @@ def _report_new_cert(cert_path, fullchain_path): def _suggest_donation_if_appropriate(config): """Potentially suggest a donation to support Let's Encrypt.""" - if not config.staging or config.verb == "renew": # --dry-run implies --staging + if not config.staging and not config.verb == "renew": # --dry-run implies --staging reporter_util = zope.component.getUtility(interfaces.IReporter) msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" From 06b6dcc9c8c483071615c40ef1bed742b3f988cf Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 2 Feb 2016 17:35:46 -0800 Subject: [PATCH 14/73] set noninteractive to true if we're doing renew --- letsencrypt/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1d1fb03ea..d52393b62 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -726,6 +726,7 @@ def renew(cli_config, plugins): # XXX: does this succeed in making a fully independent config object # each time? config = configuration.RenewerConfiguration(copy.deepcopy(cli_config)) + config.noninteractive_mode = True full_path = os.path.join(configs_dir, renewal_file) try: renewal_candidate = storage.RenewableCert(full_path, config) From 6ae0852071ce71537341fb1a78298818e17800af Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 2 Feb 2016 17:41:48 -0800 Subject: [PATCH 15/73] Refactor: decide whether to renew or not in a single function --- letsencrypt/cli.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e4a72682d..001ceb842 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -237,7 +237,8 @@ def _find_duplicative_certs(config, domains): def _treat_as_renewal(config, domains): - """Determine whether there are duplicated names and how to handle them. + """Determine whether there are duplicated names and how to handle them + (renew, reinstall, newcert, or no action). :returns: Two-element tuple containing desired new-certificate behavior as a string token ("reinstall", "renew", or "newcert"), plus either @@ -264,6 +265,21 @@ def _treat_as_renewal(config, domains): elif subset_names_cert is not None: return _handle_subset_cert_request(config, domains, subset_names_cert) + +def _should_renew(config, lineage): + "Return true if any of the circumstances for automatic renewal apply." + if config.renew_by_default: + logger.info("Auto-renewal forced with --renew-by-default...") + return True + if cert.should_autorenew(interactive=True): + logger.info("Cert is due for renewal, auto-renewing...") + return True + if config.dry_run: + logger.info("Cert not due for renewal, but simulating renewal for dry run") + return True + return False + + def _handle_identical_cert_request(config, cert): """Figure out what to do if a cert has the same names as a previously obtained one @@ -273,17 +289,12 @@ def _handle_identical_cert_request(config, cert): :rtype: tuple """ - if config.renew_by_default: - logger.info("Auto-renewal forced with --renew-by-default...") - return "renew", cert - if cert.should_autorenew(interactive=True): - logger.info("Cert is due for renewal, auto-renewing...") + if _should_renew(config, cert): return "renew", cert if config.reinstall: # Set with --reinstall, force an identical certificate to be # reinstalled without further prompting. return "reinstall", cert - question = ( "You have an existing certificate that contains exactly the same " "domains you requested and isn't close to expiry." @@ -414,12 +425,7 @@ def _auth_from_domains(le_client, config, domains, lineage=None): else: # Renewal, where we already know the specific lineage we're # interested in - action = "renew" if lineage.should_autorenew() else "reinstall" - - if config.dry_run and action == "reinstall": - logger.info( - "Cert not due for renewal, but simulating renewal for dry run") - action = "renew" + action = "renew" if _should_renew(config, lineage) else "reinstall" if action == "reinstall": # The lineage already exists; allow the caller to try installing From 674d71d4e9ec08db176d53259e454d289df77618 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 17:45:55 -0800 Subject: [PATCH 16/73] Separate constants for what to pull from renewal config --- letsencrypt/cli.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 588952bc8..f44892da8 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -45,6 +45,15 @@ from letsencrypt.plugins import disco as plugins_disco logger = logging.getLogger(__name__) +# These are the items which get pulled out of a renewal configuration +# file's renewalparams and actually used in the client configuration +# during the renewal process. We have to record their types here because +# the renewal configuration process loses this information. +STR_CONFIG_ITEMS = ["config_dir", "log_dir", "work_dir", "user_agent", + "server", "account", "authenticator", "installer", + "standalone_supported_challenges"] +INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] + # For help strings, figure out how the user ran us. # When invoked from letsencrypt-auto, sys.argv[0] is something like: # "/home/user/.local/share/letsencrypt/bin/letsencrypt" @@ -751,15 +760,14 @@ def renew(cli_config, plugins): "an authenticator. Skipping.", full_path) continue # ?? config = configuration.NamespaceConfig(_AttrDict(renewalparams)) + # webroot_map is, uniquely, a dict if "webroot_map" in renewalparams: config.__setattr__("webroot_map", renewalparams["webroot_map"]) print ("webroot_map", renewalparams["webroot_map"]) raw_input() # XXX: also need: nginx_, apache_, and plesk_ items # string-valued items to add if they're present - for config_item in ["config_dir", "log_dir", "work_dir", "user_agent", - "server", "account", "authenticator", "installer", - "standalone_supported_challenges"]: + for config_item in STR_CONFIG_ITEMS: if config_item in renewalparams: value = renewalparams[config_item] # Unfortunately, we've lost type information from ConfigObj, @@ -768,7 +776,7 @@ def renew(cli_config, plugins): value = None config.__setattr__(config_item, value) # int-valued items to add if they're present - for config_item in ["rsa_key_size", "tls_sni_01_port", "http01_port"]: + for config_item in INT_CONFIG_ITEMS: if config_item in renewalparams: try: value = int(renewalparams[config_item]) @@ -1286,6 +1294,7 @@ def prepare_and_parse_args(plugins, args): # parser (--help should display plugin-specific options last) _plugins_parsing(helpful, plugins) + import code; code.interact(local=locals()) return helpful.parse_args() From 0a2b5376295d9d15d7b5e73e039e9a7a9e76dc0e Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 17:49:42 -0800 Subject: [PATCH 17/73] Fix mistaken parameter reference in _should_renew --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f44892da8..76883bc60 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -280,7 +280,7 @@ def _should_renew(config, lineage): if config.renew_by_default: logger.info("Auto-renewal forced with --renew-by-default...") return True - if cert.should_autorenew(interactive=True): + if lineage.should_autorenew(interactive=True): logger.info("Cert is due for renewal, auto-renewing...") return True if config.dry_run: From 3697ca7e3e744d7eda8bdbb122f78f6cec465a44 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 2 Feb 2016 17:56:12 -0800 Subject: [PATCH 18/73] throw an error if manual is run non-interactively --- letsencrypt/plugins/manual.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 7f782a41b..29f4639fe 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -91,6 +91,8 @@ s.serve_forever()" """ help="Automatically allows public IP logging.") def prepare(self): # pylint: disable=missing-docstring,no-self-use + if self.config.noninteractive_mode: + raise errors.PluginError("Running manual mode non-interactively is not supported") pass # pragma: no cover def more_info(self): # pylint: disable=missing-docstring,no-self-use From 6682e370a8a0041f9bb4274117c7ec5f4ccc1399 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 18:28:04 -0800 Subject: [PATCH 19/73] Very ugly approach to extract types from the parser! --- letsencrypt/cli.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 76883bc60..c9eaa1346 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -45,6 +45,10 @@ from letsencrypt.plugins import disco as plugins_disco logger = logging.getLogger(__name__) +# This is global scope in order to be able to extract type information from +# it later +_parser = None + # These are the items which get pulled out of a renewal configuration # file's renewalparams and actually used in the client configuration # during the renewal process. We have to record their types here because @@ -54,6 +58,10 @@ STR_CONFIG_ITEMS = ["config_dir", "log_dir", "work_dir", "user_agent", "standalone_supported_challenges"] INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] +# These are the plugins for which we should try to automatically extract +# the types when pulling items from a renewal configuration. +EXTRACT_PLUGIN_PREFIXES = ["apache_", "nginx_", "standalone_"] + # For help strings, figure out how the user ran us. # When invoked from letsencrypt-auto, sys.argv[0] is something like: # "/home/user/.local/share/letsencrypt/bin/letsencrypt" @@ -723,6 +731,8 @@ def install(config, plugins): def renew(cli_config, plugins): + print ("Beginning renew") + import code; code.interact(local=locals()) """Renew previously-obtained certificates.""" cli_config = configuration.RenewerConfiguration(cli_config) if cli_config.domains != []: @@ -743,6 +753,8 @@ def renew(cli_config, plugins): config = configuration.RenewerConfiguration(copy.deepcopy(cli_config)) config.noninteractive_mode = True full_path = os.path.join(configs_dir, renewal_file) + + try: renewal_candidate = storage.RenewableCert(full_path, config) except (errors.CertStorageError, IOError): @@ -786,6 +798,19 @@ def renew(cli_config, plugins): "a non-numeric value for %s. Skipping.", full_path, config_item) continue + # Now use parser to get plugin-prefixed items with correct types + # XXX: is it true that an item will end up in _parser._actions even + # when no action was explicitly specified? + for plugin_prefix in EXTRACT_PLUGIN_PREFIXES: + for config_item in renewalparams.keys(): + if config_item.startswith(plugin_prefix): + for action in _parser.parser._actions: + if action.dest == config_item: + if action.type is not None: + config.__setattr__(config_item, action.type(renewalparams[config_item])) + break + else: + config.__setattr__(config_item, str(renewalparams[config_item])) # XXX: ensure that each call here replaces the previous one zope.component.provideUtility(config) # try: @@ -1294,7 +1319,9 @@ def prepare_and_parse_args(plugins, args): # parser (--help should display plugin-specific options last) _plugins_parsing(helpful, plugins) - import code; code.interact(local=locals()) + global _parser + _parser = helpful + print("stored _parser") return helpful.parse_args() From b69a1020872bfa8866ab52d34d025bea4577d113 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 18:30:22 -0800 Subject: [PATCH 20/73] Removing debug prints --- letsencrypt/cli.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c9eaa1346..02ccfa5f3 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -731,8 +731,6 @@ def install(config, plugins): def renew(cli_config, plugins): - print ("Beginning renew") - import code; code.interact(local=locals()) """Renew previously-obtained certificates.""" cli_config = configuration.RenewerConfiguration(cli_config) if cli_config.domains != []: @@ -761,7 +759,6 @@ def renew(cli_config, plugins): logger.warning("Renewal configuration file %s is broken. " "Skipping.", full_path) continue - print(renewal_candidate.names(), renewal_candidate.should_autorenew()) if "renewalparams" not in renewal_candidate.configuration: logger.warning("Renewal configuration file %s lacks " "renewalparams. Skipping.", full_path) @@ -775,8 +772,6 @@ def renew(cli_config, plugins): # webroot_map is, uniquely, a dict if "webroot_map" in renewalparams: config.__setattr__("webroot_map", renewalparams["webroot_map"]) - print ("webroot_map", renewalparams["webroot_map"]) - raw_input() # XXX: also need: nginx_, apache_, and plesk_ items # string-valued items to add if they're present for config_item in STR_CONFIG_ITEMS: @@ -829,7 +824,6 @@ def renew(cli_config, plugins): # continue #authenticator = authenticator.init(config) #installer = installer.init(config) - print(config) #le_client = _init_le_client(config, config, authenticator, installer) try: domains = [le_util.enforce_domain_sanity(x) for x in From 05c07ad90cfc51c07571b42ee2bea20e7ada6377 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 11:08:49 -0800 Subject: [PATCH 21/73] Reduce spaminess --- letsencrypt/configuration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 1ed7af2db..72aabe548 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -97,7 +97,6 @@ class RenewerConfiguration(object): return getattr(self.namespace, name) def __setattr_implementation__(self, var, value): - print("in __setattr_implementation__, setting", var, value) return self.namespace.__setattr__(var, value) @property From ec7e957fe6e94f82a703ee746bbf582747ddc574 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 11:52:57 -0800 Subject: [PATCH 22/73] Minimal test unbreakage Though really this test will need to be redesigned :( --- letsencrypt/cli.py | 1 - letsencrypt/tests/cli_test.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 02ccfa5f3..1d4764835 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1315,7 +1315,6 @@ def prepare_and_parse_args(plugins, args): global _parser _parser = helpful - print("stored _parser") return helpful.parse_args() diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index f0ac954f9..7f20d65df 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -567,7 +567,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue('donate' in get_utility().add_message.call_args[0][0]) def test_certonly_dry_run_reinstall_is_renewal(self): - _, get_utility = self._test_certonly_renewal_common('reinstall', + _, get_utility = self._test_certonly_renewal_common('renew', ['--dry-run']) self.assertEqual(get_utility().add_message.call_count, 1) self.assertTrue('dry run' in get_utility().add_message.call_args[0][0]) From bfd182ae39fac4b11072867acd94ff89f7b44128 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 12:14:49 -0800 Subject: [PATCH 23/73] Fix _test_certonly_csr_common (more) properly --- letsencrypt/tests/cli_test.py | 40 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 7f20d65df..6b2e6f9f1 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -530,35 +530,35 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, self._certonly_new_request_common, mock_client) - def _test_certonly_renewal_common(self, renewal_verb, extra_args=None): + @mock.patch('letsencrypt.cli._find_duplicative_certs') + def _test_certonly_renewal_common(self, extra_args, mock_fdc): cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) mock_certr = mock.MagicMock() mock_key = mock.MagicMock(pem='pem_key') - with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal: - mock_renewal.return_value = (renewal_verb, mock_lineage) - mock_client = mock.MagicMock() - mock_client.obtain_certificate.return_value = (mock_certr, 'chain', - mock_key, 'csr') - with mock.patch('letsencrypt.cli._init_le_client') as mock_init: - mock_init.return_value = mock_client - get_utility_path = 'letsencrypt.cli.zope.component.getUtility' - with mock.patch(get_utility_path) as mock_get_utility: - with mock.patch('letsencrypt.cli.OpenSSL'): - with mock.patch('letsencrypt.cli.crypto_util'): - args = ['-d', 'foo.bar', '-a', - 'standalone', 'certonly'] - if extra_args: - args += extra_args - self._call(args) + mock_fdc.return_value = (mock_lineage, None) + mock_client = mock.MagicMock() + mock_client.obtain_certificate.return_value = (mock_certr, 'chain', + mock_key, 'csr') + with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + mock_init.return_value = mock_client + get_utility_path = 'letsencrypt.cli.zope.component.getUtility' + with mock.patch(get_utility_path) as mock_get_utility: + with mock.patch('letsencrypt.cli.OpenSSL'): + with mock.patch('letsencrypt.cli.crypto_util'): + args = ['-d', 'foo.bar', '-a', + 'standalone', 'certonly'] + if extra_args: + args += extra_args + self._call(args) mock_client.obtain_certificate.assert_called_once_with(['foo.bar']) return mock_lineage, mock_get_utility def test_certonly_renewal(self): - lineage, get_utility = self._test_certonly_renewal_common('renew') + lineage, get_utility = self._test_certonly_renewal_common([]) self.assertEqual(lineage.save_successor.call_count, 1) lineage.update_all_links_to.assert_called_once_with( lineage.latest_common_version()) @@ -567,11 +567,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue('donate' in get_utility().add_message.call_args[0][0]) def test_certonly_dry_run_reinstall_is_renewal(self): - _, get_utility = self._test_certonly_renewal_common('renew', - ['--dry-run']) + _, get_utility = self._test_certonly_renewal_common(['--dry-run']) self.assertEqual(get_utility().add_message.call_count, 1) self.assertTrue('dry run' in get_utility().add_message.call_args[0][0]) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 37709f2e078dc0e7ed793993ecbb30ca84d2451c Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 3 Feb 2016 12:15:51 -0800 Subject: [PATCH 24/73] Remove commented-out lines --- letsencrypt/cli.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1d4764835..0fe0bd805 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -768,7 +768,6 @@ def renew(cli_config, plugins): logger.warning("Renewal configuration file %s does not specify " "an authenticator. Skipping.", full_path) continue - # ?? config = configuration.NamespaceConfig(_AttrDict(renewalparams)) # webroot_map is, uniquely, a dict if "webroot_map" in renewalparams: config.__setattr__("webroot_map", renewalparams["webroot_map"]) @@ -808,23 +807,6 @@ def renew(cli_config, plugins): config.__setattr__(config_item, str(renewalparams[config_item])) # XXX: ensure that each call here replaces the previous one zope.component.provideUtility(config) - # try: - # authenticator = plugins[renewalparams["authenticator"]] - # if "installer" in renewalparams and renewalparams["installer"] != "None": - # installer = plugins[renewalparams["installer"]] - # except KeyError: - # if "authenticator" in renewal_params: - # logger.warning("Renewal configuration file %s specifies an " - # "authenticator plugin (%s) that could not be " - # "found. Skipping.", full_path, - # renewal_params["authenticator"]) - # else: - # logger.warning("Renewal configuration file %s specifies no " - # "authenticator plugin. Skipping.", full_path) - # continue - #authenticator = authenticator.init(config) - #installer = installer.init(config) - #le_client = _init_le_client(config, config, authenticator, installer) try: domains = [le_util.enforce_domain_sanity(x) for x in renewal_candidate.names()] From 5642edfbffc23871d23f083bf4f75308846272c1 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 3 Feb 2016 12:54:56 -0800 Subject: [PATCH 25/73] Clobber EXTRACT_PLUGIN_PREFIXES; fix several bugs --- letsencrypt/cli.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 0fe0bd805..6278859db 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -58,10 +58,6 @@ STR_CONFIG_ITEMS = ["config_dir", "log_dir", "work_dir", "user_agent", "standalone_supported_challenges"] INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] -# These are the plugins for which we should try to automatically extract -# the types when pulling items from a renewal configuration. -EXTRACT_PLUGIN_PREFIXES = ["apache_", "nginx_", "standalone_"] - # For help strings, figure out how the user ran us. # When invoked from letsencrypt-auto, sys.argv[0] is something like: # "/home/user/.local/share/letsencrypt/bin/letsencrypt" @@ -768,9 +764,6 @@ def renew(cli_config, plugins): logger.warning("Renewal configuration file %s does not specify " "an authenticator. Skipping.", full_path) continue - # webroot_map is, uniquely, a dict - if "webroot_map" in renewalparams: - config.__setattr__("webroot_map", renewalparams["webroot_map"]) # XXX: also need: nginx_, apache_, and plesk_ items # string-valued items to add if they're present for config_item in STR_CONFIG_ITEMS: @@ -793,11 +786,23 @@ def renew(cli_config, plugins): full_path, config_item) continue # Now use parser to get plugin-prefixed items with correct types + # XXX: the current approach of extracting only prefixed items + # related to the actually-used installer and authenticator + # works as long as plugins don't need to read plugin-specific + # variables set by someone else (e.g., assuming Apache + # configurator doesn't need to read webroot_ variables). # XXX: is it true that an item will end up in _parser._actions even # when no action was explicitly specified? - for plugin_prefix in EXTRACT_PLUGIN_PREFIXES: + plugin_prefixes = [renewalparams["authenticator"]] + if "installer" in renewalparams and renewalparams["installer"] != None: + plugin_prefixes.append(renewalparams["installer"]) + for plugin_prefix in set(renewalparams): for config_item in renewalparams.keys(): - if config_item.startswith(plugin_prefix): + if renewalparams[config_item] == "None": + # Avoid confusion when, for example, csr = None (avoid + # trying to read the file called "None") + continue + if config_item.startswith(plugin_prefix + "_"): for action in _parser.parser._actions: if action.dest == config_item: if action.type is not None: @@ -805,6 +810,10 @@ def renew(cli_config, plugins): break else: config.__setattr__(config_item, str(renewalparams[config_item])) + # webroot_map is, uniquely, a dict, and the logic above is not able + # to correctly parse it from the serialized form. + if "webroot_map" in renewalparams: + config.__setattr__("webroot_map", renewalparams["webroot_map"]) # XXX: ensure that each call here replaces the previous one zope.component.provideUtility(config) try: @@ -819,7 +828,11 @@ def renew(cli_config, plugins): config.__setattr__("domains", domains) print("Trying...") - print(obtain_cert(config, plugins, renewal_candidate)) + # Because obtain_cert itself indirectly decides whether to renew + # or not, we couldn't currently make a UI/logging distinction at + # this stage to indicate whether renewal was actually attempted + # (or successful). + obtain_cert(config, plugins, renewal_candidate) def revoke(config, unused_plugins): # TODO: coop with renewal config From c0d55c7c33a032b105887fa46e96ce3e935191a3 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 12:57:49 -0800 Subject: [PATCH 26/73] Improve certonly renewal test coverage --- letsencrypt/cli.py | 3 +++ letsencrypt/tests/cli_test.py | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1d4764835..fabe33c38 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -287,12 +287,15 @@ def _should_renew(config, lineage): "Return true if any of the circumstances for automatic renewal apply." if config.renew_by_default: logger.info("Auto-renewal forced with --renew-by-default...") + print("forced") return True if lineage.should_autorenew(interactive=True): logger.info("Cert is due for renewal, auto-renewing...") + print("due") return True if config.dry_run: logger.info("Cert not due for renewal, but simulating renewal for dry run") + print("dry") return True return False diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 6b2e6f9f1..36d39590d 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -531,10 +531,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._certonly_new_request_common, mock_client) @mock.patch('letsencrypt.cli._find_duplicative_certs') - def _test_certonly_renewal_common(self, extra_args, mock_fdc): + def _test_renewal_common(self, due_for_renewal, extra_args, outstring, mock_fdc): cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) + mock_lineage.should_autorenew.return_value = due_for_renewal mock_certr = mock.MagicMock() mock_key = mock.MagicMock(pem='pem_key') mock_fdc.return_value = (mock_lineage, None) @@ -553,12 +554,16 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args += extra_args self._call(args) + if outstring: + with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: + self.assertTrue(outstring in lf.read()) + mock_client.obtain_certificate.assert_called_once_with(['foo.bar']) return mock_lineage, mock_get_utility def test_certonly_renewal(self): - lineage, get_utility = self._test_certonly_renewal_common([]) + lineage, get_utility = self._test_renewal_common(True, [], None) self.assertEqual(lineage.save_successor.call_count, 1) lineage.update_all_links_to.assert_called_once_with( lineage.latest_common_version()) @@ -566,11 +571,14 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue('fullchain.pem' in cert_msg) self.assertTrue('donate' in get_utility().add_message.call_args[0][0]) - def test_certonly_dry_run_reinstall_is_renewal(self): - _, get_utility = self._test_certonly_renewal_common(['--dry-run']) + def test_certonly_renewal_triggers(self): + # --dry-run should force renewal + _, get_utility = self._test_renewal_common(False, ['--dry-run'], None) self.assertEqual(get_utility().add_message.call_count, 1) self.assertTrue('dry run' in get_utility().add_message.call_args[0][0]) + _, _ = self._test_renewal_common(False, ['--renew-by-default', '-tvv', '--debug'], "Auto-renewal forced") + self.assertEqual(get_utility().add_message.call_count, 1) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From af39b52122f69c414b92f53af06fb27011cd804b Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 3 Feb 2016 13:20:55 -0800 Subject: [PATCH 27/73] Split out _reconstitute() from renew() --- letsencrypt/cli.py | 183 ++++++++++++++++++++++++++------------------- 1 file changed, 107 insertions(+), 76 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 6278859db..5a466dc25 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -726,6 +726,101 @@ def install(config, plugins): le_client.enhance_config(domains, config) +def _reconstitute(full_path, config): + """Try to instantiate a RenewableCert, updating config with relevant items. + + This is specifically for use in renewal and enforces several checks + and policies to ensure that we can try to proceed with the renwal + request. The config argument is modified by including relevant options + read from the renewal configuration file. + + :returns: the RenewableCert object or None if a fatal error occurred + :rtype: `storage.RenewableCert` or NoneType + """ + + try: + renewal_candidate = storage.RenewableCert(full_path, config) + except (errors.CertStorageError, IOError): + logger.warning("Renewal configuration file %s is broken. " + "Skipping.", full_path) + return None + if "renewalparams" not in renewal_candidate.configuration: + logger.warning("Renewal configuration file %s lacks " + "renewalparams. Skipping.", full_path) + return None + renewalparams = renewal_candidate.configuration["renewalparams"] + if "authenticator" not in renewalparams: + logger.warning("Renewal configuration file %s does not specify " + "an authenticator. Skipping.", full_path) + return None + # string-valued items to add if they're present + for config_item in STR_CONFIG_ITEMS: + if config_item in renewalparams: + value = renewalparams[config_item] + # Unfortunately, we've lost type information from ConfigObj, + # so we don't know if the original was NoneType or str! + if value == "None": + value = None + config.__setattr__(config_item, value) + # int-valued items to add if they're present + for config_item in INT_CONFIG_ITEMS: + if config_item in renewalparams: + try: + value = int(renewalparams[config_item]) + config.__setattr__(config_item, value) + except ValueError: + logger.warning("Renewal configuration file %s specifies " + "a non-numeric value for %s. Skipping.", + full_path, config_item) + return None + # Now use parser to get plugin-prefixed items with correct types + # XXX: the current approach of extracting only prefixed items + # related to the actually-used installer and authenticator + # works as long as plugins don't need to read plugin-specific + # variables set by someone else (e.g., assuming Apache + # configurator doesn't need to read webroot_ variables). + # XXX: is it true that an item will end up in _parser._actions even + # when no action was explicitly specified? + plugin_prefixes = [renewalparams["authenticator"]] + if "installer" in renewalparams and renewalparams["installer"] != None: + plugin_prefixes.append(renewalparams["installer"]) + for plugin_prefix in set(renewalparams): + for config_item in renewalparams.keys(): + if renewalparams[config_item] == "None": + # Avoid confusion when, for example, "csr = None" (avoid + # trying to read the file called "None") + # Should we omit the item entirely rather than setting + # its value to None? + config.__setattr__(config_item, None) + continue + if config_item.startswith(plugin_prefix + "_"): + for action in _parser.parser._actions: + if action.dest == config_item: + if action.type is not None: + config.__setattr__(config_item, action.type(renewalparams[config_item])) + break + else: + config.__setattr__(config_item, str(renewalparams[config_item])) + # webroot_map is, uniquely, a dict, and the logic above is not able + # to correctly parse it from the serialized form. + if "webroot_map" in renewalparams: + config.__setattr__("webroot_map", renewalparams["webroot_map"]) + + try: + domains = [le_util.enforce_domain_sanity(x) for x in + renewal_candidate.names()] + except UnicodeError, ValueError: + logger.warning("Renewal configuration file %s references a cert " + "that mentions a domain name that we regarded as " + "invalid. Skipping.", full_path) + return None + + config.__setattr__("domains", domains) + # XXX: ensure that each call here replaces the previous one + zope.component.provideUtility(config) + return renewal_candidate + + def renew(cli_config, plugins): """Renew previously-obtained certificates.""" cli_config = configuration.RenewerConfiguration(cli_config) @@ -738,7 +833,7 @@ def renew(cli_config, plugins): "command. The renew verb may provide other options " "for selecting certificates to renew in the future.") configs_dir = cli_config.renewal_configs_dir - for renewal_file in reversed(os.listdir(configs_dir)): + for renewal_file in os.listdir(configs_dir): if not renewal_file.endswith(".conf"): continue print("Processing " + renewal_file) @@ -748,84 +843,20 @@ def renew(cli_config, plugins): config.noninteractive_mode = True full_path = os.path.join(configs_dir, renewal_file) - + # Note that this modifies config (to add back the configuration + # elements from within the renewal configuration file). try: - renewal_candidate = storage.RenewableCert(full_path, config) - except (errors.CertStorageError, IOError): - logger.warning("Renewal configuration file %s is broken. " - "Skipping.", full_path) - continue - if "renewalparams" not in renewal_candidate.configuration: - logger.warning("Renewal configuration file %s lacks " - "renewalparams. Skipping.", full_path) - continue - renewalparams = renewal_candidate.configuration["renewalparams"] - if "authenticator" not in renewalparams: - logger.warning("Renewal configuration file %s does not specify " - "an authenticator. Skipping.", full_path) - continue - # XXX: also need: nginx_, apache_, and plesk_ items - # string-valued items to add if they're present - for config_item in STR_CONFIG_ITEMS: - if config_item in renewalparams: - value = renewalparams[config_item] - # Unfortunately, we've lost type information from ConfigObj, - # so we don't know if the original was NoneType or str! - if value == "None": - value = None - config.__setattr__(config_item, value) - # int-valued items to add if they're present - for config_item in INT_CONFIG_ITEMS: - if config_item in renewalparams: - try: - value = int(renewalparams[config_item]) - config.__setattr__(config_item, value) - except ValueError: - logger.warning("Renewal configuration file %s specifies " - "a non-numeric value for %s. Skipping.", - full_path, config_item) - continue - # Now use parser to get plugin-prefixed items with correct types - # XXX: the current approach of extracting only prefixed items - # related to the actually-used installer and authenticator - # works as long as plugins don't need to read plugin-specific - # variables set by someone else (e.g., assuming Apache - # configurator doesn't need to read webroot_ variables). - # XXX: is it true that an item will end up in _parser._actions even - # when no action was explicitly specified? - plugin_prefixes = [renewalparams["authenticator"]] - if "installer" in renewalparams and renewalparams["installer"] != None: - plugin_prefixes.append(renewalparams["installer"]) - for plugin_prefix in set(renewalparams): - for config_item in renewalparams.keys(): - if renewalparams[config_item] == "None": - # Avoid confusion when, for example, csr = None (avoid - # trying to read the file called "None") - continue - if config_item.startswith(plugin_prefix + "_"): - for action in _parser.parser._actions: - if action.dest == config_item: - if action.type is not None: - config.__setattr__(config_item, action.type(renewalparams[config_item])) - break - else: - config.__setattr__(config_item, str(renewalparams[config_item])) - # webroot_map is, uniquely, a dict, and the logic above is not able - # to correctly parse it from the serialized form. - if "webroot_map" in renewalparams: - config.__setattr__("webroot_map", renewalparams["webroot_map"]) - # XXX: ensure that each call here replaces the previous one - zope.component.provideUtility(config) - try: - domains = [le_util.enforce_domain_sanity(x) for x in - renewal_candidate.names()] - except UnicodeError, ValueError: - logger.warning("Renewal configuration file %s references a cert " - "that mentions a domain name that we regarded as " - "invalid. Skipping.", full_path) + renewal_candidate = _reconstitute(full_path, config) + except Exception as e: + # reconstitute encountered an unanticipated problem. + logger.warning("Renewal configuration file %s produced an " + "unexpected error: %s. Skipping.", full_path, e) continue - config.__setattr__("domains", domains) + if renewal_candidate is None: + # reconstitute indicated an error or problem which has + # already been logged. Go on to the next config. + continue print("Trying...") # Because obtain_cert itself indirectly decides whether to renew From f7a350b0f8cac9d030e5842d9b8ca3e4b12f39fc Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 3 Feb 2016 13:21:43 -0800 Subject: [PATCH 28/73] Move zope call back inside renew() --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5a466dc25..faa27c88c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -816,8 +816,6 @@ def _reconstitute(full_path, config): return None config.__setattr__("domains", domains) - # XXX: ensure that each call here replaces the previous one - zope.component.provideUtility(config) return renewal_candidate @@ -857,6 +855,8 @@ def renew(cli_config, plugins): # reconstitute indicated an error or problem which has # already been logged. Go on to the next config. continue + # XXX: ensure that each call here replaces the previous one + zope.component.provideUtility(config) print("Trying...") # Because obtain_cert itself indirectly decides whether to renew From fd0fd1444d4e8ace752e3d2c86fa48f0b49427e8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 13:29:13 -0800 Subject: [PATCH 29/73] Thorough checking, less printfs --- letsencrypt/cli.py | 4 +--- letsencrypt/tests/cli_test.py | 6 ++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 11a9ea292..02195645e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -283,16 +283,14 @@ def _should_renew(config, lineage): "Return true if any of the circumstances for automatic renewal apply." if config.renew_by_default: logger.info("Auto-renewal forced with --renew-by-default...") - print("forced") return True if lineage.should_autorenew(interactive=True): logger.info("Cert is due for renewal, auto-renewing...") - print("due") return True if config.dry_run: logger.info("Cert not due for renewal, but simulating renewal for dry run") - print("dry") return True + logger.info("Cert not yet due for renewal") return False diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 36d39590d..42f6c3bdd 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -573,13 +573,15 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_certonly_renewal_triggers(self): # --dry-run should force renewal - _, get_utility = self._test_renewal_common(False, ['--dry-run'], None) + _, get_utility = self._test_renewal_common(False, ['--dry-run'], "simulating renewal") self.assertEqual(get_utility().add_message.call_count, 1) self.assertTrue('dry run' in get_utility().add_message.call_args[0][0]) - _, _ = self._test_renewal_common(False, ['--renew-by-default', '-tvv', '--debug'], "Auto-renewal forced") + _, _ = self._test_renewal_common(False, ['--renew-by-default', '-tvv', '--debug'], + "Auto-renewal forced") self.assertEqual(get_utility().add_message.call_count, 1) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 22adea60bfcb495c640cea5b41e5ded294ac8468 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 3 Feb 2016 13:49:26 -0800 Subject: [PATCH 30/73] _restore_required_config_elements + fix except syntax --- letsencrypt/cli.py | 76 +++++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 31 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 247c5fc51..4c30e2ca8 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -727,33 +727,7 @@ def install(config, plugins): le_client.enhance_config(domains, config) -def _reconstitute(full_path, config): - """Try to instantiate a RenewableCert, updating config with relevant items. - - This is specifically for use in renewal and enforces several checks - and policies to ensure that we can try to proceed with the renwal - request. The config argument is modified by including relevant options - read from the renewal configuration file. - - :returns: the RenewableCert object or None if a fatal error occurred - :rtype: `storage.RenewableCert` or NoneType - """ - - try: - renewal_candidate = storage.RenewableCert(full_path, config) - except (errors.CertStorageError, IOError): - logger.warning("Renewal configuration file %s is broken. " - "Skipping.", full_path) - return None - if "renewalparams" not in renewal_candidate.configuration: - logger.warning("Renewal configuration file %s lacks " - "renewalparams. Skipping.", full_path) - return None - renewalparams = renewal_candidate.configuration["renewalparams"] - if "authenticator" not in renewalparams: - logger.warning("Renewal configuration file %s does not specify " - "an authenticator. Skipping.", full_path) - return None +def _restore_required_config_elements(full_path, config, renewalparams): # string-valued items to add if they're present for config_item in STR_CONFIG_ITEMS: if config_item in renewalparams: @@ -773,7 +747,7 @@ def _reconstitute(full_path, config): logger.warning("Renewal configuration file %s specifies " "a non-numeric value for %s. Skipping.", full_path, config_item) - return None + raise # Now use parser to get plugin-prefixed items with correct types # XXX: the current approach of extracting only prefixed items # related to the actually-used installer and authenticator @@ -802,15 +776,55 @@ def _reconstitute(full_path, config): break else: config.__setattr__(config_item, str(renewalparams[config_item])) - # webroot_map is, uniquely, a dict, and the logic above is not able - # to correctly parse it from the serialized form. + return True + + +def _reconstitute(full_path, config): + """Try to instantiate a RenewableCert, updating config with relevant items. + + This is specifically for use in renewal and enforces several checks + and policies to ensure that we can try to proceed with the renwal + request. The config argument is modified by including relevant options + read from the renewal configuration file. + + :returns: the RenewableCert object or None if a fatal error occurred + :rtype: `storage.RenewableCert` or NoneType + """ + + try: + renewal_candidate = storage.RenewableCert(full_path, config) + except (errors.CertStorageError, IOError): + logger.warning("Renewal configuration file %s is broken. " + "Skipping.", full_path) + return None + if "renewalparams" not in renewal_candidate.configuration: + logger.warning("Renewal configuration file %s lacks " + "renewalparams. Skipping.", full_path) + return None + renewalparams = renewal_candidate.configuration["renewalparams"] + if "authenticator" not in renewalparams: + logger.warning("Renewal configuration file %s does not specify " + "an authenticator. Skipping.", full_path) + return None + # Now restore specific values along with their data types, if + # those elements are present. + try: + _restore_required_config_elements(full_path, config, renewalparams) + except ValueError: + # There was a data type error which has already been + # logged. + return None + + # webroot_map is, uniquely, a dict, and the general-purpose + # configuration restoring logic is not able to correctly parse it + # from the serialized form. if "webroot_map" in renewalparams: config.__setattr__("webroot_map", renewalparams["webroot_map"]) try: domains = [le_util.enforce_domain_sanity(x) for x in renewal_candidate.names()] - except UnicodeError, ValueError: + except (UnicodeError, ValueError): logger.warning("Renewal configuration file %s references a cert " "that mentions a domain name that we regarded as " "invalid. Skipping.", full_path) From 78fd28a4865709fa5409d99e794fb69e595297ae Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 13:51:01 -0800 Subject: [PATCH 31/73] coverage++ --- letsencrypt/tests/cli_test.py | 39 +++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 42f6c3bdd..f083018b3 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -530,35 +530,38 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, self._certonly_new_request_common, mock_client) - @mock.patch('letsencrypt.cli._find_duplicative_certs') - def _test_renewal_common(self, due_for_renewal, extra_args, outstring, mock_fdc): + def _test_renewal_common(self, due_for_renewal, extra_args, outstring, renew=True): cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) mock_lineage.should_autorenew.return_value = due_for_renewal mock_certr = mock.MagicMock() mock_key = mock.MagicMock(pem='pem_key') - mock_fdc.return_value = (mock_lineage, None) mock_client = mock.MagicMock() mock_client.obtain_certificate.return_value = (mock_certr, 'chain', mock_key, 'csr') - with mock.patch('letsencrypt.cli._init_le_client') as mock_init: - mock_init.return_value = mock_client - get_utility_path = 'letsencrypt.cli.zope.component.getUtility' - with mock.patch(get_utility_path) as mock_get_utility: - with mock.patch('letsencrypt.cli.OpenSSL'): - with mock.patch('letsencrypt.cli.crypto_util'): - args = ['-d', 'foo.bar', '-a', - 'standalone', 'certonly'] - if extra_args: - args += extra_args - self._call(args) + with mock.patch('letsencrypt.cli._find_duplicative_certs') as mock_fdc: + mock_fdc.return_value = (mock_lineage, None) + with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + mock_init.return_value = mock_client + get_utility_path = 'letsencrypt.cli.zope.component.getUtility' + with mock.patch(get_utility_path) as mock_get_utility: + with mock.patch('letsencrypt.cli.OpenSSL'): + with mock.patch('letsencrypt.cli.crypto_util'): + args = ['-d', 'foo.bar', '-a', + 'standalone', 'certonly'] + if extra_args: + args += extra_args + self._call(args) if outstring: with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: self.assertTrue(outstring in lf.read()) - mock_client.obtain_certificate.assert_called_once_with(['foo.bar']) + if renew: + mock_client.obtain_certificate.assert_called_once_with(['foo.bar']) + else: + self.assertEqual(mock_client.obtain_certificate.call_count, 0) return mock_lineage, mock_get_utility @@ -573,7 +576,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_certonly_renewal_triggers(self): # --dry-run should force renewal - _, get_utility = self._test_renewal_common(False, ['--dry-run'], "simulating renewal") + _, get_utility = self._test_renewal_common(False, ['--dry-run', '--keep'], + "simulating renewal") self.assertEqual(get_utility().add_message.call_count, 1) self.assertTrue('dry run' in get_utility().add_message.call_args[0][0]) @@ -581,6 +585,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods "Auto-renewal forced") self.assertEqual(get_utility().add_message.call_count, 1) + _, _ = self._test_renewal_common(False, ['-tvv', '--debug', '--keep'], + "not yet due", renew=False) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From a706f5c8c086944185111a99999e03d1083f9a9a Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 3 Feb 2016 13:53:42 -0800 Subject: [PATCH 32/73] Try to add the new renew verb to integration testing --- tests/boulder-integration.sh | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 53996cd20..5d9ed4859 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -44,20 +44,21 @@ common --domains le3.wtf install \ --cert-path "${root}/csr/cert.pem" \ --key-path "${root}/csr/key.pem" -# the following assumes that Boulder issues certificates for less than -# 10 years, otherwise renewal will not take place -cat < "$root/conf/renewer.conf" -renew_before_expiry = 10 years -deploy_before_expiry = 10 years -EOF -letsencrypt-renewer $store_flags -dir="$root/conf/archive/le1.wtf" -for x in cert chain fullchain privkey; -do - latest="$(ls -1t $dir/ | grep -e "^${x}" | head -n1)" - live="$($readlink -f "$root/conf/live/le1.wtf/${x}.pem")" - [ "${dir}/${latest}" = "$live" ] # renewer fails this test -done +# This won't renew (because it's not time yet) +common renew + +# This will renew +sed -i "4arenew_before_expiry = 10 years" "$root/conf/renewal/le1.wtf.conf" +common renew + +# letsencrypt-renewer $store_flags +# dir="$root/conf/archive/le1.wtf" +# for x in cert chain fullchain privkey; +# do +# latest="$(ls -1t $dir/ | grep -e "^${x}" | head -n1)" +# live="$($readlink -f "$root/conf/live/le1.wtf/${x}.pem")" +# [ "${dir}/${latest}" = "$live" ] # renewer fails this test +# done # revoke by account key common revoke --cert-path "$root/conf/live/le.wtf/cert.pem" From 1c52e1982ccb580e5bc4a3e0d28902c4865fe7e0 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 3 Feb 2016 17:49:08 -0800 Subject: [PATCH 33/73] made test for renew verb --- .../letstest/scripts/test_renew_standalone.sh | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100755 tests/letstest/scripts/test_renew_standalone.sh diff --git a/tests/letstest/scripts/test_renew_standalone.sh b/tests/letstest/scripts/test_renew_standalone.sh new file mode 100755 index 000000000..955cb104a --- /dev/null +++ b/tests/letstest/scripts/test_renew_standalone.sh @@ -0,0 +1,22 @@ +#!/bin/bash -x + +# $PUBLIC_IP $PRIVATE_IP $PUBLIC_HOSTNAME $BOULDER_URL are dynamically set at execution + +# with curl, instance metadata available from EC2 metadata service: +#public_host=$(curl -s http://169.254.169.254/2014-11-05/meta-data/public-hostname) +#public_ip=$(curl -s http://169.254.169.254/2014-11-05/meta-data/public-ipv4) +#private_ip=$(curl -s http://169.254.169.254/2014-11-05/meta-data/local-ipv4) + +cd letsencrypt +./letsencrypt-auto certonly -v --standalone --debug \ + --text --agree-dev-preview --agree-tos \ + --renew-by-default --redirect \ + --register-unsafely-without-email \ + --domain $PUBLIC_HOSTNAME --server $BOULDER_URL + +./letsencrypt-auto renew --renew-by-default + +ls /etc/letsencrypt/archive/$PUBLIC_HOSTNAME | grep -q 2.pem +if [ $? -ne 0 ] ; then + FAIL=1 +fi From c152b452b253d7f17cdf3c9fa6a8c72e14f40852 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 18:40:50 -0800 Subject: [PATCH 34/73] Start testing the renew verb, plus other goodies: * --dry-run works with renew * test harness for renewal is now fairly usable * coverage on cli.py 80% -> 88% --- letsencrypt/cli.py | 34 +++++++------ letsencrypt/storage.py | 3 ++ letsencrypt/tests/cli_test.py | 49 ++++++++++++++----- .../testdata/archive/sample-renewal/cert1.pem | 28 +++++++++++ .../archive/sample-renewal/chain1.pem | 19 +++++++ .../archive/sample-renewal/fullchain1.pem | 47 ++++++++++++++++++ .../archive/sample-renewal/privkey1.pem | 28 +++++++++++ .../testdata/live/sample-renewal/cert.pem | 1 + .../testdata/live/sample-renewal/chain.pem | 1 + .../live/sample-renewal/fullchain.pem | 1 + .../testdata/live/sample-renewal/privkey.pem | 1 + 11 files changed, 184 insertions(+), 28 deletions(-) create mode 100644 letsencrypt/tests/testdata/archive/sample-renewal/cert1.pem create mode 100644 letsencrypt/tests/testdata/archive/sample-renewal/chain1.pem create mode 100644 letsencrypt/tests/testdata/archive/sample-renewal/fullchain1.pem create mode 100644 letsencrypt/tests/testdata/archive/sample-renewal/privkey1.pem create mode 120000 letsencrypt/tests/testdata/live/sample-renewal/cert.pem create mode 120000 letsencrypt/tests/testdata/live/sample-renewal/chain.pem create mode 120000 letsencrypt/tests/testdata/live/sample-renewal/fullchain.pem create mode 120000 letsencrypt/tests/testdata/live/sample-renewal/privkey.pem diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4c30e2ca8..f97e9b550 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -8,6 +8,7 @@ import argparse import atexit import copy import functools +import glob import json import logging import logging.handlers @@ -223,15 +224,12 @@ def _find_duplicative_certs(config, domains): # Verify the directory is there le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) - for renewal_file in os.listdir(configs_dir): - if not renewal_file.endswith(".conf"): - continue + for renewal_file in _renewal_conf_files(config): try: - full_path = os.path.join(configs_dir, renewal_file) - candidate_lineage = storage.RenewableCert(full_path, cli_config) + candidate_lineage = storage.RenewableCert(renewal_file, cli_config) except (errors.CertStorageError, IOError): - logger.warning("Renewal configuration file %s is broken. " - "Skipping.", full_path) + logger.warning("Renewal conf file %s is broken. Skipping.", renewal_file) + logger.info("Traceback was:\n%s", traceback.format_exc()) continue # TODO: Handle these differently depending on whether they are # expired or still valid? @@ -794,8 +792,8 @@ def _reconstitute(full_path, config): try: renewal_candidate = storage.RenewableCert(full_path, config) except (errors.CertStorageError, IOError): - logger.warning("Renewal configuration file %s is broken. " - "Skipping.", full_path) + logger.warning("Renewal configuration file %s is broken. Skipping.", full_path) + logger.info("Traceback was:\n%s", traceback.format_exc()) return None if "renewalparams" not in renewal_candidate.configuration: logger.warning("Renewal configuration file %s lacks " @@ -834,6 +832,11 @@ def _reconstitute(full_path, config): return renewal_candidate +def _renewal_conf_files(config): + """Return /path/to/*.conf in the renewal conf directory""" + return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) + + def renew(cli_config, plugins): """Renew previously-obtained certificates.""" cli_config = configuration.RenewerConfiguration(cli_config) @@ -845,8 +848,7 @@ def renew(cli_config, plugins): "renew specific certificates, use the certonly " "command. The renew verb may provide other options " "for selecting certificates to renew in the future.") - configs_dir = cli_config.renewal_configs_dir - for renewal_file in os.listdir(configs_dir): + for renewal_file in _renewal_conf_files(cli_config): if not renewal_file.endswith(".conf"): continue print("Processing " + renewal_file) @@ -854,16 +856,16 @@ def renew(cli_config, plugins): # each time? config = configuration.RenewerConfiguration(copy.deepcopy(cli_config)) config.noninteractive_mode = True - full_path = os.path.join(configs_dir, renewal_file) # Note that this modifies config (to add back the configuration # elements from within the renewal configuration file). try: - renewal_candidate = _reconstitute(full_path, config) + renewal_candidate = _reconstitute(renewal_file, config) except Exception as e: # reconstitute encountered an unanticipated problem. logger.warning("Renewal configuration file %s produced an " - "unexpected error: %s. Skipping.", full_path, e) + "unexpected error: %s. Skipping.", renewal_file, e) + logger.info("Traceback was:\n%s", traceback.format_exc()) continue if renewal_candidate is None: @@ -1063,9 +1065,9 @@ class HelpfulArgumentParser(object): parsed_args.server = constants.STAGING_URI if parsed_args.dry_run: - if self.verb != "certonly": + if self.verb not in ["certonly", "renew"]: raise errors.Error("--dry-run currently only works with the " - "'certonly' subcommand") + "'certonly' or 'renew' subcommands") parsed_args.break_my_certs = parsed_args.staging = True return parsed_args diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index e41805459..ae43c3e41 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -728,6 +728,9 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes target_version = self.next_free_version() archive = self.cli_config.archive_dir + # XXX if anyone ever moves a renewal configuration file, this will + # break... perhaps prefix should be the dirname of the previous + # cert.pem? prefix = os.path.join(archive, self.lineagename) target = dict( [(kind, diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index f083018b3..721b38e9c 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -530,7 +530,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, self._certonly_new_request_common, mock_client) - def _test_renewal_common(self, due_for_renewal, extra_args, outstring, renew=True): + def _test_renewal_common(self, due_for_renewal, extra_args, log_out=None, + args=None, renew=True, out=False): cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) @@ -546,27 +547,33 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_init.return_value = mock_client get_utility_path = 'letsencrypt.cli.zope.component.getUtility' with mock.patch(get_utility_path) as mock_get_utility: - with mock.patch('letsencrypt.cli.OpenSSL'): + with mock.patch('letsencrypt.cli.OpenSSL') as mock_ssl: + mock_latest = mock.MagicMock() + mock_latest.get_issuer.return_value = "Fake fake" + mock_ssl.crypto.load_certificate.return_value = mock_latest with mock.patch('letsencrypt.cli.crypto_util'): - args = ['-d', 'foo.bar', '-a', - 'standalone', 'certonly'] + if not args: + args = ['-d', 'isnot.org', '-a', 'standalone', 'certonly'] if extra_args: args += extra_args - self._call(args) + if out: + self._call_stdout(args) + else: + self._call(args) - if outstring: + if log_out: with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: - self.assertTrue(outstring in lf.read()) + self.assertTrue(log_out in lf.read()) if renew: - mock_client.obtain_certificate.assert_called_once_with(['foo.bar']) + mock_client.obtain_certificate.assert_called_once_with(['isnot.org']) else: self.assertEqual(mock_client.obtain_certificate.call_count, 0) return mock_lineage, mock_get_utility def test_certonly_renewal(self): - lineage, get_utility = self._test_renewal_common(True, [], None) + lineage, get_utility = self._test_renewal_common(True, []) self.assertEqual(lineage.save_successor.call_count, 1) lineage.update_all_links_to.assert_called_once_with( lineage.latest_common_version()) @@ -577,17 +584,35 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_certonly_renewal_triggers(self): # --dry-run should force renewal _, get_utility = self._test_renewal_common(False, ['--dry-run', '--keep'], - "simulating renewal") + log_out="simulating renewal") self.assertEqual(get_utility().add_message.call_count, 1) self.assertTrue('dry run' in get_utility().add_message.call_args[0][0]) _, _ = self._test_renewal_common(False, ['--renew-by-default', '-tvv', '--debug'], - "Auto-renewal forced") + log_out="Auto-renewal forced") self.assertEqual(get_utility().add_message.call_count, 1) _, _ = self._test_renewal_common(False, ['-tvv', '--debug', '--keep'], - "not yet due", renew=False) + log_out="not yet due", renew=False) + def _dump_log(self): + with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: + print "Logs:" + print lf.read() + + def test_renewal_verb(self): + + with open(test_util.vector_path('sample-renewal.conf')) as src: + # put the correct path for cert.pem, chain.pem etc in the renewal conf + renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path()) + rd = os.path.join(self.config_dir, "renewal") + os.makedirs(rd) + rc = os.path.join(rd, "sample-renewal.conf") + with open(rc, "w") as dest: + dest.write(renewal_conf) + + self._test_renewal_common(True, [], args=["renew", "--dry-run", "-tvv"], + renew=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') diff --git a/letsencrypt/tests/testdata/archive/sample-renewal/cert1.pem b/letsencrypt/tests/testdata/archive/sample-renewal/cert1.pem new file mode 100644 index 000000000..4010000ef --- /dev/null +++ b/letsencrypt/tests/testdata/archive/sample-renewal/cert1.pem @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIE1DCCA7ygAwIBAgITAPoz/CBluNQV/Eh9F+CS6dSxEDANBgkqhkiG9w0BAQsF +ADAfMR0wGwYDVQQDDBRoYXBweSBoYWNrZXIgZmFrZSBDQTAeFw0xNjAyMDIyMzQ5 +MDBaFw0xNjA1MDIyMzQ5MDBaMBQxEjAQBgNVBAMTCWlzbm90Lm9yZzCCASIwDQYJ +KoZIhvcNAQEBBQADggEPADCCAQoCggEBALyudqLKcIdWZ5VaK1fuhlEDbZtvs2E+ +slm4dmSS1nFve7MdlZ69K0gdtnhkiPQ0wGQTligeDZ8fY8iL87GZO0tp5f7S+QJN +NYCiYw6j4qp5JBy/zG22kJz1Quu7/vXMYLzLvK6x6YixiWAWyqqvlUVBLS1r4W3h +A5Z+F1EIsXeyz7TJe3lAzIWAAxpfH9OviIz2rEDotuCdU771USLLNSw4qJojNlTx +UpZG6lGFs8KGb8tqROXknaMKE4PvN3SITixSUTFbktt1Wz60moWbNdLMKvgkzuUP +r4viO2P4SO5slNAY0ZeEssPpVAelN3EvrAcEZtoKmG5fnQDVo8uVag0CAwEAAaOC +AhIwggIOMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB +BQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUqhI4u6aaPrcYQnmypxV8Tap8 +L54wHwYDVR0jBBgwFoAU+3hPEvlgFYMsnxd/NBmzLjbqQYkweAYIKwYBBQUHAQEE +bDBqMDMGCCsGAQUFBzABhidodHRwOi8vb2NzcC5zdGFnaW5nLXgxLmxldHNlbmNy +eXB0Lm9yZy8wMwYIKwYBBQUHMAKGJ2h0dHA6Ly9jZXJ0LnN0YWdpbmcteDEubGV0 +c2VuY3J5cHQub3JnLzAUBgNVHREEDTALgglpc25vdC5vcmcwgf4GA1UdIASB9jCB +8zAIBgZngQwBAgEwgeYGCysGAQQBgt8TAQEBMIHWMCYGCCsGAQUFBwIBFhpodHRw +Oi8vY3BzLmxldHNlbmNyeXB0Lm9yZzCBqwYIKwYBBQUHAgIwgZ4MgZtUaGlzIENl +cnRpZmljYXRlIG1heSBvbmx5IGJlIHJlbGllZCB1cG9uIGJ5IFJlbHlpbmcgUGFy +dGllcyBhbmQgb25seSBpbiBhY2NvcmRhbmNlIHdpdGggdGhlIENlcnRpZmljYXRl +IFBvbGljeSBmb3VuZCBhdCBodHRwczovL2xldHNlbmNyeXB0Lm9yZy9yZXBvc2l0 +b3J5LzANBgkqhkiG9w0BAQsFAAOCAQEAbAhX6FfQwELayneY4l5RvYSdw/Jj5CRy +KzrM7ISld7x9YPpxX6Pmht/YyMhLWrtxvFUR2+RNhSIYB8IjQEjmKjvR7UNeiUve +jzPEAuTg/9m3i0FJpPHc2aKGzlLFQCMm5/RrvnXI6ljIcyhocLvMiN46iexcExI2 +Ese3w8GoH6wARYKxU/QBexfoXQLgtAbYzNRE6EgKWtB+txV+7+d2MgbhCEit5VwU ++ydT8inp9URsA7iKM03hDdGOBysddkrm1/yEhVy/Oo6bT9WMAUHVvz61hHekWcSf +rAQ6BayubvWOUx06eTowXr1gln/rl+WXOxcsJeag127NuhmHOCXZxQ== +-----END CERTIFICATE----- diff --git a/letsencrypt/tests/testdata/archive/sample-renewal/chain1.pem b/letsencrypt/tests/testdata/archive/sample-renewal/chain1.pem new file mode 100644 index 000000000..760417fe9 --- /dev/null +++ b/letsencrypt/tests/testdata/archive/sample-renewal/chain1.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDETCCAfmgAwIBAgIJAJzxkS6o1QkIMA0GCSqGSIb3DQEBCwUAMB8xHTAbBgNV +BAMMFGhhcHB5IGhhY2tlciBmYWtlIENBMB4XDTE1MDQwNzIzNTAzOFoXDTI1MDQw +NDIzNTAzOFowHzEdMBsGA1UEAwwUaGFwcHkgaGFja2VyIGZha2UgQ0EwggEiMA0G +CSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDCCkd5mgXFErJ3F2M0E9dw+Ta/md5i +8TDId01HberAApqmydG7UZYF3zLTSzNjlNSOmtybvrSGUnZ9r9tSQcL8VM6WUOM8 +tnIpiIjEA2QkBycMwvRmZ/B2ltPdYs/R9BqNwO1g18GDZrHSzUYtNKNeFI6Glamj +7GK2Vr0SmiEamlNIR5ktAFsEErzf/d4jCF7sosMsJpMCm1p58QkP4LHLShVLXDa8 +BMfVoI+ipYcA08iNUFkgW8VWDclIDxcysa0psDDtMjX3+4aPkE/cefmP+1xOfUuD +HOGV8XFynsP4EpTfVOZr0/g9gYQ7ZArqXX7GTQkFqduwPm/w5qxSPTarAgMBAAGj +UDBOMB0GA1UdDgQWBBT7eE8S+WAVgyyfF380GbMuNupBiTAfBgNVHSMEGDAWgBT7 +eE8S+WAVgyyfF380GbMuNupBiTAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBCwUA +A4IBAQAd9Da+Zv+TjMv7NTAmliqnWHY6d3UxEZN3hFEJ58IQVHbBZVZdW7zhRktB +vR05Kweac0HJeK91TKmzvXl21IXLvh0gcNLU/uweD3no/snfdB4OoFompljThmgl +zBqiqWoKBJQrLCA8w5UB+ReomRYd/EYXF/6TAfzm6hr//Xt5mPiUHPdvYt75lMAo +vRxLSbF8TSQ6b7BYxISWjPgFASNNqJNHEItWsmQMtAjjwzb9cs01XH9pChVAWn9L +oeMKa+SlHSYrWG93+EcrIH/dGU76uNOiaDzBSKvaehG53h25MHuO1anNICJvZovW +rFo4Uv1EnkKJm3vJFe50eJGhEKlx +-----END CERTIFICATE----- diff --git a/letsencrypt/tests/testdata/archive/sample-renewal/fullchain1.pem b/letsencrypt/tests/testdata/archive/sample-renewal/fullchain1.pem new file mode 100644 index 000000000..6e24d6038 --- /dev/null +++ b/letsencrypt/tests/testdata/archive/sample-renewal/fullchain1.pem @@ -0,0 +1,47 @@ +-----BEGIN CERTIFICATE----- +MIIE1DCCA7ygAwIBAgITAPoz/CBluNQV/Eh9F+CS6dSxEDANBgkqhkiG9w0BAQsF +ADAfMR0wGwYDVQQDDBRoYXBweSBoYWNrZXIgZmFrZSBDQTAeFw0xNjAyMDIyMzQ5 +MDBaFw0xNjA1MDIyMzQ5MDBaMBQxEjAQBgNVBAMTCWlzbm90Lm9yZzCCASIwDQYJ +KoZIhvcNAQEBBQADggEPADCCAQoCggEBALyudqLKcIdWZ5VaK1fuhlEDbZtvs2E+ +slm4dmSS1nFve7MdlZ69K0gdtnhkiPQ0wGQTligeDZ8fY8iL87GZO0tp5f7S+QJN +NYCiYw6j4qp5JBy/zG22kJz1Quu7/vXMYLzLvK6x6YixiWAWyqqvlUVBLS1r4W3h +A5Z+F1EIsXeyz7TJe3lAzIWAAxpfH9OviIz2rEDotuCdU771USLLNSw4qJojNlTx +UpZG6lGFs8KGb8tqROXknaMKE4PvN3SITixSUTFbktt1Wz60moWbNdLMKvgkzuUP +r4viO2P4SO5slNAY0ZeEssPpVAelN3EvrAcEZtoKmG5fnQDVo8uVag0CAwEAAaOC +AhIwggIOMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB +BQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUqhI4u6aaPrcYQnmypxV8Tap8 +L54wHwYDVR0jBBgwFoAU+3hPEvlgFYMsnxd/NBmzLjbqQYkweAYIKwYBBQUHAQEE +bDBqMDMGCCsGAQUFBzABhidodHRwOi8vb2NzcC5zdGFnaW5nLXgxLmxldHNlbmNy +eXB0Lm9yZy8wMwYIKwYBBQUHMAKGJ2h0dHA6Ly9jZXJ0LnN0YWdpbmcteDEubGV0 +c2VuY3J5cHQub3JnLzAUBgNVHREEDTALgglpc25vdC5vcmcwgf4GA1UdIASB9jCB +8zAIBgZngQwBAgEwgeYGCysGAQQBgt8TAQEBMIHWMCYGCCsGAQUFBwIBFhpodHRw +Oi8vY3BzLmxldHNlbmNyeXB0Lm9yZzCBqwYIKwYBBQUHAgIwgZ4MgZtUaGlzIENl +cnRpZmljYXRlIG1heSBvbmx5IGJlIHJlbGllZCB1cG9uIGJ5IFJlbHlpbmcgUGFy +dGllcyBhbmQgb25seSBpbiBhY2NvcmRhbmNlIHdpdGggdGhlIENlcnRpZmljYXRl +IFBvbGljeSBmb3VuZCBhdCBodHRwczovL2xldHNlbmNyeXB0Lm9yZy9yZXBvc2l0 +b3J5LzANBgkqhkiG9w0BAQsFAAOCAQEAbAhX6FfQwELayneY4l5RvYSdw/Jj5CRy +KzrM7ISld7x9YPpxX6Pmht/YyMhLWrtxvFUR2+RNhSIYB8IjQEjmKjvR7UNeiUve +jzPEAuTg/9m3i0FJpPHc2aKGzlLFQCMm5/RrvnXI6ljIcyhocLvMiN46iexcExI2 +Ese3w8GoH6wARYKxU/QBexfoXQLgtAbYzNRE6EgKWtB+txV+7+d2MgbhCEit5VwU ++ydT8inp9URsA7iKM03hDdGOBysddkrm1/yEhVy/Oo6bT9WMAUHVvz61hHekWcSf +rAQ6BayubvWOUx06eTowXr1gln/rl+WXOxcsJeag127NuhmHOCXZxQ== +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDETCCAfmgAwIBAgIJAJzxkS6o1QkIMA0GCSqGSIb3DQEBCwUAMB8xHTAbBgNV +BAMMFGhhcHB5IGhhY2tlciBmYWtlIENBMB4XDTE1MDQwNzIzNTAzOFoXDTI1MDQw +NDIzNTAzOFowHzEdMBsGA1UEAwwUaGFwcHkgaGFja2VyIGZha2UgQ0EwggEiMA0G +CSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDCCkd5mgXFErJ3F2M0E9dw+Ta/md5i +8TDId01HberAApqmydG7UZYF3zLTSzNjlNSOmtybvrSGUnZ9r9tSQcL8VM6WUOM8 +tnIpiIjEA2QkBycMwvRmZ/B2ltPdYs/R9BqNwO1g18GDZrHSzUYtNKNeFI6Glamj +7GK2Vr0SmiEamlNIR5ktAFsEErzf/d4jCF7sosMsJpMCm1p58QkP4LHLShVLXDa8 +BMfVoI+ipYcA08iNUFkgW8VWDclIDxcysa0psDDtMjX3+4aPkE/cefmP+1xOfUuD +HOGV8XFynsP4EpTfVOZr0/g9gYQ7ZArqXX7GTQkFqduwPm/w5qxSPTarAgMBAAGj +UDBOMB0GA1UdDgQWBBT7eE8S+WAVgyyfF380GbMuNupBiTAfBgNVHSMEGDAWgBT7 +eE8S+WAVgyyfF380GbMuNupBiTAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBCwUA +A4IBAQAd9Da+Zv+TjMv7NTAmliqnWHY6d3UxEZN3hFEJ58IQVHbBZVZdW7zhRktB +vR05Kweac0HJeK91TKmzvXl21IXLvh0gcNLU/uweD3no/snfdB4OoFompljThmgl +zBqiqWoKBJQrLCA8w5UB+ReomRYd/EYXF/6TAfzm6hr//Xt5mPiUHPdvYt75lMAo +vRxLSbF8TSQ6b7BYxISWjPgFASNNqJNHEItWsmQMtAjjwzb9cs01XH9pChVAWn9L +oeMKa+SlHSYrWG93+EcrIH/dGU76uNOiaDzBSKvaehG53h25MHuO1anNICJvZovW +rFo4Uv1EnkKJm3vJFe50eJGhEKlx +-----END CERTIFICATE----- diff --git a/letsencrypt/tests/testdata/archive/sample-renewal/privkey1.pem b/letsencrypt/tests/testdata/archive/sample-renewal/privkey1.pem new file mode 100644 index 000000000..f03fdd0a3 --- /dev/null +++ b/letsencrypt/tests/testdata/archive/sample-renewal/privkey1.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC8rnaiynCHVmeV +WitX7oZRA22bb7NhPrJZuHZkktZxb3uzHZWevStIHbZ4ZIj0NMBkE5YoHg2fH2PI +i/OxmTtLaeX+0vkCTTWAomMOo+KqeSQcv8xttpCc9ULru/71zGC8y7yusemIsYlg +Fsqqr5VFQS0ta+Ft4QOWfhdRCLF3ss+0yXt5QMyFgAMaXx/Tr4iM9qxA6LbgnVO+ +9VEiyzUsOKiaIzZU8VKWRupRhbPChm/LakTl5J2jChOD7zd0iE4sUlExW5LbdVs+ +tJqFmzXSzCr4JM7lD6+L4jtj+EjubJTQGNGXhLLD6VQHpTdxL6wHBGbaCphuX50A +1aPLlWoNAgMBAAECggEAfKKWFWS6PnwSAnNErFoQeZVVItb/XB5JO8EA2+CvLNFi +mefR/MCixYlzDkYCvaXW7ISPrMJlZxYaGNBx0oAQzfkPB2wfNqj/zY/29SXGxast +8puzk0mEb1oHsaZGfeFaiXvfkFpPlI8J2uJTT7qaVNv/1sArciSv9QonpsyiRhlB +yqT49juNVoR1tJHyXzkkRfHKTG8OlJd4kuFOl3fM9dTFPQ/ft0kTNAQ/B4SFvSwF +RJsbLbsbFGsUdV9ekE6UX6oWD/Ah707rvgtCyS0Bc+0O3t2EKwmm3RXPRUMHCVxE +bKdTxRB4etbjMVXMuVhB8Y4GbfrtMCy+qxZQ6znCAQKBgQDr7bcYAZVZp/nBMVB+ +lBO9w73J6lnEWm6bZ9728KlGAKETaRhxZQSi6TN6MWwNwnk6rinyz4uVwVr9ZRCs +WkB1TbvW0JNcWdr3YClwsKXAt8X22bjGe0LagDJHG6r1TPS+MdovOS2M6IMaxlbT +rzFhSJ8ojLX3tqnOsmc7YAFLjQKBgQDMu8E9hoJt82lQzOGrjHmGzGEu2GLx9WKO +e4nkj335kX6fIhMMqSXBFbTJZwXoYvk5J8ZnaARbYG0m5nxDCwRjX5HWa8q0B2Po +ta53w01sKKznzlPjUhsdhEthun7MCFfLZpgvcZ9xVzOXo3/Zfn2+RrsPSjrVDqBy +hj+k5mW4gQKBgHFWKf3LTO7cBdvsD8ou4mjn7nVgMi1kb/wR4wdnxzmMtdR4STi4 +GYkVVBhgQ5M8mDY7UoWFdH3FfCt8cI0Lcimn5ROl8RSNSeZKeL3c7lNtNRmHr/8R +WaVTrlOAlBjxFiWEF1dWNW6ah9jF7RIV+DfOxj6ZkhTk2CAmjfb1AMpFAoGABf96 +KdNG/vGipDtcYSo8ZTaXoke0nmISARqdb5TEnAsnKoJVDInoEUARi9T411YO9x2z +MlRZzFOG3xzhhxVLi53BKAcAaUXOJ4MrGVcfbYvDhQcGbiJ5qOO3UaWlEVUtPUhE +LR+nDCsB1+9yT2zlQi3QTSJflt5W1QQZ2TrmwAECgYEAvQ7+sTcHs1K9yKj7koEu +A19FbMA0IwvrVRcV/VqmlsoW6e6wW2YND+GtaDbKdD0aBPivqLJwpNFrsRA+W0iB +vzmML6sKhhL+j7tjSgq+iQdBkKz0j9PyReuhe9CRnljMmyun+4qKEk0KUvxBrjPY +Skn+ML18qyUoEPnmbpfHxCs= +-----END PRIVATE KEY----- diff --git a/letsencrypt/tests/testdata/live/sample-renewal/cert.pem b/letsencrypt/tests/testdata/live/sample-renewal/cert.pem new file mode 120000 index 000000000..e06effe40 --- /dev/null +++ b/letsencrypt/tests/testdata/live/sample-renewal/cert.pem @@ -0,0 +1 @@ +../../archive/sample-renewal/cert1.pem \ No newline at end of file diff --git a/letsencrypt/tests/testdata/live/sample-renewal/chain.pem b/letsencrypt/tests/testdata/live/sample-renewal/chain.pem new file mode 120000 index 000000000..71f665f29 --- /dev/null +++ b/letsencrypt/tests/testdata/live/sample-renewal/chain.pem @@ -0,0 +1 @@ +../../archive/sample-renewal/chain1.pem \ No newline at end of file diff --git a/letsencrypt/tests/testdata/live/sample-renewal/fullchain.pem b/letsencrypt/tests/testdata/live/sample-renewal/fullchain.pem new file mode 120000 index 000000000..0f06f077d --- /dev/null +++ b/letsencrypt/tests/testdata/live/sample-renewal/fullchain.pem @@ -0,0 +1 @@ +../../archive/sample-renewal/fullchain1.pem \ No newline at end of file diff --git a/letsencrypt/tests/testdata/live/sample-renewal/privkey.pem b/letsencrypt/tests/testdata/live/sample-renewal/privkey.pem new file mode 120000 index 000000000..5187eda6b --- /dev/null +++ b/letsencrypt/tests/testdata/live/sample-renewal/privkey.pem @@ -0,0 +1 @@ +../../archive/sample-renewal/privkey1.pem \ No newline at end of file From a659b07b4cd0cebec53cf294e7d3b531e18caca8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 18:44:27 -0800 Subject: [PATCH 35/73] Reininitialize plugins for every lineage --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4c30e2ca8..4361ed886 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -878,8 +878,8 @@ def renew(cli_config, plugins): # or not, we couldn't currently make a UI/logging distinction at # this stage to indicate whether renewal was actually attempted # (or successful). - obtain_cert(config, plugins, renewal_candidate) - + obtain_cert(config, plugins_disco.PluginsRegistry.find_all(), + renewal_candidate) def revoke(config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" From 4d8dbc9d81de9f7d8d2d1209f53df4d815bb99d1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 18:55:06 -0800 Subject: [PATCH 36/73] Lint this entire monstrosity - Doing some of @schoen's refactoring homework for him :) --- letsencrypt/cli.py | 18 ++++++++++-------- letsencrypt/plugins/manual.py | 1 - letsencrypt/tests/cli_test.py | 8 +++----- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f97e9b550..b6bb34a68 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -746,6 +746,8 @@ def _restore_required_config_elements(full_path, config, renewalparams): "a non-numeric value for %s. Skipping.", full_path, config_item) raise + +def _restore_plugin_configs(config, renewalparams): # Now use parser to get plugin-prefixed items with correct types # XXX: the current approach of extracting only prefixed items # related to the actually-used installer and authenticator @@ -767,13 +769,12 @@ def _restore_required_config_elements(full_path, config, renewalparams): config.__setattr__(config_item, None) continue if config_item.startswith(plugin_prefix + "_"): - for action in _parser.parser._actions: - if action.dest == config_item: - if action.type is not None: - config.__setattr__(config_item, action.type(renewalparams[config_item])) - break + for action in _parser.parser._actions: # pylint: disable=protected-access + if action.type is not None and action.dest == config_item: + config.__setattr__(config_item, action.type(renewalparams[config_item])) + break else: - config.__setattr__(config_item, str(renewalparams[config_item])) + config.__setattr__(config_item, str(renewalparams[config_item])) return True @@ -808,6 +809,7 @@ def _reconstitute(full_path, config): # those elements are present. try: _restore_required_config_elements(full_path, config, renewalparams) + _restore_plugin_configs(config, renewalparams) except ValueError: # There was a data type error which has already been # logged. @@ -861,7 +863,7 @@ def renew(cli_config, plugins): # elements from within the renewal configuration file). try: renewal_candidate = _reconstitute(renewal_file, config) - except Exception as e: + except Exception as e: # pylint: disable=broad-except # reconstitute encountered an unanticipated problem. logger.warning("Renewal configuration file %s produced an " "unexpected error: %s. Skipping.", renewal_file, e) @@ -1356,7 +1358,7 @@ def prepare_and_parse_args(plugins, args): # parser (--help should display plugin-specific options last) _plugins_parsing(helpful, plugins) - global _parser + global _parser # pylint: disable=global-statement _parser = helpful return helpful.parse_args() diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 29f4639fe..54244db2a 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -93,7 +93,6 @@ s.serve_forever()" """ def prepare(self): # pylint: disable=missing-docstring,no-self-use if self.config.noninteractive_mode: raise errors.PluginError("Running manual mode non-interactively is not supported") - pass # pragma: no cover def more_info(self): # pylint: disable=missing-docstring,no-self-use return ("This plugin requires user's manual intervention in setting " diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 721b38e9c..46672973c 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -531,7 +531,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._certonly_new_request_common, mock_client) def _test_renewal_common(self, due_for_renewal, extra_args, log_out=None, - args=None, renew=True, out=False): + args=None, renew=True): + # pylint: disable=too-many-locals cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) @@ -556,10 +557,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args = ['-d', 'isnot.org', '-a', 'standalone', 'certonly'] if extra_args: args += extra_args - if out: - self._call_stdout(args) - else: - self._call(args) + self._call(args) if log_out: with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: From 605979ce99599b93987addb24133a47b18f34d09 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 19:07:07 -0800 Subject: [PATCH 37/73] Revert "Avoid dangerous and mysterious behaviour if someone tries to modify a config" This reverts commit 83afb58a9a6099ad1e3c54097c2bb509e98f38f8. --- letsencrypt/configuration.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 72aabe548..04053c8c3 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -43,12 +43,6 @@ class NamespaceConfig(object): # Check command line parameters sanity, and error out in case of problem. check_config_sanity(self) - # We're done setting up the attic. Now pull up the ladder after ourselves... - self.__setattr__ = self.__setattr_implementation__ - - def __setattr_implementation__(self, var, value): - return self.namespace.__setattr__(var, value) - def __getattr__(self, name): return getattr(self.namespace, name) From 2762a541fffc672784b7d8685247f971080566cc Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 19:08:41 -0800 Subject: [PATCH 38/73] git add a missing file --- .../tests/testdata/sample-renewal.conf | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100755 letsencrypt/tests/testdata/sample-renewal.conf diff --git a/letsencrypt/tests/testdata/sample-renewal.conf b/letsencrypt/tests/testdata/sample-renewal.conf new file mode 100755 index 000000000..16778303a --- /dev/null +++ b/letsencrypt/tests/testdata/sample-renewal.conf @@ -0,0 +1,76 @@ +cert = MAGICDIR/live/sample-renewal/cert.pem +privkey = MAGICDIR/live/sample-renewal/privkey.pem +chain = MAGICDIR/live/sample-renewal/chain.pem +fullchain = MAGICDIR/live/sample-renewal/fullchain.pem +renew_before_expiry = 1 year + +# Options and defaults used in the renewal process +[renewalparams] +no_self_upgrade = False +apache_enmod = a2enmod +no_verify_ssl = False +ifaces = None +apache_dismod = a2dismod +register_unsafely_without_email = False +apache_handle_modules = True +uir = None +installer = none +nginx_ctl = nginx +config_dir = MAGICDIR +text_mode = False +func = +staging = True +prepare = False +work_dir = /var/lib/letsencrypt +tos = False +init = False +http01_port = 80 +duplicate = False +noninteractive_mode = True +key_path = None +nginx = False +nginx_server_root = /etc/nginx +fullchain_path = /home/ubuntu/letsencrypt/chain.pem +email = None +csr = None +agree_dev_preview = None +redirect = None +verb = certonly +verbose_count = -3 +config_file = None +renew_by_default = False +hsts = False +apache_handle_sites = True +authenticator = standalone +domains = isnot.org, +rsa_key_size = 2048 +apache_challenge_location = /etc/apache2 +checkpoints = 1 +manual_test_mode = False +apache = False +cert_path = /home/ubuntu/letsencrypt/cert.pem +webroot_path = None +reinstall = False +expand = False +strict_permissions = False +apache_server_root = /etc/apache2 +account = None +dry_run = False +manual_public_ip_logging_ok = False +chain_path = /home/ubuntu/letsencrypt/chain.pem +break_my_certs = False +standalone = True +manual = False +server = https://acme-staging.api.letsencrypt.org/directory +standalone_supported_challenges = "tls-sni-01,http-01" +webroot = False +os_packages_only = False +apache_init_script = None +user_agent = None +apache_le_vhost_ext = -le-ssl.conf +debug = False +tls_sni_01_port = 443 +logs_dir = /var/log/letsencrypt +apache_vhost_root = /etc/apache2/sites-available +configurator = None +[[webroot_map]] From ea76c07832b12166fb46ac7f9f13c60f2c3469df Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 19:09:40 -0800 Subject: [PATCH 39/73] s/config\./config\.namespace/ --- letsencrypt/cli.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 385915750..caf6d677c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -734,13 +734,13 @@ def _restore_required_config_elements(full_path, config, renewalparams): # so we don't know if the original was NoneType or str! if value == "None": value = None - config.__setattr__(config_item, value) + config.namespace__setattr__(config_item, value) # int-valued items to add if they're present for config_item in INT_CONFIG_ITEMS: if config_item in renewalparams: try: value = int(renewalparams[config_item]) - config.__setattr__(config_item, value) + config.namespace__setattr__(config_item, value) except ValueError: logger.warning("Renewal configuration file %s specifies " "a non-numeric value for %s. Skipping.", @@ -766,15 +766,18 @@ def _restore_plugin_configs(config, renewalparams): # trying to read the file called "None") # Should we omit the item entirely rather than setting # its value to None? - config.__setattr__(config_item, None) + config.namespace__setattr__(config_item, None) continue if config_item.startswith(plugin_prefix + "_"): for action in _parser.parser._actions: # pylint: disable=protected-access if action.type is not None and action.dest == config_item: - config.__setattr__(config_item, action.type(renewalparams[config_item])) + config.namespace__setattr__( + config_item, + action.type(renewalparams[config_item])) break else: - config.__setattr__(config_item, str(renewalparams[config_item])) + config.namespace__setattr__( + config_item, str(renewalparams[config_item])) return True @@ -819,7 +822,8 @@ def _reconstitute(full_path, config): # configuration restoring logic is not able to correctly parse it # from the serialized form. if "webroot_map" in renewalparams: - config.__setattr__("webroot_map", renewalparams["webroot_map"]) + config.namespace__setattr__( + "webroot_map", renewalparams["webroot_map"]) try: domains = [le_util.enforce_domain_sanity(x) for x in @@ -830,7 +834,7 @@ def _reconstitute(full_path, config): "invalid. Skipping.", full_path) return None - config.__setattr__("domains", domains) + config.namespace__setattr__("domains", domains) return renewal_candidate From 1536c8fca3252aeb4dcd28b0cd2c881b322c48a7 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 19:29:27 -0800 Subject: [PATCH 40/73] Fix the things I broke --- letsencrypt/cli.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index caf6d677c..7911ba999 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -734,13 +734,13 @@ def _restore_required_config_elements(full_path, config, renewalparams): # so we don't know if the original was NoneType or str! if value == "None": value = None - config.namespace__setattr__(config_item, value) + setattr(config.namespace, config_item, value) # int-valued items to add if they're present for config_item in INT_CONFIG_ITEMS: if config_item in renewalparams: try: value = int(renewalparams[config_item]) - config.namespace__setattr__(config_item, value) + setattr(config.namespace, config_item, value) except ValueError: logger.warning("Renewal configuration file %s specifies " "a non-numeric value for %s. Skipping.", @@ -766,18 +766,17 @@ def _restore_plugin_configs(config, renewalparams): # trying to read the file called "None") # Should we omit the item entirely rather than setting # its value to None? - config.namespace__setattr__(config_item, None) + setattr(config.namespace, config_item, None) continue if config_item.startswith(plugin_prefix + "_"): for action in _parser.parser._actions: # pylint: disable=protected-access if action.type is not None and action.dest == config_item: - config.namespace__setattr__( - config_item, - action.type(renewalparams[config_item])) + setattr(config.namespace, config_item, + action.type(renewalparams[config_item])) break else: - config.namespace__setattr__( - config_item, str(renewalparams[config_item])) + setattr(config.namespace, config_item, + str(renewalparams[config_item])) return True @@ -822,8 +821,7 @@ def _reconstitute(full_path, config): # configuration restoring logic is not able to correctly parse it # from the serialized form. if "webroot_map" in renewalparams: - config.namespace__setattr__( - "webroot_map", renewalparams["webroot_map"]) + setattr(config.namespace, "webroot_map", renewalparams["webroot_map"]) try: domains = [le_util.enforce_domain_sanity(x) for x in @@ -834,7 +832,7 @@ def _reconstitute(full_path, config): "invalid. Skipping.", full_path) return None - config.namespace__setattr__("domains", domains) + setattr(config.namespace, "domains", domains) return renewal_candidate @@ -843,7 +841,7 @@ def _renewal_conf_files(config): return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) -def renew(cli_config, plugins): +def renew(cli_config, unused_plugins): """Renew previously-obtained certificates.""" cli_config = configuration.RenewerConfiguration(cli_config) if cli_config.domains != []: From 5e656122dee7767944b7212e750a3d16f877bd8c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 19:36:14 -0800 Subject: [PATCH 41/73] Use correct config --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7911ba999..f4335c701 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -224,7 +224,7 @@ def _find_duplicative_certs(config, domains): # Verify the directory is there le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) - for renewal_file in _renewal_conf_files(config): + for renewal_file in _renewal_conf_files(cli_config): try: candidate_lineage = storage.RenewableCert(renewal_file, cli_config) except (errors.CertStorageError, IOError): From 4d6a3dfdff02d8ea5078be7857b4395becdc9b9a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 20:04:07 -0800 Subject: [PATCH 42/73] Apparently py26 can't deepcopy a MagicMock? --- letsencrypt/tests/cli_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index f397c1081..3e6c050cf 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -609,8 +609,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with open(rc, "w") as dest: dest.write(renewal_conf) - self._test_renewal_common(True, [], args=["renew", "--dry-run", "-tvv"], - renew=True) + with mock.patch('letsencrypt.cli.copy.deepcopy'): + self._test_renewal_common(True, [], args=["renew", "--dry-run", "-tvv"], + renew=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From 139326db7a4e401c2e91342f04dc9d837dca5115 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 20:20:27 -0800 Subject: [PATCH 43/73] That didn't work :( --- letsencrypt/tests/cli_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 3e6c050cf..f397c1081 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -609,9 +609,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with open(rc, "w") as dest: dest.write(renewal_conf) - with mock.patch('letsencrypt.cli.copy.deepcopy'): - self._test_renewal_common(True, [], args=["renew", "--dry-run", "-tvv"], - renew=True) + self._test_renewal_common(True, [], args=["renew", "--dry-run", "-tvv"], + renew=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From 77e9f9f9b47efdd6101b55cbedaccfcab54873b5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 22:17:38 -0800 Subject: [PATCH 44/73] hack around horrible ancient py26 + deepcopy + mock issue --- letsencrypt/tests/cli_test.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index f397c1081..13470dbb2 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -559,14 +559,17 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args += extra_args self._call(args) - if log_out: - with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: - self.assertTrue(log_out in lf.read()) + try: + if log_out: + with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: + self.assertTrue(log_out in lf.read()) - if renew: - mock_client.obtain_certificate.assert_called_once_with(['isnot.org']) - else: - self.assertEqual(mock_client.obtain_certificate.call_count, 0) + if renew: + mock_client.obtain_certificate.assert_called_once_with(['isnot.org']) + else: + self.assertEqual(mock_client.obtain_certificate.call_count, 0) + except: + self._dump_log() return mock_lineage, mock_get_utility @@ -609,8 +612,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with open(rc, "w") as dest: dest.write(renewal_conf) - self._test_renewal_common(True, [], args=["renew", "--dry-run", "-tvv"], - renew=True) + # Work around https://bugs.python.org/issue1515 for py26 tests :( :( + # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276 + with mock.patch('letsencrypt.cli.copy.deepcopy', side_effect=lambda x: x) as hack: + args = ["renew", "--dry-run", "-tvv"] + self._test_renewal_common(True, [], args=args, renew=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From 94816f32a5972af13e4f83d6e650df441b669689 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 22:36:30 -0800 Subject: [PATCH 45/73] Try this a different way --- letsencrypt/tests/cli_test.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 13470dbb2..fd00c4465 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -32,6 +32,9 @@ CERT = test_util.vector_path('cert.pem') CSR = test_util.vector_path('csr.der') KEY = test_util.vector_path('rsa256_key.pem') +def hack(x): + return x + class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods """Tests for different commands.""" @@ -601,7 +604,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods print "Logs:" print lf.read() - def test_renewal_verb(self): + + # Work around https://bugs.python.org/issue1515 for py26 tests :( :( + # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276 + @mock.patch('letsencrypt.cli.copy.deepcopy') + def test_renewal_verb(self, hack_copy): with open(test_util.vector_path('sample-renewal.conf')) as src: # put the correct path for cert.pem, chain.pem etc in the renewal conf @@ -611,12 +618,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods rc = os.path.join(rd, "sample-renewal.conf") with open(rc, "w") as dest: dest.write(renewal_conf) - - # Work around https://bugs.python.org/issue1515 for py26 tests :( :( - # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276 - with mock.patch('letsencrypt.cli.copy.deepcopy', side_effect=lambda x: x) as hack: - args = ["renew", "--dry-run", "-tvv"] - self._test_renewal_common(True, [], args=args, renew=True) + hack_copy.side_effect = hack + args = ["renew", "--dry-run", "-tvv"] + self._test_renewal_common(True, [], args=args, renew=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From 8fdcb772d9047318c8b678a41af4eb8f8f095452 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 4 Feb 2016 09:29:11 -0800 Subject: [PATCH 46/73] return failure --- tests/letstest/scripts/test_renew_standalone.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/letstest/scripts/test_renew_standalone.sh b/tests/letstest/scripts/test_renew_standalone.sh index 955cb104a..b9f00efe0 100755 --- a/tests/letstest/scripts/test_renew_standalone.sh +++ b/tests/letstest/scripts/test_renew_standalone.sh @@ -20,3 +20,7 @@ ls /etc/letsencrypt/archive/$PUBLIC_HOSTNAME | grep -q 2.pem if [ $? -ne 0 ] ; then FAIL=1 fi + +if [ "$FAIL" = 1 ] ; then + exit 1 +fi From dc9a51b2e68c65ecda197842835d13e8cd94fbc3 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 4 Feb 2016 09:38:45 -0800 Subject: [PATCH 47/73] make a robust test script --- .../letstest/scripts/test_renew_standalone.sh | 55 ++++++++++++++----- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/tests/letstest/scripts/test_renew_standalone.sh b/tests/letstest/scripts/test_renew_standalone.sh index b9f00efe0..d90ae9ab6 100755 --- a/tests/letstest/scripts/test_renew_standalone.sh +++ b/tests/letstest/scripts/test_renew_standalone.sh @@ -1,26 +1,55 @@ #!/bin/bash -x -# $PUBLIC_IP $PRIVATE_IP $PUBLIC_HOSTNAME $BOULDER_URL are dynamically set at execution - -# with curl, instance metadata available from EC2 metadata service: -#public_host=$(curl -s http://169.254.169.254/2014-11-05/meta-data/public-hostname) -#public_ip=$(curl -s http://169.254.169.254/2014-11-05/meta-data/public-ipv4) -#private_ip=$(curl -s http://169.254.169.254/2014-11-05/meta-data/local-ipv4) +# $OS_TYPE $PUBLIC_IP $PRIVATE_IP $PUBLIC_HOSTNAME $BOULDER_URL +# are dynamically set at execution +# run letsencrypt-apache2 via letsencrypt-auto cd letsencrypt -./letsencrypt-auto certonly -v --standalone --debug \ - --text --agree-dev-preview --agree-tos \ - --renew-by-default --redirect \ - --register-unsafely-without-email \ - --domain $PUBLIC_HOSTNAME --server $BOULDER_URL -./letsencrypt-auto renew --renew-by-default +export SUDO=sudo +if [ -f /etc/debian_version ] ; then + echo "Bootstrapping dependencies for Debian-based OSes..." + $SUDO bootstrap/_deb_common.sh +elif [ -f /etc/redhat-release ] ; then + echo "Bootstrapping dependencies for RedHat-based OSes..." + $SUDO bootstrap/_rpm_common.sh +else + echo "Dont have bootstrapping for this OS!" + exit 1 +fi -ls /etc/letsencrypt/archive/$PUBLIC_HOSTNAME | grep -q 2.pem +bootstrap/dev/venv.sh +sudo venv/bin/letsencrypt certonly --debug --standalone -t --agree-dev-preview --agree-tos \ + --renew-by-default --redirect --register-unsafely-without-email \ + --domain $PUBLIC_HOSTNAME --server $BOULDER_URL -v if [ $? -ne 0 ] ; then FAIL=1 fi +if [ "$OS_TYPE" = "ubuntu" ] ; then + venv/bin/tox -e apacheconftest +else + echo Not running hackish apache tests on $OS_TYPE +fi + +if [ $? -ne 0 ] ; then + FAIL=1 +fi + +sudo venv/bin/letsencrypt renew --renew-by-default + +if [ $? -ne 0 ] ; then + FAIL=1 +fi + + +ls /etc/letsencrypt/archive/$PUBLIC_HOSTNAME | grep -q 2.pem + +if [ $? -ne 0 ] ; then + FAIL=1 +fi + +# return error if any of the subtests failed if [ "$FAIL" = 1 ] ; then exit 1 fi From f623df772a9fd7d217d5de164183b153ee5e8548 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 4 Feb 2016 10:04:12 -0800 Subject: [PATCH 48/73] Experimental solution to deepcopy py26 problems --- letsencrypt/cli.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f4335c701..ba282ce49 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -840,11 +840,15 @@ def _renewal_conf_files(config): """Return /path/to/*.conf in the renewal conf directory""" return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) +def _rc_from_config(config): + ns = copy.deepcopy(config.namespace) + new_config = configuration.NamespaceConfig(ns) + return configuration.RenewerConfiguration(new_config) def renew(cli_config, unused_plugins): """Renew previously-obtained certificates.""" - cli_config = configuration.RenewerConfiguration(cli_config) - if cli_config.domains != []: + config = _rc_from_config(cli_config) + if config.domains != []: raise errors.Error("Currently, the renew verb is only capable of " "renewing all installed certificates that are due " "to be renewed; individual domains cannot be " @@ -852,13 +856,13 @@ def renew(cli_config, unused_plugins): "renew specific certificates, use the certonly " "command. The renew verb may provide other options " "for selecting certificates to renew in the future.") - for renewal_file in _renewal_conf_files(cli_config): + for renewal_file in _renewal_conf_files(config): if not renewal_file.endswith(".conf"): continue print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? - config = configuration.RenewerConfiguration(copy.deepcopy(cli_config)) + config = _rc_from_config(cli_config) config.noninteractive_mode = True # Note that this modifies config (to add back the configuration From ab2fed0e1d3c89273150a61adeddcfddf219fd05 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 4 Feb 2016 10:20:05 -0800 Subject: [PATCH 49/73] Lint --- letsencrypt/tests/cli_test.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index fd00c4465..393531c6e 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -32,9 +32,6 @@ CERT = test_util.vector_path('cert.pem') CSR = test_util.vector_path('csr.der') KEY = test_util.vector_path('rsa256_key.pem') -def hack(x): - return x - class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods """Tests for different commands.""" @@ -573,6 +570,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(mock_client.obtain_certificate.call_count, 0) except: self._dump_log() + raise return mock_lineage, mock_get_utility @@ -618,7 +616,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods rc = os.path.join(rd, "sample-renewal.conf") with open(rc, "w") as dest: dest.write(renewal_conf) - hack_copy.side_effect = hack + hack_copy.side_effect = lambda x: x args = ["renew", "--dry-run", "-tvv"] self._test_renewal_common(True, [], args=args, renew=True) From 375543eb3208bd8171d1709000c1965d8a71b4d1 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 4 Feb 2016 14:43:05 -0800 Subject: [PATCH 50/73] Hoping to see if integration test is really renewing --- tests/boulder-integration.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 5d9ed4859..8c5a93e39 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -51,7 +51,7 @@ common renew sed -i "4arenew_before_expiry = 10 years" "$root/conf/renewal/le1.wtf.conf" common renew -# letsencrypt-renewer $store_flags +ls "$root/conf/archive/le1.wtf" # dir="$root/conf/archive/le1.wtf" # for x in cert chain fullchain privkey; # do From e14feb2919ecdda071e743137f2d6c0fb3a6cd6b Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 4 Feb 2016 15:44:54 -0800 Subject: [PATCH 51/73] renew should imply noninteractive --- letsencrypt/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3a68eab99..88c23c24b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -859,7 +859,6 @@ def renew(cli_config, unused_plugins): # XXX: does this succeed in making a fully independent config object # each time? config = configuration.RenewerConfiguration(copy.deepcopy(cli_config)) - config.noninteractive_mode = True # Note that this modifies config (to add back the configuration # elements from within the renewal configuration file). @@ -1700,6 +1699,8 @@ def main(cli_args=sys.argv[1:]): displayer = display_util.NoninteractiveDisplay(sys.stdout) elif config.text_mode: displayer = display_util.FileDisplay(sys.stdout) + elif config.renew: + displayer = display_util.NoninteractiveDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() zope.component.provideUtility(displayer) From e2e0dddaa4d31ed7676d17ff709c2f77b4e4f7b1 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 4 Feb 2016 16:04:53 -0800 Subject: [PATCH 52/73] Responding to comments (logger.debug, reject --csr in renew) --- letsencrypt/cli.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3a68eab99..c43eeaadb 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -229,7 +229,7 @@ def _find_duplicative_certs(config, domains): candidate_lineage = storage.RenewableCert(renewal_file, cli_config) except (errors.CertStorageError, IOError): logger.warning("Renewal conf file %s is broken. Skipping.", renewal_file) - logger.info("Traceback was:\n%s", traceback.format_exc()) + logger.debug("Traceback was:\n%s", traceback.format_exc()) continue # TODO: Handle these differently depending on whether they are # expired or still valid? @@ -248,8 +248,10 @@ def _find_duplicative_certs(config, domains): def _treat_as_renewal(config, domains): - """Determine whether there are duplicated names and how to handle them - (renew, reinstall, newcert, or no action). + """Determine whether there are duplicated names and how to handle + them (renew, reinstall, newcert, or raising an error to stop + the client run if the user chooses to cancel the operation when + prompted). :returns: Two-element tuple containing desired new-certificate behavior as a string token ("reinstall", "renew", or "newcert"), plus either @@ -852,6 +854,10 @@ def renew(cli_config, unused_plugins): "renew specific certificates, use the certonly " "command. The renew verb may provide other options " "for selecting certificates to renew in the future.") + if cli_config.csr is not None: + raise errors.Error("Currently, the renew verb cannot be used when " + "specifying a CSR file. Please try the certonly " + "command instead.") for renewal_file in _renewal_conf_files(cli_config): if not renewal_file.endswith(".conf"): continue From 893918de00453a4552c64cc43eba1bdba14d8b66 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 4 Feb 2016 16:07:56 -0800 Subject: [PATCH 53/73] Check for config.verb == "renew" rather than config.renew --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5443e0ac7..9036804cb 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1705,7 +1705,7 @@ def main(cli_args=sys.argv[1:]): displayer = display_util.NoninteractiveDisplay(sys.stdout) elif config.text_mode: displayer = display_util.FileDisplay(sys.stdout) - elif config.renew: + elif config.verb == "renew": displayer = display_util.NoninteractiveDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() From 8dfb2a1d4c9ace1d8153a3d7a3b6c84916cba6d7 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 4 Feb 2016 16:09:42 -0800 Subject: [PATCH 54/73] check verb --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 88c23c24b..570484f46 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1699,7 +1699,7 @@ def main(cli_args=sys.argv[1:]): displayer = display_util.NoninteractiveDisplay(sys.stdout) elif config.text_mode: displayer = display_util.FileDisplay(sys.stdout) - elif config.renew: + elif config.verb == renew: displayer = display_util.NoninteractiveDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() From 7dd1ea4dcfc2160e616af4fe27e628b61c46758e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 17:30:52 -0800 Subject: [PATCH 55/73] Kill this now plz --- letsencrypt/configuration.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 04053c8c3..37eaba3bd 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -84,15 +84,10 @@ class RenewerConfiguration(object): def __init__(self, namespace): self.namespace = namespace - # We're done setting up the attic. Now pull up the ladder after ourselves... - self.__setattr__ = self.__setattr_implementation__ def __getattr__(self, name): return getattr(self.namespace, name) - def __setattr_implementation__(self, var, value): - return self.namespace.__setattr__(var, value) - @property def archive_dir(self): # pylint: disable=missing-docstring return os.path.join(self.namespace.config_dir, constants.ARCHIVE_DIR) From b2dae6cae27335276653ec6c466e1fe6a093cc00 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 18:10:39 -0800 Subject: [PATCH 56/73] Fixed it? --- letsencrypt/cli.py | 64 +++++++++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 204327e13..e4f841d50 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -727,7 +727,15 @@ def install(config, plugins): le_client.enhance_config(domains, config) -def _restore_required_config_elements(full_path, config, renewalparams): +def _restore_required_config_elements(config, renewalparams): + """Sets non-plugin specific values in config from renewalparams + + :param configuration.NamespaceConfig config: configuration for the + current lineage + :param configobj.Section renewalparams: Parameters from the renewal + configuration file that defines this lineage + + """ # string-valued items to add if they're present for config_item in STR_CONFIG_ITEMS: if config_item in renewalparams: @@ -744,12 +752,18 @@ def _restore_required_config_elements(full_path, config, renewalparams): value = int(renewalparams[config_item]) setattr(config.namespace, config_item, value) except ValueError: - logger.warning("Renewal configuration file %s specifies " - "a non-numeric value for %s. Skipping.", - full_path, config_item) - raise + raise errors.Error( + "Expected a numeric value for {0}".format(config_item)) def _restore_plugin_configs(config, renewalparams): + """Sets plugin specific values in config from renewalparams + + :param configuration.NamespaceConfig config: configuration for the + current lineage + :param configobj.Section renewalparams: Parameters from the renewal + configuration file that defines this lineage + + """ # Now use parser to get plugin-prefixed items with correct types # XXX: the current approach of extracting only prefixed items # related to the actually-used installer and authenticator @@ -782,7 +796,7 @@ def _restore_plugin_configs(config, renewalparams): return True -def _reconstitute(full_path, config): +def _reconstitute(config, full_path): """Try to instantiate a RenewableCert, updating config with relevant items. This is specifically for use in renewal and enforces several checks @@ -790,12 +804,18 @@ def _reconstitute(full_path, config): request. The config argument is modified by including relevant options read from the renewal configuration file. + :param configuration.NamespaceConfig config: configuration for the + current lineage + :param str full_path: Absolute path to the configuration file that + defines this lineage + :returns: the RenewableCert object or None if a fatal error occurred :rtype: `storage.RenewableCert` or NoneType - """ + """ try: - renewal_candidate = storage.RenewableCert(full_path, config) + renewal_candidate = storage.RenewableCert( + full_path, configuration.RenewerConfiguration(config)) except (errors.CertStorageError, IOError): logger.warning("Renewal configuration file %s is broken. Skipping.", full_path) logger.info("Traceback was:\n%s", traceback.format_exc()) @@ -812,11 +832,12 @@ def _reconstitute(full_path, config): # Now restore specific values along with their data types, if # those elements are present. try: - _restore_required_config_elements(full_path, config, renewalparams) + _restore_required_config_elements(config, renewalparams) _restore_plugin_configs(config, renewalparams) - except ValueError: - # There was a data type error which has already been - # logged. + except (ValueError, errors.Error) as error: + logger.warning( + "An error occured while parsing %s. The error was %s. " + "Skipping the file.", full_path, error.message) return None # webroot_map is, uniquely, a dict, and the general-purpose @@ -843,10 +864,9 @@ def _renewal_conf_files(config): return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) -def renew(cli_config, unused_plugins): +def renew(config, unused_plugins): """Renew previously-obtained certificates.""" - cli_config = configuration.RenewerConfiguration(cli_config) - if cli_config.domains != []: + if config.domains != []: raise errors.Error("Currently, the renew verb is only capable of " "renewing all installed certificates that are due " "to be renewed; individual domains cannot be " @@ -854,22 +874,24 @@ def renew(cli_config, unused_plugins): "renew specific certificates, use the certonly " "command. The renew verb may provide other options " "for selecting certificates to renew in the future.") - if cli_config.csr is not None: + if config.csr is not None: raise errors.Error("Currently, the renew verb cannot be used when " "specifying a CSR file. Please try the certonly " "command instead.") - for renewal_file in _renewal_conf_files(cli_config): + renewer_config = configuration.RenewerConfiguration(config) + for renewal_file in _renewal_conf_files(renewer_config): if not renewal_file.endswith(".conf"): continue print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? - config = configuration.RenewerConfiguration(copy.deepcopy(cli_config)) + lineage_config = configuration.RenewerConfiguration( + copy.deepcopy(config)) # Note that this modifies config (to add back the configuration # elements from within the renewal configuration file). try: - renewal_candidate = _reconstitute(renewal_file, config) + renewal_candidate = _reconstitute(renewal_file, lineage_config) except Exception as e: # pylint: disable=broad-except # reconstitute encountered an unanticipated problem. logger.warning("Renewal configuration file %s produced an " @@ -882,14 +904,14 @@ def renew(cli_config, unused_plugins): # already been logged. Go on to the next config. continue # XXX: ensure that each call here replaces the previous one - zope.component.provideUtility(config) + zope.component.provideUtility(lineage_config) print("Trying...") # Because obtain_cert itself indirectly decides whether to renew # or not, we couldn't currently make a UI/logging distinction at # this stage to indicate whether renewal was actually attempted # (or successful). - obtain_cert(config, plugins_disco.PluginsRegistry.find_all(), + obtain_cert(lineage_config, plugins_disco.PluginsRegistry.find_all(), renewal_candidate) def revoke(config, unused_plugins): # TODO: coop with renewal config From 36a42d18304a502393a769e5fb37025abd6a5232 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 18:15:39 -0800 Subject: [PATCH 57/73] Tracebacks are useful --- letsencrypt/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e4f841d50..2838e8395 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -838,6 +838,7 @@ def _reconstitute(config, full_path): logger.warning( "An error occured while parsing %s. The error was %s. " "Skipping the file.", full_path, error.message) + logger.debug("Traceback was:\n%s", traceback.format_exc()) return None # webroot_map is, uniquely, a dict, and the general-purpose From b4f1d94d096e735cbaeca35a381977a7460fabbe Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 18:21:36 -0800 Subject: [PATCH 58/73] less nesting + fixed argument order --- letsencrypt/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 2838e8395..370024a25 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -892,7 +892,7 @@ def renew(config, unused_plugins): # Note that this modifies config (to add back the configuration # elements from within the renewal configuration file). try: - renewal_candidate = _reconstitute(renewal_file, lineage_config) + renewal_candidate = _reconstitute(lineage_config, renewal_file) except Exception as e: # pylint: disable=broad-except # reconstitute encountered an unanticipated problem. logger.warning("Renewal configuration file %s produced an " @@ -912,7 +912,8 @@ def renew(config, unused_plugins): # or not, we couldn't currently make a UI/logging distinction at # this stage to indicate whether renewal was actually attempted # (or successful). - obtain_cert(lineage_config, plugins_disco.PluginsRegistry.find_all(), + obtain_cert(lineage_config.namespace, + plugins_disco.PluginsRegistry.find_all(), renewal_candidate) def revoke(config, unused_plugins): # TODO: coop with renewal config From 8c4721531886499f5d9f7c86590b706b56b74e66 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 18:28:10 -0800 Subject: [PATCH 59/73] More unnesting --- letsencrypt/cli.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 370024a25..14298a645 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -886,8 +886,7 @@ def renew(config, unused_plugins): print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? - lineage_config = configuration.RenewerConfiguration( - copy.deepcopy(config)) + lineage_config = copy.deepcopy(config) # Note that this modifies config (to add back the configuration # elements from within the renewal configuration file). @@ -912,8 +911,7 @@ def renew(config, unused_plugins): # or not, we couldn't currently make a UI/logging distinction at # this stage to indicate whether renewal was actually attempted # (or successful). - obtain_cert(lineage_config.namespace, - plugins_disco.PluginsRegistry.find_all(), + obtain_cert(lineage_config, plugins_disco.PluginsRegistry.find_all(), renewal_candidate) def revoke(config, unused_plugins): # TODO: coop with renewal config From 8933c51e22921f0c7597b455d82865fbe2171a8f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 18:32:39 -0800 Subject: [PATCH 60/73] Satisfied OCD by keeping comment capitalization consistent --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 14298a645..82bce0f21 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -732,7 +732,7 @@ def _restore_required_config_elements(config, renewalparams): :param configuration.NamespaceConfig config: configuration for the current lineage - :param configobj.Section renewalparams: Parameters from the renewal + :param configobj.Section renewalparams: parameters from the renewal configuration file that defines this lineage """ From 9cd0d5497f13d283cfb6eca0ab15a9e26deb09a8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 4 Feb 2016 18:58:57 -0800 Subject: [PATCH 61/73] Remove older workarounds, comment newer ones --- letsencrypt/cli.py | 2 ++ letsencrypt/tests/cli_test.py | 7 +------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 77ac9be37..471f4a2fd 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -865,6 +865,8 @@ def _renewal_conf_files(config): return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) def _copy_nsconfig(config): + # Work around https://bugs.python.org/issue1515 for py26 tests :( :( + # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276 ns = copy.deepcopy(config.namespace) new_config = configuration.NamespaceConfig(ns) return new_config diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 393531c6e..a5757399e 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -603,11 +603,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods print lf.read() - # Work around https://bugs.python.org/issue1515 for py26 tests :( :( - # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276 - @mock.patch('letsencrypt.cli.copy.deepcopy') - def test_renewal_verb(self, hack_copy): - + def test_renewal_verb(self): with open(test_util.vector_path('sample-renewal.conf')) as src: # put the correct path for cert.pem, chain.pem etc in the renewal conf renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path()) @@ -616,7 +612,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods rc = os.path.join(rd, "sample-renewal.conf") with open(rc, "w") as dest: dest.write(renewal_conf) - hack_copy.side_effect = lambda x: x args = ["renew", "--dry-run", "-tvv"] self._test_renewal_common(True, [], args=args, renew=True) From 9f57236bb124314a1c9f773e0fc0bc7bd3ed4830 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 4 Feb 2016 19:14:36 -0800 Subject: [PATCH 62/73] Try things the EvenMoreProper way --- letsencrypt/cli.py | 8 +------- letsencrypt/configuration.py | 7 +++++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 471f4a2fd..fd568afa2 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -864,12 +864,6 @@ def _renewal_conf_files(config): """Return /path/to/*.conf in the renewal conf directory""" return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) -def _copy_nsconfig(config): - # Work around https://bugs.python.org/issue1515 for py26 tests :( :( - # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276 - ns = copy.deepcopy(config.namespace) - new_config = configuration.NamespaceConfig(ns) - return new_config def renew(config, unused_plugins): """Renew previously-obtained certificates.""" @@ -893,7 +887,7 @@ def renew(config, unused_plugins): print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? - lineage_config = _copy_nsconfig(config) + lineage_config = copy.deepcopy(config) # Note that this modifies config (to add back the configuration # elements from within the renewal configuration file). diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 37eaba3bd..2bbf1b019 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -1,4 +1,5 @@ """Let's Encrypt user-supplied configuration.""" +import copy import os import urlparse @@ -78,6 +79,12 @@ class NamespaceConfig(object): return os.path.join( self.namespace.work_dir, constants.TEMP_CHECKPOINT_DIR) + def __deepcopy__(self, _memo): + # Work around https://bugs.python.org/issue1515 for py26 tests :( :( + # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276 + new_ns = copy.deepcopy(self.namespace) + return type(self)(new_ns) + class RenewerConfiguration(object): """Configuration wrapper for renewer.""" From 0b62495581327fced5a30b60306d8257b448ee15 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 4 Feb 2016 19:25:50 -0800 Subject: [PATCH 63/73] Move noninteractivity to the place Brad suggests --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7cfdbb9bb..141c19fe1 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -879,7 +879,6 @@ def renew(config, unused_plugins): raise errors.Error("Currently, the renew verb cannot be used when " "specifying a CSR file. Please try the certonly " "command instead.") - config.noninteractive_mode = True renewer_config = configuration.RenewerConfiguration(config) for renewal_file in _renewal_conf_files(renewer_config): if not renewal_file.endswith(".conf"): @@ -1729,6 +1728,7 @@ def main(cli_args=sys.argv[1:]): elif config.text_mode: displayer = display_util.FileDisplay(sys.stdout) elif config.verb == "renew": + config.noninteractive_mode = True displayer = display_util.NoninteractiveDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() From 3260efd519d77e4a056e1cfd6c21174c9edb2838 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 4 Feb 2016 19:31:05 -0800 Subject: [PATCH 64/73] Be consistent about using logger.debug for tracebacks --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 141c19fe1..1ab23ea76 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -818,7 +818,7 @@ def _reconstitute(config, full_path): full_path, configuration.RenewerConfiguration(config)) except (errors.CertStorageError, IOError): logger.warning("Renewal configuration file %s is broken. Skipping.", full_path) - logger.info("Traceback was:\n%s", traceback.format_exc()) + logger.debug("Traceback was:\n%s", traceback.format_exc()) return None if "renewalparams" not in renewal_candidate.configuration: logger.warning("Renewal configuration file %s lacks " @@ -896,7 +896,7 @@ def renew(config, unused_plugins): # reconstitute encountered an unanticipated problem. logger.warning("Renewal configuration file %s produced an " "unexpected error: %s. Skipping.", renewal_file, e) - logger.info("Traceback was:\n%s", traceback.format_exc()) + logger.debug("Traceback was:\n%s", traceback.format_exc()) continue if renewal_candidate is None: From b7717bbc8e5e524f2972d616ca8df2fca75a2e7d Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 13:06:50 -0800 Subject: [PATCH 65/73] Fixes for comments from PR review --- letsencrypt/cli.py | 47 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1ab23ea76..e51490379 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -773,27 +773,25 @@ def _restore_plugin_configs(config, renewalparams): # XXX: is it true that an item will end up in _parser._actions even # when no action was explicitly specified? plugin_prefixes = [renewalparams["authenticator"]] - if "installer" in renewalparams and renewalparams["installer"] != None: + if renewalparams.get("installer", None) is not None: plugin_prefixes.append(renewalparams["installer"]) for plugin_prefix in set(renewalparams): - for config_item in renewalparams.keys(): - if renewalparams[config_item] == "None": + for config_item, config_value in renewalparams.iteritems(): + if config_item.startswith(plugin_prefix + "_"): # Avoid confusion when, for example, "csr = None" (avoid # trying to read the file called "None") # Should we omit the item entirely rather than setting # its value to None? - setattr(config.namespace, config_item, None) - continue - if config_item.startswith(plugin_prefix + "_"): + if config_value == "None": + setattr(config.namespace, config_item, None) + continue for action in _parser.parser._actions: # pylint: disable=protected-access if action.type is not None and action.dest == config_item: setattr(config.namespace, config_item, - action.type(renewalparams[config_item])) + action.type(config_value)) break else: - setattr(config.namespace, config_item, - str(renewalparams[config_item])) - return True + setattr(config.namespace, config_item, str(config_value)) def _reconstitute(config, full_path): @@ -881,8 +879,6 @@ def renew(config, unused_plugins): "command instead.") renewer_config = configuration.RenewerConfiguration(config) for renewal_file in _renewal_conf_files(renewer_config): - if not renewal_file.endswith(".conf"): - continue print("Processing " + renewal_file) # XXX: does this succeed in making a fully independent config object # each time? @@ -899,20 +895,21 @@ def renew(config, unused_plugins): logger.debug("Traceback was:\n%s", traceback.format_exc()) continue - if renewal_candidate is None: - # reconstitute indicated an error or problem which has - # already been logged. Go on to the next config. - continue - # XXX: ensure that each call here replaces the previous one - zope.component.provideUtility(lineage_config) + if renewal_candidate is not None: + # _reconstitute succeeded in producing a RenewableCert, so we + # have something to work with from this particular config file. + + # XXX: ensure that each call here replaces the previous one + zope.component.provideUtility(lineage_config) + print("Trying...") + # Because obtain_cert itself indirectly decides whether to renew + # or not, we couldn't currently make a UI/logging distinction at + # this stage to indicate whether renewal was actually attempted + # (or successful). + obtain_cert(lineage_config, + plugins_disco.PluginsRegistry.find_all(), + renewal_candidate) - print("Trying...") - # Because obtain_cert itself indirectly decides whether to renew - # or not, we couldn't currently make a UI/logging distinction at - # this stage to indicate whether renewal was actually attempted - # (or successful). - obtain_cert(lineage_config, plugins_disco.PluginsRegistry.find_all(), - renewal_candidate) def revoke(config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" From 5c14c09027aba71ef02d7b93f7f9974498eadb3e Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 13:10:00 -0800 Subject: [PATCH 66/73] @bmw noticed we were iterating over the wrong thing! --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e51490379..88d16be73 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -775,7 +775,7 @@ def _restore_plugin_configs(config, renewalparams): plugin_prefixes = [renewalparams["authenticator"]] if renewalparams.get("installer", None) is not None: plugin_prefixes.append(renewalparams["installer"]) - for plugin_prefix in set(renewalparams): + for plugin_prefix in set(plugin_prefixes): for config_item, config_value in renewalparams.iteritems(): if config_item.startswith(plugin_prefix + "_"): # Avoid confusion when, for example, "csr = None" (avoid From 5c31b000b40566e2a5a552624ab82db96b02278f Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 14:51:14 -0800 Subject: [PATCH 67/73] Error handling around obtain_cert() --- letsencrypt/cli.py | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 88d16be73..abe4ccc0c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -770,8 +770,10 @@ def _restore_plugin_configs(config, renewalparams): # works as long as plugins don't need to read plugin-specific # variables set by someone else (e.g., assuming Apache # configurator doesn't need to read webroot_ variables). - # XXX: is it true that an item will end up in _parser._actions even - # when no action was explicitly specified? + # Note: if a parameter that used to be defined in the parser is no + # longer defined, stored copies of that parameter will be + # deserialized as strings by this logic even if they were + # originally meant to be some other type. plugin_prefixes = [renewalparams["authenticator"]] if renewalparams.get("installer", None) is not None: plugin_prefixes.append(renewalparams["installer"]) @@ -895,20 +897,26 @@ def renew(config, unused_plugins): logger.debug("Traceback was:\n%s", traceback.format_exc()) continue - if renewal_candidate is not None: - # _reconstitute succeeded in producing a RenewableCert, so we - # have something to work with from this particular config file. + try: + if renewal_candidate is not None: + # _reconstitute succeeded in producing a RenewableCert, so we + # have something to work with from this particular config file. - # XXX: ensure that each call here replaces the previous one - zope.component.provideUtility(lineage_config) - print("Trying...") - # Because obtain_cert itself indirectly decides whether to renew - # or not, we couldn't currently make a UI/logging distinction at - # this stage to indicate whether renewal was actually attempted - # (or successful). - obtain_cert(lineage_config, - plugins_disco.PluginsRegistry.find_all(), - renewal_candidate) + # XXX: ensure that each call here replaces the previous one + zope.component.provideUtility(lineage_config) + print("Trying...") + # Because obtain_cert itself indirectly decides whether to renew + # or not, we couldn't currently make a UI/logging distinction at + # this stage to indicate whether renewal was actually attempted + # (or successful). + obtain_cert(lineage_config, + plugins_disco.PluginsRegistry.find_all(), + renewal_candidate) + except Exception as e: # pylint: disable=broad-except + # obtain_cert (presumably) encountered an unanticipated problem. + logger.warning("Attempting to renew cert from %s produced an " + "unexpected error: %s. Skipping.", renewal_file, e) + logger.debug("Traceback was:\n%s", traceback.format_exc()) def revoke(config, unused_plugins): # TODO: coop with renewal config From 21fe41c53b6cb8847680715497fef54614076ff2 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 15:37:50 -0800 Subject: [PATCH 68/73] call .restart() on installer after renew if possible --- letsencrypt/cli.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index abe4ccc0c..bc9777ad6 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -704,6 +704,12 @@ def obtain_cert(config, plugins, lineage=None): if config.dry_run: _report_successful_dry_run() + elif config.verb == "renew" and installer is not None: + # In case of a renewal, reload server to pick up new certificate. + # In principle we could have a configuration option to inhibit this + # from happening. + installer.restart() + print("reloaded") _suggest_donation_if_appropriate(config) From 772d424fc791937647912b4988a68810804ed4c1 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 15:38:10 -0800 Subject: [PATCH 69/73] =?UTF-8?q?log=5Fdir=20=E2=86=92=20logs=5Fdir?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bc9777ad6..d6c4ce930 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -54,7 +54,7 @@ _parser = None # file's renewalparams and actually used in the client configuration # during the renewal process. We have to record their types here because # the renewal configuration process loses this information. -STR_CONFIG_ITEMS = ["config_dir", "log_dir", "work_dir", "user_agent", +STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent", "server", "account", "authenticator", "installer", "standalone_supported_challenges"] INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] From 7eb2bb4d037bd64ce4e31e3de303a968cc45db11 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 16:30:19 -0800 Subject: [PATCH 70/73] Fix renew + noninteractive --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 63c6d96f8..838da4015 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1621,7 +1621,7 @@ def setup_log_file_handler(config, logfile, fmt): def _cli_log_handler(config, level, fmt): - if config.text_mode or config.noninteractive_mode: + if config.text_mode or config.noninteractive_mode or config.verb == "renew": handler = colored_logging.StreamHandler() handler.setFormatter(logging.Formatter(fmt)) else: From 09337517d3cb24ad8606ec3bc37f289e5f7a65e7 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 16:57:41 -0800 Subject: [PATCH 71/73] Try to distinguish renew and non-renew in integration test --- tests/boulder-integration.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 8c5a93e39..294522e05 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -44,12 +44,13 @@ common --domains le3.wtf install \ --cert-path "${root}/csr/cert.pem" \ --key-path "${root}/csr/key.pem" -# This won't renew (because it's not time yet) -common renew +# This won't renew (because it's not time yet) - not using common because +# common forces renewal +letsencrypt_test --authenticator standalone --installer null renew # This will renew sed -i "4arenew_before_expiry = 10 years" "$root/conf/renewal/le1.wtf.conf" -common renew +letsencrypt_test --authenticator standalone --installer null renew ls "$root/conf/archive/le1.wtf" # dir="$root/conf/archive/le1.wtf" From 8b02f485b02e2170a140a5fc07bacf5e1916e658 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 17:13:30 -0800 Subject: [PATCH 72/73] Have a way not to force renewal in integration test --- tests/boulder-integration.sh | 16 +++++++++++----- tests/integration/_common.sh | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 294522e05..b6c76ee22 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -27,6 +27,13 @@ common() { "$@" } +common_no_force_renew() { + letsencrypt_test_no_force_renew \ + --authenticator standalone \ + --installer null \ + "$@" +} + common --domains le1.wtf --standalone-supported-challenges tls-sni-01 auth common --domains le2.wtf --standalone-supported-challenges http-01 run common -a manual -d le.wtf auth @@ -44,13 +51,12 @@ common --domains le3.wtf install \ --cert-path "${root}/csr/cert.pem" \ --key-path "${root}/csr/key.pem" -# This won't renew (because it's not time yet) - not using common because -# common forces renewal -letsencrypt_test --authenticator standalone --installer null renew +# This won't renew (because it's not time yet) +letsencrypt_test_no_force_renew --authenticator standalone --installer null renew -# This will renew +# This will renew because the expiry is less than 10 years from now sed -i "4arenew_before_expiry = 10 years" "$root/conf/renewal/le1.wtf.conf" -letsencrypt_test --authenticator standalone --installer null renew +letsencrypt_test_no_force_renew --authenticator standalone --installer null renew ls "$root/conf/archive/le1.wtf" # dir="$root/conf/archive/le1.wtf" diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index 4572b0fb3..f133600a0 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -28,3 +28,21 @@ letsencrypt_test () { -vvvvvvv \ "$@" } + +letsencrypt_test_no_force_renew () { + letsencrypt \ + --server "${SERVER:-http://localhost:4000/directory}" \ + --no-verify-ssl \ + --tls-sni-01-port 5001 \ + --http-01-port 5002 \ + --manual-test-mode \ + $store_flags \ + --text \ + --no-redirect \ + --agree-tos \ + --register-unsafely-without-email \ + --renew-by-default \ + --debug \ + -vvvvvvv \ + "$@" +} From fd3d2fa8224b85179f7a278e78fdf1bd90b951df Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Feb 2016 17:19:39 -0800 Subject: [PATCH 73/73] Make _no_force_renew not force renewal --- tests/integration/_common.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index f133600a0..9230cc682 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -41,7 +41,6 @@ letsencrypt_test_no_force_renew () { --no-redirect \ --agree-tos \ --register-unsafely-without-email \ - --renew-by-default \ --debug \ -vvvvvvv \ "$@"