From fa49976baf67449fb3ac2b60ac2fd51e4f9d5444 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 16:05:39 -0800 Subject: [PATCH 01/76] Remove need to use --debug with py26 --- letsencrypt-auto-source/letsencrypt-auto | 4 +--- letsencrypt-auto-source/letsencrypt-auto.template | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 7800a5eb6..5f16f55a8 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -105,9 +105,7 @@ DeterminePythonVersion() { fi PYVER=`"$LE_PYTHON" --version 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//'` - if [ $PYVER -eq 26 ]; then - ExperimentalBootstrap "Python 2.6" - elif [ $PYVER -lt 26 ]; then + if [ $PYVER -lt 26 ]; then echo "You have an ancient version of Python entombed in your operating system..." echo "This isn't going to work; you'll need at least version 2.6." exit 1 diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template index ba17d7a99..de4844c9e 100755 --- a/letsencrypt-auto-source/letsencrypt-auto.template +++ b/letsencrypt-auto-source/letsencrypt-auto.template @@ -105,9 +105,7 @@ DeterminePythonVersion() { fi PYVER=`"$LE_PYTHON" --version 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//'` - if [ $PYVER -eq 26 ]; then - ExperimentalBootstrap "Python 2.6" - elif [ $PYVER -lt 26 ]; then + if [ $PYVER -lt 26 ]; then echo "You have an ancient version of Python entombed in your operating system..." echo "This isn't going to work; you'll need at least version 2.6." exit 1 From 4e149225c8a19904855734ebe75321beb1b80ff0 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 16:17:19 -0800 Subject: [PATCH 02/76] Remove need for --debug in py26 with old le-auto --- letsencrypt-auto | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index 20465dbb1..2b956aaf5 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -101,9 +101,7 @@ DeterminePythonVersion() { fi PYVER=`$LE_PYTHON --version 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//'` - if [ $PYVER -eq 26 ] ; then - ExperimentalBootstrap "Python 2.6" - elif [ $PYVER -lt 26 ] ; then + if [ $PYVER -lt 26 ] ; then echo "You have an ancient version of Python entombed in your operating system..." echo "This isn't going to work; you'll need at least version 2.6." exit 1 From 273a78a5c63cdb7337c218aeb5fdd8173387b91b Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 15:02:15 -0800 Subject: [PATCH 03/76] 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 3ea1c499f43f79d66ebd5473793889be3bd633d4 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 15:20:49 -0800 Subject: [PATCH 04/76] 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 05/76] 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 06/76] 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 07/76] 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 81b3a98346051ed7fad67d922515df41ac644a3e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 2 Feb 2016 17:23:01 -0800 Subject: [PATCH 08/76] Added write_renewal_config function --- letsencrypt/storage.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index e41805459..5da9de932 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -50,6 +50,43 @@ def add_time_interval(base_time, interval, textparser=parsedatetime.Calendar()): return textparser.parseDT(interval, base_time, tzinfo=tzinfo)[0] +def write_renewal_config(filename, target, cli_config): + """Writes a renewal config file with the specified name and values. + + :param str filename: Absolute path to the config file + :param dict target: Maps ALL_FOUR to their symlink paths + :param .RenewerConfiguration cli_config: parsed command line + arguments + + :returns: Configuration object for the new config file + :rtype: configobj.ConfigObj + + """ + # create_empty creates a new config file if filename does not exist + config = configobj.ConfigObj(filename, create_empty=True) + for kind in ALL_FOUR: + config[kind] = target[kind] + + # XXX: We clearly need a more general and correct way of getting + # options into the configobj for the RenewableCert instance. + # This is a quick-and-dirty way to do it to allow integration + # testing to start. (Note that the config parameter to new_lineage + # ideally should be a ConfigObj, but in this case a dict will be + # accepted in practice.) + renewalparams = vars(cli_config.namespace) + if renewalparams: + config["renewalparams"] = renewalparams + config.comments["renewalparams"] = ["", + "Options and defaults used" + " in the renewal process"] + + # TODO: add human-readable comments explaining other available + # parameters + logger.debug("Writing new config %s.", filename) + config.write() + return config + + class RenewableCert(object): # pylint: disable=too-many-instance-attributes """Renewable certificate. From b97ddb92f06f5a8c1dc4fa321466ff38862ca138 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 2 Feb 2016 17:27:40 -0800 Subject: [PATCH 09/76] 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 10/76] 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 11/76] 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 12/76] 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 13/76] 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 14/76] 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 c818b4f68924c793a37ce3d77325e04153b3a4f1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 2 Feb 2016 18:02:31 -0800 Subject: [PATCH 15/76] Refactor new_lineage --- letsencrypt/client.py | 18 ++++------------ letsencrypt/storage.py | 35 +++++-------------------------- letsencrypt/tests/renewer_test.py | 27 +++++++++--------------- 3 files changed, 19 insertions(+), 61 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index b7d486ba1..d8b8ca8c1 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -282,23 +282,13 @@ class Client(object): """ certr, chain, key, _ = self.obtain_certificate(domains) - # XXX: We clearly need a more general and correct way of getting - # options into the configobj for the RenewableCert instance. - # This is a quick-and-dirty way to do it to allow integration - # testing to start. (Note that the config parameter to new_lineage - # ideally should be a ConfigObj, but in this case a dict will be - # accepted in practice.) - params = vars(self.config.namespace) - config = {} - cli_config = configuration.RenewerConfiguration(self.config.namespace) - - if (cli_config.config_dir != constants.CLI_DEFAULTS["config_dir"] or - cli_config.work_dir != constants.CLI_DEFAULTS["work_dir"]): + if (self.config.config_dir != constants.CLI_DEFAULTS["config_dir"] or + self.config.work_dir != constants.CLI_DEFAULTS["work_dir"]): logger.warning( "Non-standard path(s), might not work with crontab installed " "by your operating system package manager") - if cli_config.dry_run: + if self.config.dry_run: logger.info("Dry run: Skipping creating new lineage for %s", domains[0]) return None @@ -307,7 +297,7 @@ class Client(object): domains[0], OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped), key.pem, crypto_util.dump_pyopenssl_chain(chain), - params, config, cli_config) + configuration.RenewerConfiguration(self.config.namespace)) def save_certificate(self, certr, chain_cert, cert_path, chain_path, fullchain_path): diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 5da9de932..8cc26d5b5 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -626,9 +626,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes return False @classmethod - def new_lineage(cls, lineagename, cert, privkey, chain, - renewalparams=None, config=None, cli_config=None): - # pylint: disable=too-many-locals,too-many-arguments + def new_lineage(cls, lineagename, cert, privkey, chain, cli_config): + # pylint: disable=too-many-locals """Create a new certificate lineage. Attempts to create a certificate lineage -- enrolled for @@ -648,26 +647,13 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :param str cert: the initial certificate version in PEM format :param str privkey: the private key in PEM format :param str chain: the certificate chain in PEM format - :param configobj.ConfigObj renewalparams: parameters that - should be used when instantiating authenticator and installer - objects in the future to attempt to renew this cert or deploy - new versions of it - :param configobj.ConfigObj config: renewal configuration - defaults, affecting, for example, the locations of the - directories where the associated files will be saved :param .RenewerConfiguration cli_config: parsed command line arguments :returns: the newly-created RenewalCert object - :rtype: :class:`storage.renewableCert`""" - - config = config_with_defaults(config) - # This attempts to read the renewer config file and augment or replace - # the renewer defaults with any options contained in that file. If - # renewer_config_file is undefined or if the file is nonexistent or - # empty, this .merge() will have no effect. - config.merge(configobj.ConfigObj(cli_config.renewer_config_file)) + :rtype: :class:`storage.renewableCert` + """ # Examine the configuration and find the new lineage's name for i in (cli_config.renewal_configs_dir, cli_config.archive_dir, cli_config.live_dir): @@ -722,18 +708,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Document what we've done in a new renewal config file config_file.close() - new_config = configobj.ConfigObj(config_filename, create_empty=True) - for kind in ALL_FOUR: - new_config[kind] = target[kind] - if renewalparams: - new_config["renewalparams"] = renewalparams - new_config.comments["renewalparams"] = ["", - "Options and defaults used" - " in the renewal process"] - # TODO: add human-readable comments explaining other available - # parameters - logger.debug("Writing new config %s.", config_filename) - new_config.write() + new_config = write_renewal_config(config_filename, target, cli_config) return cls(new_config.filename, cli_config) def save_successor(self, prior_version, new_cert, new_privkey, new_chain): diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 3c8e3cb95..0628fc073 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -553,8 +553,7 @@ class RenewableCertTests(BaseRenewableCertTest): """Test for new_lineage() class method.""" from letsencrypt import storage result = storage.RenewableCert.new_lineage( - "the-lineage.com", "cert", "privkey", "chain", None, - self.defaults, self.cli_config) + "the-lineage.com", "cert", "privkey", "chain", self.cli_config) # This consistency check tests most relevant properties about the # newly created cert lineage. # pylint: disable=protected-access @@ -565,27 +564,23 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(f.read(), "cert" + "chain") # Let's do it again and make sure it makes a different lineage result = storage.RenewableCert.new_lineage( - "the-lineage.com", "cert2", "privkey2", "chain2", None, - self.defaults, self.cli_config) + "the-lineage.com", "cert2", "privkey2", "chain2", self.cli_config) self.assertTrue(os.path.exists(os.path.join( self.cli_config.renewal_configs_dir, "the-lineage.com-0001.conf"))) # Now trigger the detection of already existing files os.mkdir(os.path.join( self.cli_config.live_dir, "the-lineage.com-0002")) self.assertRaises(errors.CertStorageError, - storage.RenewableCert.new_lineage, - "the-lineage.com", "cert3", "privkey3", "chain3", - None, self.defaults, self.cli_config) + storage.RenewableCert.new_lineage, "the-lineage.com", + "cert3", "privkey3", "chain3", self.cli_config) os.mkdir(os.path.join(self.cli_config.archive_dir, "other-example.com")) self.assertRaises(errors.CertStorageError, storage.RenewableCert.new_lineage, - "other-example.com", "cert4", "privkey4", "chain4", - None, self.defaults, self.cli_config) + "other-example.com", "cert4", + "privkey4", "chain4", self.cli_config) # Make sure it can accept renewal parameters - params = {"stuff": "properties of stuff", "great": "awesome"} result = storage.RenewableCert.new_lineage( - "the-lineage.com", "cert2", "privkey2", "chain2", - params, self.defaults, self.cli_config) + "the-lineage.com", "cert2", "privkey2", "chain2", self.cli_config) # TODO: Conceivably we could test that the renewal parameters actually # got saved @@ -597,8 +592,7 @@ class RenewableCertTests(BaseRenewableCertTest): shutil.rmtree(self.cli_config.live_dir) storage.RenewableCert.new_lineage( - "the-lineage.com", "cert2", "privkey2", "chain2", - None, self.defaults, self.cli_config) + "the-lineage.com", "cert2", "privkey2", "chain2", self.cli_config) self.assertTrue(os.path.exists( os.path.join( self.cli_config.renewal_configs_dir, "the-lineage.com.conf"))) @@ -612,9 +606,8 @@ class RenewableCertTests(BaseRenewableCertTest): from letsencrypt import storage mock_uln.return_value = "this_does_not_end_with_dot_conf", "yikes" self.assertRaises(errors.CertStorageError, - storage.RenewableCert.new_lineage, - "example.com", "cert", "privkey", "chain", - None, self.defaults, self.cli_config) + storage.RenewableCert.new_lineage, "example.com", + "cert", "privkey", "chain", self.cli_config) def test_bad_kind(self): self.assertRaises( From 6682e370a8a0041f9bb4274117c7ec5f4ccc1399 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 2 Feb 2016 18:28:04 -0800 Subject: [PATCH 16/76] 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 17/76] 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 18/76] 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 19/76] 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 20/76] 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 21/76] 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 22/76] 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 23/76] 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 24/76] 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 25/76] 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 26/76] 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 27/76] _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 28/76] 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 29/76] 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 04be86ce4b9d541595d3f84ab56609a92152fa7e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 14:49:48 -0800 Subject: [PATCH 30/76] Remove entry point --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index b51b53b18..b094e2d04 100644 --- a/setup.py +++ b/setup.py @@ -130,7 +130,6 @@ setup( entry_points={ 'console_scripts': [ 'letsencrypt = letsencrypt.cli:main', - 'letsencrypt-renewer = letsencrypt.renewer:main', ], 'letsencrypt.plugins': [ 'manual = letsencrypt.plugins.manual:Authenticator', From fd4d390ac2bfeba011d3ef6e2b15ebfe4fd27066 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 14:52:23 -0800 Subject: [PATCH 31/76] No renewer test --- tests/boulder-integration.sh | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 53996cd20..fd8233694 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -44,21 +44,6 @@ 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 - # revoke by account key common revoke --cert-path "$root/conf/live/le.wtf/cert.pem" # revoke renewed From d4222ea6b6f92ef0290698d6ef79f0dd8c976827 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 14:54:45 -0800 Subject: [PATCH 32/76] Remove renewer docs --- docs/api/renewer.rst | 5 ----- docs/conf.py | 2 -- docs/man/letsencrypt-renewer.rst | 1 - 3 files changed, 8 deletions(-) delete mode 100644 docs/api/renewer.rst delete mode 100644 docs/man/letsencrypt-renewer.rst diff --git a/docs/api/renewer.rst b/docs/api/renewer.rst deleted file mode 100644 index cc42c6eab..000000000 --- a/docs/api/renewer.rst +++ /dev/null @@ -1,5 +0,0 @@ -:mod:`letsencrypt.renewer` --------------------------- - -.. automodule:: letsencrypt.renewer - :members: diff --git a/docs/conf.py b/docs/conf.py index 21bcc6817..739d6ee43 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -281,8 +281,6 @@ man_pages = [ [project], 7), ('man/letsencrypt', 'letsencrypt', u'letsencrypt script documentation', [project], 1), - ('man/letsencrypt-renewer', 'letsencrypt-renewer', - u'letsencrypt-renewer script documentation', [project], 1), ] # If true, show URL addresses after external links. diff --git a/docs/man/letsencrypt-renewer.rst b/docs/man/letsencrypt-renewer.rst deleted file mode 100644 index 8fd232fa8..000000000 --- a/docs/man/letsencrypt-renewer.rst +++ /dev/null @@ -1 +0,0 @@ -.. program-output:: letsencrypt-renewer --help From cc1638a1af8e223eb26d644e74791beb1116193a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 14:59:07 -0800 Subject: [PATCH 33/76] Remove renewer tests --- letsencrypt/tests/renewer_test.py | 110 ------------------------------ 1 file changed, 110 deletions(-) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 3c8e3cb95..6e63281df 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -9,8 +9,6 @@ import unittest import configobj import mock -from acme import jose - from letsencrypt import configuration from letsencrypt import errors from letsencrypt.storage import ALL_FOUR @@ -681,114 +679,6 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(storage.add_time_interval(base_time, interval), excepted) - @mock.patch("letsencrypt.renewer.plugins_disco") - @mock.patch("letsencrypt.account.AccountFileStorage") - @mock.patch("letsencrypt.client.Client") - def test_renew(self, mock_c, mock_acc_storage, mock_pd): - from letsencrypt import renewer - - test_cert = test_util.load_vector("cert-san.pem") - for kind in ALL_FOUR: - os.symlink(os.path.join("..", "..", "archive", "example.org", - kind + "1.pem"), - getattr(self.test_rc, kind)) - fill_with_sample_data(self.test_rc) - with open(self.test_rc.cert, "w") as f: - f.write(test_cert) - - # Fails because renewalparams are missing - self.assertFalse(renewer.renew(self.test_rc, 1)) - self.test_rc.configfile["renewalparams"] = {"some": "stuff"} - # Fails because there's no authenticator specified - self.assertFalse(renewer.renew(self.test_rc, 1)) - self.test_rc.configfile["renewalparams"]["rsa_key_size"] = "2048" - self.test_rc.configfile["renewalparams"]["server"] = "acme.example.com" - self.test_rc.configfile["renewalparams"]["authenticator"] = "fake" - self.test_rc.configfile["renewalparams"]["tls_sni_01_port"] = "4430" - self.test_rc.configfile["renewalparams"]["http01_port"] = "1234" - self.test_rc.configfile["renewalparams"]["account"] = "abcde" - self.test_rc.configfile["renewalparams"]["domains"] = ["example.com"] - self.test_rc.configfile["renewalparams"]["config_dir"] = "config" - self.test_rc.configfile["renewalparams"]["work_dir"] = "work" - self.test_rc.configfile["renewalparams"]["logs_dir"] = "logs" - mock_auth = mock.MagicMock() - mock_pd.PluginsRegistry.find_all.return_value = {"apache": mock_auth} - # Fails because "fake" != "apache" - self.assertFalse(renewer.renew(self.test_rc, 1)) - self.test_rc.configfile["renewalparams"]["authenticator"] = "apache" - mock_client = mock.MagicMock() - # pylint: disable=star-args - comparable_cert = jose.ComparableX509(CERT) - mock_client.obtain_certificate.return_value = ( - mock.MagicMock(body=comparable_cert), [comparable_cert], - mock.Mock(pem="key"), mock.sentinel.csr) - mock_c.return_value = mock_client - self.assertEqual(2, renewer.renew(self.test_rc, 1)) - # TODO: We could also make several assertions about calls that should - # have been made to the mock functions here. - mock_acc_storage().load.assert_called_once_with(account_id="abcde") - mock_client.obtain_certificate.return_value = ( - mock.sentinel.certr, [], mock.sentinel.key, mock.sentinel.csr) - # This should fail because the renewal itself appears to fail - self.assertFalse(renewer.renew(self.test_rc, 1)) - - def _common_cli_args(self): - return [ - "--config-dir", self.cli_config.config_dir, - "--work-dir", self.cli_config.work_dir, - "--logs-dir", self.cli_config.logs_dir, - ] - - @mock.patch("letsencrypt.renewer.notify") - @mock.patch("letsencrypt.storage.RenewableCert") - @mock.patch("letsencrypt.renewer.renew") - def test_main(self, mock_renew, mock_rc, mock_notify): - from letsencrypt import renewer - mock_rc_instance = mock.MagicMock() - mock_rc_instance.should_autodeploy.return_value = True - mock_rc_instance.should_autorenew.return_value = True - mock_rc_instance.latest_common_version.return_value = 10 - mock_rc.return_value = mock_rc_instance - with open(os.path.join(self.cli_config.renewal_configs_dir, - "example.org.conf"), "w") as f: - # This isn't actually parsed in this test; we have a separate - # test_initialization that tests the initialization, assuming - # that configobj can correctly parse the config file. - f.write("cert = cert.pem\nprivkey = privkey.pem\n") - f.write("chain = chain.pem\nfullchain = fullchain.pem\n") - with open(os.path.join(self.cli_config.renewal_configs_dir, - "example.com.conf"), "w") as f: - f.write("cert = cert.pem\nprivkey = privkey.pem\n") - f.write("chain = chain.pem\nfullchain = fullchain.pem\n") - renewer.main(cli_args=self._common_cli_args()) - self.assertEqual(mock_rc.call_count, 2) - self.assertEqual(mock_rc_instance.update_all_links_to.call_count, 2) - self.assertEqual(mock_notify.notify.call_count, 4) - self.assertEqual(mock_renew.call_count, 2) - # If we have instances that don't need any work done, no work should - # be done (call counts associated with processing deployments or - # renewals should not increase). - mock_happy_instance = mock.MagicMock() - mock_happy_instance.should_autodeploy.return_value = False - mock_happy_instance.should_autorenew.return_value = False - mock_happy_instance.latest_common_version.return_value = 10 - mock_rc.return_value = mock_happy_instance - renewer.main(cli_args=self._common_cli_args()) - self.assertEqual(mock_rc.call_count, 4) - self.assertEqual(mock_happy_instance.update_all_links_to.call_count, 0) - self.assertEqual(mock_notify.notify.call_count, 4) - self.assertEqual(mock_renew.call_count, 2) - - def test_bad_config_file(self): - from letsencrypt import renewer - os.unlink(os.path.join(self.cli_config.renewal_configs_dir, - "example.org.conf")) - with open(os.path.join(self.cli_config.renewal_configs_dir, - "bad.conf"), "w") as f: - f.write("incomplete = configfile\n") - renewer.main(cli_args=self._common_cli_args()) - # The errors.CertStorageError is caught inside and nothing happens. - def test_missing_cert(self): from letsencrypt import storage self.assertRaises(errors.CertStorageError, From d8a2252c5c0fd6f2fae66672885c10f5905f294d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 15:02:09 -0800 Subject: [PATCH 34/76] Rename renewer tests --- letsencrypt/tests/cli_test.py | 4 ++-- letsencrypt/tests/{renewer_test.py => storage_test.py} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename letsencrypt/tests/{renewer_test.py => storage_test.py} (100%) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index f0ac954f9..45fde7eba 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -24,7 +24,7 @@ from letsencrypt import le_util from letsencrypt.plugins import disco from letsencrypt.plugins import manual -from letsencrypt.tests import renewer_test +from letsencrypt.tests import storage_test from letsencrypt.tests import test_util @@ -782,7 +782,7 @@ class DetermineAccountTest(unittest.TestCase): self.assertEqual('other email', self.config.email) -class DuplicativeCertsTest(renewer_test.BaseRenewableCertTest): +class DuplicativeCertsTest(storage_test.BaseRenewableCertTest): """Test to avoid duplicate lineages.""" def setUp(self): diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/storage_test.py similarity index 100% rename from letsencrypt/tests/renewer_test.py rename to letsencrypt/tests/storage_test.py From 49e430ab1353765c6f89eb1a19f62fcb999fd656 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 15:02:49 -0800 Subject: [PATCH 35/76] Remove stray comments about renewer.py --- letsencrypt/tests/storage_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py index 6e63281df..ea236e4c2 100644 --- a/letsencrypt/tests/storage_test.py +++ b/letsencrypt/tests/storage_test.py @@ -1,4 +1,4 @@ -"""Tests for letsencrypt.renewer.""" +"""Tests for letsencrypt.storage.""" import datetime import pytz import os @@ -98,7 +98,7 @@ class BaseRenewableCertTest(unittest.TestCase): class RenewableCertTests(BaseRenewableCertTest): # pylint: disable=too-many-public-methods - """Tests for letsencrypt.renewer.*.""" + """Tests for letsencrypt.storage.""" def test_initialization(self): self.assertEqual(self.test_rc.lineagename, "example.org") From 972c596af9c761bb3e1e36c67d668e72dcacca65 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 15:03:52 -0800 Subject: [PATCH 36/76] Delete renewer.py --- letsencrypt/renewer.py | 210 ----------------------------------------- 1 file changed, 210 deletions(-) delete mode 100644 letsencrypt/renewer.py diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py deleted file mode 100644 index 83c6106c0..000000000 --- a/letsencrypt/renewer.py +++ /dev/null @@ -1,210 +0,0 @@ -"""Renewer tool. - -Renewer tool handles autorenewal and autodeployment of renewed certs -within lineages of successor certificates, according to configuration. - -.. todo:: Sanity checking consistency, validity, freshness? -.. todo:: Call new installer API to restart servers after deployment - -""" -from __future__ import print_function - -import argparse -import logging -import os -import sys - -import OpenSSL -import zope.component - -from letsencrypt import account -from letsencrypt import configuration -from letsencrypt import constants -from letsencrypt import colored_logging -from letsencrypt import cli -from letsencrypt import client -from letsencrypt import crypto_util -from letsencrypt import errors -from letsencrypt import le_util -from letsencrypt import notify -from letsencrypt import storage - -from letsencrypt.display import util as display_util -from letsencrypt.plugins import disco as plugins_disco - - -logger = logging.getLogger(__name__) - - -class _AttrDict(dict): - """Attribute dictionary. - - A trick to allow accessing dictionary keys as object attributes. - - """ - def __init__(self, *args, **kwargs): - super(_AttrDict, self).__init__(*args, **kwargs) - self.__dict__ = self - - -def renew(cert, old_version): - """Perform automated renewal of the referenced cert, if possible. - - :param letsencrypt.storage.RenewableCert cert: The certificate - lineage to attempt to renew. - :param int old_version: The version of the certificate lineage - relative to which the renewal should be attempted. - - :returns: A number referring to newly created version of this cert - lineage, or ``False`` if renewal was not successful. - :rtype: `int` or `bool` - - """ - # TODO: handle partial success (some names can be renewed but not - # others) - # TODO: handle obligatory key rotation vs. optional key rotation vs. - # requested key rotation - if "renewalparams" not in cert.configfile: - # TODO: notify user? - return False - renewalparams = cert.configfile["renewalparams"] - if "authenticator" not in renewalparams: - # TODO: notify user? - return False - # Instantiate the appropriate authenticator - plugins = plugins_disco.PluginsRegistry.find_all() - config = configuration.NamespaceConfig(_AttrDict(renewalparams)) - # XXX: this loses type data (for example, the fact that key_size - # was an int, not a str) - config.rsa_key_size = int(config.rsa_key_size) - config.tls_sni_01_port = int(config.tls_sni_01_port) - config.namespace.http01_port = int(config.namespace.http01_port) - zope.component.provideUtility(config) - try: - authenticator = plugins[renewalparams["authenticator"]] - except KeyError: - # TODO: Notify user? (authenticator could not be found) - return False - authenticator = authenticator.init(config) - - authenticator.prepare() - acc = account.AccountFileStorage(config).load( - account_id=renewalparams["account"]) - - le_client = client.Client(config, acc, authenticator, None) - with open(cert.version("cert", old_version)) as f: - sans = crypto_util.get_sans_from_cert(f.read()) - new_certr, new_chain, new_key, _ = le_client.obtain_certificate(sans) - if new_chain: - # XXX: Assumes that there was a key change. We need logic - # for figuring out whether there was or not. Probably - # best is to have obtain_certificate return None for - # new_key if the old key is to be used (since save_successor - # already understands this distinction!) - return cert.save_successor( - old_version, OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), - new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) - # TODO: Notify results - else: - # TODO: Notify negative results - return False - # TODO: Consider the case where the renewal was partially successful - # (where fewer than all names were renewed) - - -def _cli_log_handler(args, level, fmt): # pylint: disable=unused-argument - handler = colored_logging.StreamHandler() - handler.setFormatter(logging.Formatter(fmt)) - handler.setLevel(level) - return handler - - -def _paths_parser(parser): - add = parser.add_argument_group("paths").add_argument - add("--config-dir", default=cli.flag_default("config_dir"), - help=cli.config_help("config_dir")) - add("--work-dir", default=cli.flag_default("work_dir"), - help=cli.config_help("work_dir")) - add("--logs-dir", default=cli.flag_default("logs_dir"), - help="Path to a directory where logs are stored.") - - return parser - - -def _create_parser(): - parser = argparse.ArgumentParser() - #parser.add_argument("--cron", action="store_true", help="Run as cronjob.") - parser.add_argument( - "-v", "--verbose", dest="verbose_count", action="count", - default=cli.flag_default("verbose_count"), help="This flag can be used " - "multiple times to incrementally increase the verbosity of output, " - "e.g. -vvv.") - - return _paths_parser(parser) - - -def main(cli_args=sys.argv[1:]): - """Main function for autorenewer script.""" - # TODO: Distinguish automated invocation from manual invocation, - # perhaps by looking at sys.argv[0] and inhibiting automated - # invocations if /etc/letsencrypt/renewal.conf defaults have - # turned it off. (The boolean parameter should probably be - # called renewer_enabled.) - - # TODO: When we have a more elaborate renewer command line, we will - # presumably also be able to specify a config file on the - # command line, which, if provided, should take precedence over - # te default config files - - zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) - - args = _create_parser().parse_args(cli_args) - - uid = os.geteuid() - le_util.make_or_verify_dir(args.logs_dir, 0o700, uid) - cli.setup_logging(args, _cli_log_handler, logfile='renewer.log') - - cli_config = configuration.RenewerConfiguration(args) - - # Ensure that all of the needed folders have been created before continuing - le_util.make_or_verify_dir(cli_config.work_dir, - constants.CONFIG_DIRS_MODE, uid) - - for renewal_file in os.listdir(cli_config.renewal_configs_dir): - if not renewal_file.endswith(".conf"): - continue - print("Processing " + renewal_file) - try: - # TODO: Before trying to initialize the RenewableCert object, - # we could check here whether the combination of the config - # and the rc_config together disables all autorenewal and - # autodeployment applicable to this cert. In that case, we - # can simply continue and don't need to instantiate a - # RenewableCert object for this cert at all, which could - # dramatically improve performance for large deployments - # where autorenewal is widely turned off. - cert = storage.RenewableCert( - os.path.join(cli_config.renewal_configs_dir, renewal_file), - cli_config) - except errors.CertStorageError: - # This indicates an invalid renewal configuration file, such - # as one missing a required parameter (in the future, perhaps - # also one that is internally inconsistent or is missing a - # required parameter). As a TODO, maybe we should warn the - # user about the existence of an invalid or corrupt renewal - # config rather than simply ignoring it. - continue - if cert.should_autorenew(): - # Note: not cert.current_version() because the basis for - # the renewal is the latest version, even if it hasn't been - # deployed yet! - old_version = cert.latest_common_version() - renew(cert, old_version) - notify.notify("Autorenewed a cert!!!", "root", "It worked!") - # TODO: explain what happened - if cert.should_autodeploy(): - cert.update_all_links_to(cert.latest_common_version()) - # TODO: restart web server (invoke IInstaller.restart() method) - notify.notify("Autodeployed a cert!!!", "root", "It worked!") - # TODO: explain what happened From ca8b4751adbb1edefa6880133ee849a147d44e5d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 15:13:17 -0800 Subject: [PATCH 37/76] Update config in save_successor --- letsencrypt/storage.py | 36 ++++++++++++++++++++++++++++++- letsencrypt/tests/storage_test.py | 25 ++++++++++++--------- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 8cc26d5b5..70a4ea302 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -87,6 +87,31 @@ def write_renewal_config(filename, target, cli_config): return config +def update_configuration(lineagename, target, cli_config): + """Modifies lineagename's config to contain the specified values. + + :param str lineagename: Name of the lineage being modified + :param dict target: Maps ALL_FOUR to their symlink paths + :param .RenewerConfiguration cli_config: parsed command line + arguments + + :returns: Configuration object for the updated config file + :rtype: configobj.ConfigObj + + """ + config_filename = os.path.join( + cli_config.renewal_configs_dir, lineagename) + ".conf" + temp_filename = config_filename + ".new" + + # If an existing tempfile exists, delete it + if os.path.exists(temp_filename): + os.unlink(temp_filename) + write_renewal_config(temp_filename, target, cli_config) + os.rename(temp_filename, config_filename) + + return configobj.ConfigObj(config_filename) + + class RenewableCert(object): # pylint: disable=too-many-instance-attributes """Renewable certificate. @@ -711,7 +736,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes new_config = write_renewal_config(config_filename, target, cli_config) return cls(new_config.filename, cli_config) - def save_successor(self, prior_version, new_cert, new_privkey, new_chain): + def save_successor(self, prior_version, new_cert, + new_privkey, new_chain, cli_config): """Save new cert and chain as a successor of a prior version. Returns the new version number that was created. @@ -727,6 +753,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :param str new_privkey: the new private key, in PEM format, or ``None``, if the private key has not changed :param str new_chain: the new chain, in PEM format + :param .RenewerConfiguration cli_config: parsed command line + arguments :returns: the new version number that was created :rtype: int @@ -775,4 +803,10 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes with open(target["fullchain"], "w") as f: logger.debug("Writing full chain to %s.", target["fullchain"]) f.write(new_cert + new_chain) + + # Update renewal config file + self.configfile = update_configuration( + self.lineagename, target, cli_config) + self.configuration = config_with_defaults(self.configfile) + return target_version diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py index e8ef28932..963df60c2 100644 --- a/letsencrypt/tests/storage_test.py +++ b/letsencrypt/tests/storage_test.py @@ -504,8 +504,9 @@ class RenewableCertTests(BaseRenewableCertTest): with open(where, "w") as f: f.write(kind) self.test_rc.update_all_links_to(3) - self.assertEqual(6, self.test_rc.save_successor(3, "new cert", None, - "new chain")) + self.assertEqual( + 6, self.test_rc.save_successor(3, "new cert", None, + "new chain", self.cli_config)) with open(self.test_rc.version("cert", 6)) as f: self.assertEqual(f.read(), "new cert") with open(self.test_rc.version("chain", 6)) as f: @@ -516,10 +517,12 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertFalse(os.path.islink(self.test_rc.version("privkey", 3))) self.assertTrue(os.path.islink(self.test_rc.version("privkey", 6))) # Let's try two more updates - self.assertEqual(7, self.test_rc.save_successor(6, "again", None, - "newer chain")) - self.assertEqual(8, self.test_rc.save_successor(7, "hello", None, - "other chain")) + self.assertEqual( + 7, self.test_rc.save_successor(6, "again", None, + "newer chain", self.cli_config)) + self.assertEqual( + 8, self.test_rc.save_successor(7, "hello", None, + "other chain", self.cli_config)) # All of the subsequent versions should link directly to the original # privkey. for i in (6, 7, 8): @@ -532,8 +535,9 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(self.test_rc.current_version(kind), 3) # Test updating from latest version rather than old version self.test_rc.update_all_links_to(8) - self.assertEqual(9, self.test_rc.save_successor(8, "last", None, - "attempt")) + self.assertEqual( + 9, self.test_rc.save_successor(8, "last", None, + "attempt", self.cli_config)) for kind in ALL_FOUR: self.assertEqual(self.test_rc.available_versions(kind), range(1, 10)) @@ -542,8 +546,9 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(f.read(), "last" + "attempt") # Test updating when providing a new privkey. The key should # be saved in a new file rather than creating a new symlink. - self.assertEqual(10, self.test_rc.save_successor(9, "with", "a", - "key")) + self.assertEqual( + 10, self.test_rc.save_successor(9, "with", "a", + "key", self.cli_config)) self.assertTrue(os.path.exists(self.test_rc.version("privkey", 10))) self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10))) From c9909810353fc9ebd8de85e491e4f68c12436364 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 15:18:31 -0800 Subject: [PATCH 38/76] Pass config in call to save_successor --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 92e985313..832cc9117 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -428,8 +428,8 @@ def _auth_from_domains(le_client, config, domains): lineage.save_successor( lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), - new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) - + new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain), + configuration.RenewerConfiguration(config)) lineage.update_all_links_to(lineage.latest_common_version()) # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant From b60583e416610f59226907cb9857ea78e3868493 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 15:47:46 -0800 Subject: [PATCH 39/76] Test deleting stray .new file --- letsencrypt/tests/storage_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py index 963df60c2..9d402089c 100644 --- a/letsencrypt/tests/storage_test.py +++ b/letsencrypt/tests/storage_test.py @@ -544,6 +544,10 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(self.test_rc.current_version(kind), 8) with open(self.test_rc.version("fullchain", 9)) as f: self.assertEqual(f.read(), "last" + "attempt") + temp_config_file = os.path.join(self.cli_config.renewal_configs_dir, + self.test_rc.lineagename) + ".conf.new" + with open(temp_config_file, "w") as f: + f.write("We previously crashed while writing me :(") # Test updating when providing a new privkey. The key should # be saved in a new file rather than creating a new symlink. self.assertEqual( @@ -551,6 +555,7 @@ class RenewableCertTests(BaseRenewableCertTest): "key", self.cli_config)) self.assertTrue(os.path.exists(self.test_rc.version("privkey", 10))) self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10))) + self.assertFalse(os.path.exists(temp_config_file)) def test_new_lineage(self): """Test for new_lineage() class method.""" From db9a931fb050f526b5e37b56e0a8887ff7f86f88 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 16:10:47 -0800 Subject: [PATCH 40/76] Update cli_config in save_successor --- letsencrypt/storage.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 70a4ea302..e338c890a 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -766,6 +766,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # if needed (ensuring their permissions are correct) # Figure out what the new version is and hence where to save things + self.cli_config = cli_config target_version = self.next_free_version() archive = self.cli_config.archive_dir prefix = os.path.join(archive, self.lineagename) From 04f13a9cf83bf1bae9ca24086fd15947ef264a0f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 16:22:52 -0800 Subject: [PATCH 41/76] Use symlinks not their targets --- letsencrypt/storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index e338c890a..67d40a58f 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -805,9 +805,10 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes logger.debug("Writing full chain to %s.", target["fullchain"]) f.write(new_cert + new_chain) + symlinks = dict((kind, self.configuration[kind]) for kind in ALL_FOUR) # Update renewal config file self.configfile = update_configuration( - self.lineagename, target, cli_config) + self.lineagename, symlinks, cli_config) self.configuration = config_with_defaults(self.configfile) return target_version From 41bb67f7a269ba82995c3dd1ff9194072aea0290 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Feb 2016 16:29:36 -0800 Subject: [PATCH 42/76] Use .namespace --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 832cc9117..8c33ddfd0 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -429,7 +429,7 @@ def _auth_from_domains(le_client, config, domains): lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain), - configuration.RenewerConfiguration(config)) + configuration.RenewerConfiguration(config.namespace)) lineage.update_all_links_to(lineage.latest_common_version()) # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant From 1c52e1982ccb580e5bc4a3e0d28902c4865fe7e0 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 3 Feb 2016 17:49:08 -0800 Subject: [PATCH 43/76] 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 44/76] 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 45/76] 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 46/76] 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 47/76] 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 48/76] 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 49/76] 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 50/76] 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 51/76] 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 df111febfd0237930157bbd7583a8f16d361f0cd Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Thu, 4 Feb 2016 05:48:00 +0200 Subject: [PATCH 52/76] Fixing parameter type for obtain_certificate's domains parameter --- letsencrypt/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index c2dfca1bf..dee5866ea 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -249,7 +249,7 @@ class Client(object): `.register` must be called before `.obtain_certificate` - :param set domains: domains to get a certificate + :param list domains: domains to get a certificate :returns: `.CertificateResource`, certificate chain (as returned by `.fetch_chain`), and newly generated private key From 4d6a3dfdff02d8ea5078be7857b4395becdc9b9a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 20:04:07 -0800 Subject: [PATCH 53/76] 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 54/76] 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 55/76] 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 56/76] 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 57/76] 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 58/76] 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 59/76] 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 60/76] 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 61/76] 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 62/76] 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 63/76] 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 64/76] 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 65/76] 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 dace0aecfaee06af3c9e5cbfcc49ceebade3cb56 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 16:23:39 -0800 Subject: [PATCH 66/76] I missed these --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 8c33ddfd0..a4fa409c1 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -159,7 +159,7 @@ def _determine_account(config): acc = accounts[0] else: # no account registered yet if config.email is None and not config.register_unsafely_without_email: - config.email = display_ops.get_email() + config.namespace.email = display_ops.get_email() def _tos_cb(regr): if config.tos: @@ -181,7 +181,7 @@ def _determine_account(config): raise errors.Error( "Unable to register an account with ACME server") - config.account = acc.id + config.namespace.account = acc.id return acc, acme From 7dd1ea4dcfc2160e616af4fe27e628b61c46758e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 4 Feb 2016 17:30:52 -0800 Subject: [PATCH 67/76] 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 68/76] 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 69/76] 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 70/76] 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 71/76] 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 72/76] 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 73/76] 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 74/76] 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 75/76] 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 76/76] 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: