Merge pull request #2403 from letsencrypt/webroot-map-and-flags

Webroot map and flags

letsencrypt/cli.py

@@ -677,11 +677,6 @@ def run(config, plugins):  # pylint: disable=too-many-branches,too-many-locals
 def obtain_cert(config, plugins, lineage=None):
     """Implements "certonly": authenticate & obtain cert, but do not install it."""
 
-    if config.domains and config.csr is not None:
-        # TODO: --csr could have a priority, when --domains is
-        # supplied, check if CSR matches given domains?
-        return "--domains and --csr are mutually exclusive"
-
     try:
         # installers are used in auth mode to determine domain names
         installer, authenticator = choose_configurator_plugins(config, plugins, "certonly")
@@ -695,8 +690,8 @@ def obtain_cert(config, plugins, lineage=None):
     # This is a special case; cert and chain are simply saved
     if config.csr is not None:
         assert lineage is None, "Did not expect a CSR with a RenewableCert"
-        certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR(
-            file=config.csr[0], data=config.csr[1], form="der"))
+        csr, typ = config.actual_csr
+        certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ)
         if config.dry_run:
             logger.info(
                 "Dry run: skipping saving certificate to %s", config.cert_path)
@@ -948,7 +943,6 @@ def renew(config, unused_plugins):
         try:
             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 "
                            "unexpected error: %s. Skipping.", renewal_file, e)
             logger.debug("Traceback was:\n%s", traceback.format_exc())
@@ -959,24 +953,12 @@ def renew(config, unused_plugins):
             if renewal_candidate is None:
                 parse_failures.append(renewal_file)
             else:
-                # _reconstitute succeeded in producing a RenewableCert, so we
-                # have something to work with from this particular config file.
-
                 # XXX: ensure that each call here replaces the previous one
                 zope.component.provideUtility(lineage_config)
-                # Although obtain_cert itself also indirectly decides
-                # whether to renew or not, we need to check at this
-                # stage in order to avoid claiming that renewal
-                # succeeded when it wasn't even attempted (since
-                # obtain_cert wouldn't raise an error in that case).
                 if _should_renew(lineage_config, renewal_candidate):
-                    err = obtain_cert(lineage_config,
-                                      plugins_disco.PluginsRegistry.find_all(),
-                                      renewal_candidate)
-                    if err is None:
-                        renew_successes.append(renewal_candidate.fullchain)
-                    else:
-                        renew_failures.append(renewal_candidate.fullchain)
+                    plugins = plugins_disco.PluginsRegistry.find_all()
+                    obtain_cert(lineage_config, plugins, renewal_candidate)
+                    renew_successes.append(renewal_candidate.fullchain)
                 else:
                     renew_skipped.append(renewal_candidate.fullchain)
         except Exception as e:  # pylint: disable=broad-except
@@ -1178,8 +1160,53 @@ class HelpfulArgumentParser(object):
                                        "'certonly' or 'renew' subcommands")
                 parsed_args.break_my_certs = parsed_args.staging = True
 
+        if parsed_args.csr:
+            self.handle_csr(parsed_args)
+
         return parsed_args
 
+    def handle_csr(self, parsed_args):
+        """
+        Process a --csr flag. This needs to happen early enough that the
+        webroot plugin can know about the calls to _process_domain
+        """
+        try:
+            csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der")
+            typ = OpenSSL.crypto.FILETYPE_ASN1
+            domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1)
+        except OpenSSL.crypto.Error:
+            try:
+                e1 = traceback.format_exc()
+                typ = OpenSSL.crypto.FILETYPE_PEM
+                csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="pem")
+                domains = crypto_util.get_sans_from_csr(csr.data, typ)
+            except OpenSSL.crypto.Error:
+                logger.debug("DER CSR parse error %s", e1)
+                logger.debug("PEM CSR parse error %s", traceback.format_exc())
+                raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0]))
+        for d in domains:
+            _process_domain(parsed_args, d)
+
+        for d in domains:
+            sanitised = le_util.enforce_domain_sanity(d)
+            if d.lower() != sanitised:
+                raise errors.ConfigurationError(
+                    "CSR domain {0} needs to be sanitised to {1}.".format(d, sanitised))
+
+        if not domains:
+            # TODO: add CN to domains instead:
+            raise errors.Error(
+                "Unfortunately, your CSR %s needs to have a SubjectAltName for every domain"
+                % parsed_args.csr[0])
+
+        parsed_args.actual_csr = (csr, typ)
+        csr_domains, config_domains = set(domains), set(parsed_args.domains)
+        if csr_domains != config_domains:
+            raise errors.ConfigurationError(
+                "Inconsistent domain requests:\nFrom the CSR: {0}\nFrom command line/config: {1}"
+                .format(", ".join(csr_domains), ", ".join(config_domains)))
+
+
     def determine_verb(self):
         """Determines the verb/subcommand provided by the user.
 

letsencrypt/client.py

@@ -195,7 +195,8 @@ class Client(object):
         else:
             self.auth_handler = None
 
-    def _obtain_certificate(self, domains, csr):
+    def obtain_certificate_from_csr(self, domains, csr,
+        typ=OpenSSL.crypto.FILETYPE_ASN1):
         """Obtain certificate.
 
         Internal function with precondition that `domains` are
@@ -223,26 +224,11 @@ class Client(object):
 
         authzr = self.auth_handler.get_authorizations(domains)
         certr = self.acme.request_issuance(
-            jose.ComparableX509(OpenSSL.crypto.load_certificate_request(
-                OpenSSL.crypto.FILETYPE_ASN1, csr.data)),
+            jose.ComparableX509(
+                OpenSSL.crypto.load_certificate_request(typ, csr.data)),
             authzr)
         return certr, self.acme.fetch_chain(certr)
 
