Merge pull request #970 from letsencrypt/storage_paranoia

Storage paranoia

letsencrypt/cli.py

@@ -12,7 +12,6 @@
 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

letsencrypt/renewer.py

@@ -12,7 +12,6 @@
 import os
 import sys
 
-import configobj
 import OpenSSL
 import zope.component
 
@@ -142,14 +141,19 @@ 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
     #       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)
@@ -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

letsencrypt/storage.py

@@ -11,6 +11,7 @@
 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 @@ def _fix(self):
     #       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 @@ def _update_link_to(self, kind, version):
     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 @@ def new_lineage(cls, lineagename, cert, privkey, chain,
         :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 @@ def new_lineage(cls, lineagename, cert, privkey, chain,
         # 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.

letsencrypt/tests/cli_test.py

@@ -346,26 +346,25 @@ def test_find_duplicative_names(self, unused_makedir):
             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))