diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5163f2868..822f231ef 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -12,7 +12,6 @@ import time import traceback import configargparse -import configobj import OpenSSL import zope.component import zope.interface.exceptions @@ -167,25 +166,21 @@ def _init_le_client(args, config, authenticator, installer): return client.Client(config, acc, authenticator, installer, acme=acme) -def _find_duplicative_certs(domains, config, renew_config): +def _find_duplicative_certs(config, domains): """Find existing certs that duplicate the request.""" identical_names_cert, subset_names_cert = None, None - configs_dir = renew_config.renewal_configs_dir + cli_config = configuration.RenewerConfiguration(config) + configs_dir = cli_config.renewal_configs_dir # Verify the directory is there le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) - cli_config = configuration.RenewerConfiguration(config) for renewal_file in os.listdir(configs_dir): try: full_path = os.path.join(configs_dir, renewal_file) - rc_config = configobj.ConfigObj(renew_config.renewer_config_file) - rc_config.merge(configobj.ConfigObj(full_path)) - rc_config.filename = full_path - candidate_lineage = storage.RenewableCert( - rc_config, config_opts=None, cli_config=cli_config) - except (configobj.ConfigObjError, CertStorageError, IOError): + candidate_lineage = storage.RenewableCert(full_path, cli_config) + except (CertStorageError, IOError): logger.warning("Renewal configuration file %s is broken. " "Skipping.", full_path) continue @@ -217,7 +212,7 @@ def _treat_as_renewal(config, domains): # kind of certificate to be obtained with renewal = False.) if not config.duplicate: ident_names_cert, subset_names_cert = _find_duplicative_certs( - domains, config, configuration.RenewerConfiguration(config)) + config, domains) # I am not sure whether that correctly reads the systemwide # configuration file. question = None diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 33492f344..c0014f4b2 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -12,7 +12,6 @@ import logging import os import sys -import configobj import OpenSSL import zope.component @@ -142,7 +141,7 @@ def _create_parser(): return _paths_parser(parser) -def main(config=None, cli_args=sys.argv[1:]): +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 @@ -150,6 +149,11 @@ def main(config=None, cli_args=sys.argv[1:]): # 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) @@ -160,27 +164,12 @@ def main(config=None, cli_args=sys.argv[1:]): cli_config = configuration.RenewerConfiguration(args) - config = storage.config_with_defaults(config) - # Now attempt 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. 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 this one. - config.merge(configobj.ConfigObj(cli_config.renewer_config_file)) # 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 i in os.listdir(cli_config.renewal_configs_dir): - print "Processing", i - if not i.endswith(".conf"): - continue - rc_config = configobj.ConfigObj(cli_config.renewer_config_file) - rc_config.merge(configobj.ConfigObj( - os.path.join(cli_config.renewal_configs_dir, i))) - rc_config.filename = os.path.join(cli_config.renewal_configs_dir, i) + for renewal_file in os.listdir(cli_config.renewal_configs_dir): + print "Processing", renewal_file try: # TODO: Before trying to initialize the RenewableCert object, # we could check here whether the combination of the config @@ -190,7 +179,7 @@ def main(config=None, cli_args=sys.argv[1:]): # RenewableCert object for this cert at all, which could # dramatically improve performance for large deployments # where autorenewal is widely turned off. - cert = storage.RenewableCert(rc_config, cli_config=cli_config) + cert = storage.RenewableCert(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 diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 12411ffb4..ece3df4b7 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -11,6 +11,7 @@ import pytz from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors +from letsencrypt import error_handler from letsencrypt import le_util ALL_FOUR = ("cert", "privkey", "chain", "fullchain") @@ -78,55 +79,50 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes renewal configuration file and/or systemwide defaults. """ - def __init__(self, configfile, config_opts=None, cli_config=None): + def __init__(self, config_filename, cli_config): """Instantiate a RenewableCert object from an existing lineage. - :param configobj.ConfigObj configfile: an already-parsed - ConfigObj object made from reading the renewal config file + :param str config_filename: the path to the renewal config file that defines this lineage. - - :param configobj.ConfigObj config_opts: systemwide defaults for - renewal properties not otherwise specified in the individual - renewal config file. - :param .RenewerConfiguration cli_config: + :param .RenewerConfiguration: parsed command line arguments :raises .CertStorageError: if the configuration file's name didn't end in ".conf", or the file is missing or broken. - :raises TypeError: if the provided renewal configuration isn't a - ConfigObj object. """ self.cli_config = cli_config - if isinstance(configfile, configobj.ConfigObj): - if not os.path.basename(configfile.filename).endswith(".conf"): - raise errors.CertStorageError( - "renewal config file name must end in .conf") - self.lineagename = os.path.basename( - configfile.filename)[:-len(".conf")] - else: - raise TypeError("RenewableCert config must be ConfigObj object") + if not config_filename.endswith(".conf"): + raise errors.CertStorageError( + "renewal config file name must end in .conf") + self.lineagename = os.path.basename( + config_filename[:-len(".conf")]) # self.configuration should be used to read parameters that # may have been chosen based on default values from the # systemwide renewal configuration; self.configfile should be # used to make and save changes. - self.configfile = configfile + try: + self.configfile = configobj.ConfigObj(config_filename) + except configobj.ConfigObjError: + raise errors.CertStorageError( + "error parsing {0}".format(config_filename)) # TODO: Do we actually use anything from defaults and do we want to # read further defaults from the systemwide renewal configuration # file at this stage? - self.configuration = config_with_defaults(config_opts) - self.configuration.merge(self.configfile) + self.configuration = config_with_defaults(self.configfile) if not all(x in self.configuration for x in ALL_FOUR): raise errors.CertStorageError( "renewal config file {0} is missing a required " - "file reference".format(configfile)) + "file reference".format(self.configfile)) self.cert = self.configuration["cert"] self.privkey = self.configuration["privkey"] self.chain = self.configuration["chain"] self.fullchain = self.configuration["fullchain"] + self._fix_symlinks() + def _consistent(self): """Are the files associated with this lineage self-consistent? @@ -203,6 +199,40 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # happen as a result of random tampering by a sysadmin, or # filesystem errors, or crashes.) + def _previous_symlinks(self): + """Returns the kind and path of all symlinks used in recovery. + + :returns: list of (kind, symlink) tuples + :rtype: list + + """ + previous_symlinks = [] + for kind in ALL_FOUR: + link_dir = os.path.dirname(getattr(self, kind)) + link_base = "previous_{0}.pem".format(kind) + previous_symlinks.append((kind, os.path.join(link_dir, link_base))) + + return previous_symlinks + + def _fix_symlinks(self): + """Fixes symlinks in the event of an incomplete version update. + + If there is no problem with the current symlinks, this function + has no effect. + + """ + previous_symlinks = self._previous_symlinks() + if all(os.path.exists(link[1]) for link in previous_symlinks): + for kind, previous_link in previous_symlinks: + current_link = getattr(self, kind) + if os.path.lexists(current_link): + os.unlink(current_link) + os.symlink(os.readlink(previous_link), current_link) + + for _, link in previous_symlinks: + if os.path.exists(link): + os.unlink(link) + def current_target(self, kind): """Returns full path to which the specified item currently points. @@ -374,10 +404,19 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def update_all_links_to(self, version): """Change all member objects to point to the specified version. - :param int version: the desired version""" + :param int version: the desired version - for kind in ALL_FOUR: - self._update_link_to(kind, version) + """ + with error_handler.ErrorHandler(self._fix_symlinks): + previous_links = self._previous_symlinks() + for kind, link in previous_links: + os.symlink(self.current_target(kind), link) + + for kind in ALL_FOUR: + self._update_link_to(kind, version) + + for _, link in previous_links: + os.unlink(link) def names(self, version=None): """What are the subject names of this certificate? @@ -532,6 +571,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :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`""" @@ -601,7 +642,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # TODO: add human-readable comments explaining other available # parameters new_config.write() - return cls(new_config, config, cli_config) + return cls(new_config.filename, cli_config) def save_successor(self, prior_version, new_cert, new_privkey, new_chain): """Save new cert and chain as a successor of a prior version. diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 8e9172055..03f291723 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -346,26 +346,25 @@ class DuplicativeCertsTest(renewer_test.BaseRenewableCertTest): f.write(test_cert) # No overlap at all - result = _find_duplicative_certs(['wow.net', 'hooray.org'], - self.config, self.cli_config) + result = _find_duplicative_certs( + self.cli_config, ['wow.net', 'hooray.org']) self.assertEqual(result, (None, None)) # Totally identical - result = _find_duplicative_certs(['example.com', 'www.example.com'], - self.config, self.cli_config) + result = _find_duplicative_certs( + self.cli_config, ['example.com', 'www.example.com']) self.assertTrue(result[0].configfile.filename.endswith('example.org.conf')) self.assertEqual(result[1], None) # Superset - result = _find_duplicative_certs(['example.com', 'www.example.com', - 'something.new'], self.config, - self.cli_config) + result = _find_duplicative_certs( + self.cli_config, ['example.com', 'www.example.com', 'something.new']) self.assertEqual(result[0], None) self.assertTrue(result[1].configfile.filename.endswith('example.org.conf')) # Partial overlap doesn't count - result = _find_duplicative_certs(['example.com', 'something.new'], - self.config, self.cli_config) + result = _find_duplicative_certs( + self.cli_config, ['example.com', 'something.new']) self.assertEqual(result, (None, None)) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 3599d8ad3..a856fcf4f 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -62,11 +62,11 @@ class BaseRenewableCertTest(unittest.TestCase): kind + ".pem") config.filename = os.path.join(self.tempdir, "renewal", "example.org.conf") + config.write() self.config = config self.defaults = configobj.ConfigObj() - self.test_rc = storage.RenewableCert( - self.config, self.defaults, self.cli_config) + self.test_rc = storage.RenewableCert(config.filename, self.cli_config) def tearDown(self): shutil.rmtree(self.tempdir) @@ -98,34 +98,32 @@ class RenewableCertTests(BaseRenewableCertTest): def test_renewal_bad_config(self): """Test that the RenewableCert constructor will complain if - the renewal configuration file doesn't end in ".conf" or if it - isn't a ConfigObj.""" + the renewal configuration file doesn't end in ".conf" + + """ from letsencrypt import storage - defaults = configobj.ConfigObj() - config = configobj.ConfigObj() - # These files don't exist and aren't created here; the point of the test - # is to confirm that the constructor rejects them outright because of - # the configfile's name. - for kind in ALL_FOUR: - config["cert"] = "nonexistent_" + kind + ".pem" - config.filename = "nonexistent_sillyfile" - self.assertRaises( - errors.CertStorageError, storage.RenewableCert, config, defaults) - self.assertRaises(TypeError, storage.RenewableCert, "fun", defaults) + broken = os.path.join(self.tempdir, "broken.conf") + with open(broken, "w") as f: + f.write("[No closing bracket for you!") + self.assertRaises(errors.CertStorageError, storage.RenewableCert, + broken, self.cli_config) + os.unlink(broken) + self.assertRaises(errors.CertStorageError, storage.RenewableCert, + "fun", self.cli_config) def test_renewal_incomplete_config(self): """Test that the RenewableCert constructor will complain if the renewal configuration file is missing a required file element.""" from letsencrypt import storage - defaults = configobj.ConfigObj() config = configobj.ConfigObj() config["cert"] = "imaginary_cert.pem" # Here the required privkey is missing. config["chain"] = "imaginary_chain.pem" config["fullchain"] = "imaginary_fullchain.pem" - config.filename = "imaginary_config.conf" - self.assertRaises( - errors.CertStorageError, storage.RenewableCert, config, defaults) + config.filename = os.path.join(self.tempdir, "imaginary_config.conf") + config.write() + self.assertRaises(errors.CertStorageError, storage.RenewableCert, + config.filename, self.cli_config) def test_consistent(self): # pylint: disable=too-many-statements,protected-access @@ -289,7 +287,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual("cert8.pem", os.path.basename(self.test_rc.version("cert", 8))) - def test_update_all_links_to(self): + def test_update_all_links_to_success(self): for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) @@ -307,6 +305,39 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(ver, self.test_rc.current_version(kind)) self.assertEqual(self.test_rc.latest_common_version(), 5) + def test_update_all_links_to_partial_failure(self): + def unlink_or_raise(path, real_unlink=os.unlink): + # pylint: disable=missing-docstring + basename = os.path.basename(path) + if "fullchain" in basename and basename.startswith("prev"): + raise ValueError + else: + real_unlink(path) + + self._write_out_ex_kinds() + with mock.patch("letsencrypt.storage.os.unlink") as mock_unlink: + mock_unlink.side_effect = unlink_or_raise + self.assertRaises(ValueError, self.test_rc.update_all_links_to, 12) + + for kind in ALL_FOUR: + self.assertEqual(self.test_rc.current_version(kind), 12) + + def test_update_all_links_to_full_failure(self): + def unlink_or_raise(path, real_unlink=os.unlink): + # pylint: disable=missing-docstring + if "fullchain" in os.path.basename(path): + raise ValueError + else: + real_unlink(path) + + self._write_out_ex_kinds() + with mock.patch("letsencrypt.storage.os.unlink") as mock_unlink: + mock_unlink.side_effect = unlink_or_raise + self.assertRaises(ValueError, self.test_rc.update_all_links_to, 12) + + for kind in ALL_FOUR: + self.assertEqual(self.test_rc.current_version(kind), 11) + def test_has_pending_deployment(self): for ver in xrange(1, 6): for kind in ALL_FOUR: @@ -667,10 +698,6 @@ class RenewableCertTests(BaseRenewableCertTest): 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, - "README"), "w") as f: - f.write("This is a README file to make sure that the renewer is") - f.write("able to correctly ignore files that don't end in .conf.") 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 @@ -682,7 +709,7 @@ class RenewableCertTests(BaseRenewableCertTest): "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(self.defaults, cli_args=self._common_cli_args()) + 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) @@ -695,7 +722,7 @@ class RenewableCertTests(BaseRenewableCertTest): 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(self.defaults, cli_args=self._common_cli_args()) + 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) @@ -706,7 +733,7 @@ class RenewableCertTests(BaseRenewableCertTest): with open(os.path.join(self.cli_config.renewal_configs_dir, "bad.conf"), "w") as f: f.write("incomplete = configfile\n") - renewer.main(self.defaults, cli_args=self._common_cli_args()) + renewer.main(cli_args=self._common_cli_args()) # The errors.CertStorageError is caught inside and nothing happens.