-    def obtain_certificate_from_csr(self, csr):
-        """Obtain certficiate from CSR.
-
-        :param .le_util.CSR csr: DER-encoded Certificate Signing
-            Request.
-
-        :returns: `.CertificateResource` and certificate chain (as
-            returned by `.fetch_chain`).
-        :rtype: tuple
-
-        """
-        return self._obtain_certificate(
-            # TODO: add CN to domains?
-            crypto_util.get_sans_from_csr(
-                csr.data, OpenSSL.crypto.FILETYPE_ASN1), csr)
 
     def obtain_certificate(self, domains):
         """Obtains a certificate from the ACME server.
@@ -263,7 +249,7 @@ class Client(object):
             self.config.rsa_key_size, self.config.key_dir)
         csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir)
 
-        return self._obtain_certificate(domains, csr) + (key, csr)
+        return self.obtain_certificate_from_csr(domains, csr) + (key, csr)
 
     def obtain_and_enroll_certificate(self, domains):
         """Obtain and enroll certificate.

letsencrypt/le_util.py

@@ -308,7 +308,7 @@ def enforce_domain_sanity(domain):
 
     # Unicode
     try:
-        domain = domain.encode('ascii')
+        domain = domain.encode('ascii').lower()
     except UnicodeDecodeError:
         raise errors.ConfigurationError(
             "Internationalized domain names are not presently supported: {0}"

letsencrypt/plugins/webroot.py

@@ -49,8 +49,10 @@ to serve all files under specified web root ({0})."""
         path_map = self.conf("map")
 
         if not path_map:
-            raise errors.PluginError("--{0} must be set".format(
-                self.option_name("path")))
+            raise errors.PluginError(
+                "Missing parts of webroot configuration; please set either "
+                "--webroot-path and --domains, or --webroot-map. Run with "
+                " --help webroot for examples.")
         for name, path in path_map.items():
             if not os.path.isdir(path):
                 raise errors.PluginError(path + " does not exist or is not a directory")

letsencrypt/tests/cli_test.py

@@ -231,7 +231,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             self._call(args)
             assert False, "Exception should have been raised"
         except errors.PluginSelectionError as e:
-            self.assertTrue("--webroot-path must be set" in e.message)
+            self.assertTrue("please set either --webroot-path" in e.message)
 
         self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably")
 
@@ -326,9 +326,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(config.fullchain_path, os.path.abspath(fullchain))
 
     def test_certonly_bad_args(self):
-        ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR])
-        self.assertEqual(ret, '--domains and --csr are mutually exclusive')
-
         try:
             self._call(['-a', 'bad_auth', 'certonly'])
             assert False, "Exception should have been raised"

letsencrypt/tests/client_test.py

@@ -82,6 +82,7 @@ class ClientTest(unittest.TestCase):
             no_verify_ssl=False, config_dir="/etc/letsencrypt")
         # pylint: disable=star-args
         self.account = mock.MagicMock(**{"key.pem": KEY})
+        self.eg_domains = ["example.com", "www.example.com"]
 
         from letsencrypt.client import Client
         with mock.patch("letsencrypt.client.acme_client.Client") as acme:
@@ -101,21 +102,40 @@ class ClientTest(unittest.TestCase):
         self.acme.fetch_chain.return_value = mock.sentinel.chain
 
     def _check_obtain_certificate(self):
-        self.client.auth_handler.get_authorizations.assert_called_once_with(
-            ["example.com", "www.example.com"])
+        self.client.auth_handler.get_authorizations.assert_called_once_with(self.eg_domains)
         self.acme.request_issuance.assert_called_once_with(
             jose.ComparableX509(OpenSSL.crypto.load_certificate_request(
                 OpenSSL.crypto.FILETYPE_ASN1, CSR_SAN)),
             self.client.auth_handler.get_authorizations())
         self.acme.fetch_chain.assert_called_once_with(mock.sentinel.certr)
 
-    def test_obtain_certificate_from_csr(self):
+    # FIXME move parts of this to test_cli.py...
+    @mock.patch("letsencrypt.cli._process_domain")
+    def test_obtain_certificate_from_csr(self, mock_process_domain):
         self._mock_obtain_certificate()
-        self.assertEqual(
-            (mock.sentinel.certr, mock.sentinel.chain),
-            self.client.obtain_certificate_from_csr(le_util.CSR(
-                form="der", file=None, data=CSR_SAN)))
-        self._check_obtain_certificate()
+        from letsencrypt import cli
+        test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN)
+        mock_parsed_args = mock.MagicMock()
+        with mock.patch("letsencrypt.client.le_util.CSR") as mock_CSR:
+            mock_CSR.return_value = test_csr
+            mock_parsed_args.domains = self.eg_domains[:]
+            mock_parser = mock.MagicMock(cli.HelpfulArgumentParser)
+            cli.HelpfulArgumentParser.handle_csr(mock_parser, mock_parsed_args)
+
+            # make sure cli processing occurred
+            cli_processed = (call[0][1] for call in mock_process_domain.call_args_list)
+            self.assertEqual(set(cli_processed), set(("example.com", "www.example.com")))
+            # Now provoke an inconsistent domains error...
+            mock_parsed_args.domains.append("hippopotamus.io")
+            self.assertRaises(errors.ConfigurationError,
+                cli.HelpfulArgumentParser.handle_csr, mock_parser, mock_parsed_args)
+
+            self.assertEqual(
+                (mock.sentinel.certr, mock.sentinel.chain),
+                self.client.obtain_certificate_from_csr(self.eg_domains, test_csr))
+            # and that the cert was obtained correctly
+            self._check_obtain_certificate()
+
 
     @mock.patch("letsencrypt.client.crypto_util")
     def test_obtain_certificate(self, mock_crypto_util):