From f3655f9ab3ffde9f7a5be6580b7cb4b17beb62c5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 7 Feb 2016 19:14:24 -0800 Subject: [PATCH 01/10] Throw in an extra test for good measure --- letsencrypt/tests/client_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 6a8899c3b..222e9c707 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -127,6 +127,12 @@ class ClientTest(unittest.TestCase): # and that the cert was obtained correctly self._check_obtain_certificate() + # Now provoke an inconsistent domains error... + + self.client.config.domains.append("hippopotamus.io") + self.assertRaises(errors.ConfigurationError, + self.client.obtain_certificate_from_csr, mock_process_domain) + @mock.patch("letsencrypt.client.crypto_util") def test_obtain_certificate(self, mock_crypto_util): self._mock_obtain_certificate() From c23aa37f4b910e5357c191b729d50e2d7042a715 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 18:06:46 -0800 Subject: [PATCH 02/10] Refactor --csr handling to run early enough for --webroot --- letsencrypt/cli.py | 24 +++++++++++++++++++++++- letsencrypt/client.py | 32 ++------------------------------ letsencrypt/tests/client_test.py | 17 +++++++++++------ 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index de1321ac9..c9c58ea3b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -684,7 +684,7 @@ 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(_process_domain) + certr, chain = le_client.obtain_certificate_from_csr(config.domains, config.actual_csr) if config.dry_run: logger.info( "Dry run: skipping saving certificate to %s", config.cert_path) @@ -1106,8 +1106,30 @@ 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 enought that the + webroot plugin can know about the calls to _process_domain + """ + csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der") + # TODO: add CN to domains? + domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) + for d in domains: + _process_domain(parsed_args, d) + parsed_args.actual_csr = csr + csr_domains, config_domains = set(domains), set(parsed_args.domains) + if csr_domains != config_domains: + raise errors.ConfigurationError( + "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}" + .format(", ".join(csr_domains), ", ".join(config_domains)) + ) + + def determine_verb(self): """Determines the verb/subcommand provided by the user. diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 413409ded..fd851c163 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -195,7 +195,7 @@ class Client(object): else: self.auth_handler = None - def _obtain_certificate(self, domains, csr): + def obtain_certificate_from_csr(self, domains, csr): """Obtain certificate. Internal function with precondition that `domains` are @@ -228,34 +228,6 @@ class Client(object): authzr) return certr, self.acme.fetch_chain(certr) - def obtain_certificate_from_csr(self, domain_callback): - """Obtain certficiate from CSR. - - :param function(config, domains) domain_callback: callback for each - domain extracted from the CSR, to ensure that webroot-map and similar - housekeeping in cli.py is performed correctly - - :returns: `.CertificateResource` and certificate chain (as - returned by `.fetch_chain`). - :rtype: tuple - - """ - - #raise TypeError("About to call %r" % le_util.CSR) - csr = le_util.CSR(file=self.config.csr[0], data=self.config.csr[1], form="der") - # TODO: add CN to domains? - domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) - for d in domains: - domain_callback(self.config, d) - - csr_domains, config_domains = set(domains), set(self.config.domains) - if csr_domains != config_domains: - raise errors.ConfigurationError( - "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}" - .format(", ".join(csr_domains), ", ".join(config_domains)) - ) - - return self._obtain_certificate(domains, csr) def obtain_certificate(self, domains): """Obtains a certificate from the ACME server. @@ -276,7 +248,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. diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 6a8899c3b..d75237bab 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -109,21 +109,26 @@ class ClientTest(unittest.TestCase): 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() - mock_process_domain = mock.MagicMock() + 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 - self.client.config.domains = self.eg_domains - self.assertEqual( - (mock.sentinel.certr, mock.sentinel.chain), - self.client.obtain_certificate_from_csr(mock_process_domain)) + 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"))) + 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() From 3999d65d1c51b0de8f8b1e7a586583ffed038dc6 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 18:06:46 -0800 Subject: [PATCH 03/10] Refactor --csr handling to run early enough for --webroot --- letsencrypt/cli.py | 24 +++++++++++++++++++++++- letsencrypt/client.py | 32 ++------------------------------ letsencrypt/tests/client_test.py | 17 +++++++++++------ 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 99ee7884a..e01275153 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -684,7 +684,7 @@ 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(_process_domain) + certr, chain = le_client.obtain_certificate_from_csr(config.domains, config.actual_csr) if config.dry_run: logger.info( "Dry run: skipping saving certificate to %s", config.cert_path) @@ -1106,8 +1106,30 @@ 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 enought that the + webroot plugin can know about the calls to _process_domain + """ + csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der") + # TODO: add CN to domains? + domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) + for d in domains: + _process_domain(parsed_args, d) + parsed_args.actual_csr = csr + csr_domains, config_domains = set(domains), set(parsed_args.domains) + if csr_domains != config_domains: + raise errors.ConfigurationError( + "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}" + .format(", ".join(csr_domains), ", ".join(config_domains)) + ) + + def determine_verb(self): """Determines the verb/subcommand provided by the user. diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 413409ded..fd851c163 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -195,7 +195,7 @@ class Client(object): else: self.auth_handler = None - def _obtain_certificate(self, domains, csr): + def obtain_certificate_from_csr(self, domains, csr): """Obtain certificate. Internal function with precondition that `domains` are @@ -228,34 +228,6 @@ class Client(object): authzr) return certr, self.acme.fetch_chain(certr) - def obtain_certificate_from_csr(self, domain_callback): - """Obtain certficiate from CSR. - - :param function(config, domains) domain_callback: callback for each - domain extracted from the CSR, to ensure that webroot-map and similar - housekeeping in cli.py is performed correctly - - :returns: `.CertificateResource` and certificate chain (as - returned by `.fetch_chain`). - :rtype: tuple - - """ - - #raise TypeError("About to call %r" % le_util.CSR) - csr = le_util.CSR(file=self.config.csr[0], data=self.config.csr[1], form="der") - # TODO: add CN to domains? - domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) - for d in domains: - domain_callback(self.config, d) - - csr_domains, config_domains = set(domains), set(self.config.domains) - if csr_domains != config_domains: - raise errors.ConfigurationError( - "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}" - .format(", ".join(csr_domains), ", ".join(config_domains)) - ) - - return self._obtain_certificate(domains, csr) def obtain_certificate(self, domains): """Obtains a certificate from the ACME server. @@ -276,7 +248,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. diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 222e9c707..429945526 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -109,21 +109,26 @@ class ClientTest(unittest.TestCase): 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() - mock_process_domain = mock.MagicMock() + 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 - self.client.config.domains = self.eg_domains - self.assertEqual( - (mock.sentinel.certr, mock.sentinel.chain), - self.client.obtain_certificate_from_csr(mock_process_domain)) + 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"))) + 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() From 9af8a875cd6a6511a95d8a3e51885464be3861ad Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 18:12:49 -0800 Subject: [PATCH 04/10] Update hippopotamus test --- letsencrypt/tests/client_test.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 429945526..dbc57565e 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -118,13 +118,17 @@ class ClientTest(unittest.TestCase): 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_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), @@ -132,11 +136,6 @@ class ClientTest(unittest.TestCase): # and that the cert was obtained correctly self._check_obtain_certificate() - # Now provoke an inconsistent domains error... - - self.client.config.domains.append("hippopotamus.io") - self.assertRaises(errors.ConfigurationError, - self.client.obtain_certificate_from_csr, mock_process_domain) @mock.patch("letsencrypt.client.crypto_util") def test_obtain_certificate(self, mock_crypto_util): From a774922f8f5374c0919d2324661083c64739bd21 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 18:14:55 -0800 Subject: [PATCH 05/10] Revert "Revert "Allow webroot-map and --csr to exist together."" This reverts commit d65a3c65c20ca7c12c4f85802a8bc84a14b95611. --- letsencrypt/cli.py | 8 +------- letsencrypt/client.py | 27 ++++++++++++++++++++------- letsencrypt/plugins/webroot.py | 6 ++++-- letsencrypt/tests/cli_test.py | 5 +---- letsencrypt/tests/client_test.py | 24 +++++++++++++++++------- 5 files changed, 43 insertions(+), 27 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c335d8d5b..de1321ac9 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -672,11 +672,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") @@ -689,8 +684,7 @@ 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")) + certr, chain = le_client.obtain_certificate_from_csr(_process_domain) if config.dry_run: logger.info( "Dry run: skipping saving certificate to %s", config.cert_path) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 57b21a55f..413409ded 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -228,21 +228,34 @@ class Client(object): authzr) return certr, self.acme.fetch_chain(certr) - def obtain_certificate_from_csr(self, csr): + def obtain_certificate_from_csr(self, domain_callback): """Obtain certficiate from CSR. - :param .le_util.CSR csr: DER-encoded Certificate Signing - Request. + :param function(config, domains) domain_callback: callback for each + domain extracted from the CSR, to ensure that webroot-map and similar + housekeeping in cli.py is performed correctly :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) + + #raise TypeError("About to call %r" % le_util.CSR) + csr = le_util.CSR(file=self.config.csr[0], data=self.config.csr[1], form="der") + # TODO: add CN to domains? + domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) + for d in domains: + domain_callback(self.config, d) + + csr_domains, config_domains = set(domains), set(self.config.domains) + if csr_domains != config_domains: + raise errors.ConfigurationError( + "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}" + .format(", ".join(csr_domains), ", ".join(config_domains)) + ) + + return self._obtain_certificate(domains, csr) def obtain_certificate(self, domains): """Obtains a certificate from the ACME server. diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index f8176417c..3f5bc6d28 100644 --- a/letsencrypt/plugins/webroot.py +++ b/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") diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index c41f45116..2d36a9d21 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -228,7 +228,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args = ["certonly", "--webroot"] ret, _, _, _ = self._call(args) - self.assertTrue("--webroot-path must be set" in ret) + self.assertTrue("please set either --webroot-path" in ret) self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably") @@ -323,9 +323,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') - ret, _, _, _ = self._call(['-a', 'bad_auth', 'certonly']) self.assertEqual(ret, 'The requested bad_auth plugin does not appear to be installed') diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 2f117f80c..6a8899c3b 100644 --- a/letsencrypt/tests/client_test.py +++ b/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,8 +102,7 @@ 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)), @@ -111,11 +111,21 @@ class ClientTest(unittest.TestCase): def test_obtain_certificate_from_csr(self): 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() + mock_process_domain = mock.MagicMock() + test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN) + with mock.patch("letsencrypt.client.le_util.CSR") as mock_CSR: + mock_CSR.return_value = test_csr + self.client.config.domains = self.eg_domains + self.assertEqual( + (mock.sentinel.certr, mock.sentinel.chain), + self.client.obtain_certificate_from_csr(mock_process_domain)) + + # 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"))) + + # 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): From 63c0718d869a95eadc2e05a38d5df375d71ee07c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 19:15:28 -0800 Subject: [PATCH 06/10] Accept --csr PEMFILE * Closes: #1082 #1935 * Also produce better errors if SANs are missing, though not yet fixing #1076 --- letsencrypt/cli.py | 28 +++++++++++++++++++++++----- letsencrypt/client.py | 7 ++++--- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c9c58ea3b..d9497d8fe 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -684,7 +684,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(config.domains, config.actual_csr) + 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) @@ -1116,12 +1117,29 @@ class HelpfulArgumentParser(object): Process a --csr flag. This needs to happen early enought that the webroot plugin can know about the calls to _process_domain """ - csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der") - # TODO: add CN to domains? - domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) + 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: + 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: + logger.debug("DER CSR parse error %s", e1) + logger.debug("PEM CSR parse error %s", traceback.format_exc()) + raise errors.Error("Failed to CSR file: %s", parsed_args.csr[0]) + + 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]) for d in domains: _process_domain(parsed_args, d) - parsed_args.actual_csr = csr + parsed_args.actual_csr = (csr, typ) csr_domains, config_domains = set(domains), set(parsed_args.domains) if csr_domains != config_domains: raise errors.ConfigurationError( diff --git a/letsencrypt/client.py b/letsencrypt/client.py index fd851c163..9dfa70e8d 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -195,7 +195,8 @@ class Client(object): else: self.auth_handler = None - def obtain_certificate_from_csr(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,8 +224,8 @@ 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) From b2e460f34bb5b29a545c6a4aa61d9cb56f9b6977 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 19:54:32 -0800 Subject: [PATCH 07/10] Address the comments of reviewers & lintmonsters --- letsencrypt/cli.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index d9497d8fe..375495833 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1114,23 +1114,23 @@ class HelpfulArgumentParser(object): def handle_csr(self, parsed_args): """ - Process a --csr flag. This needs to happen early enought that the + 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: + 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: + 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 CSR file: %s", parsed_args.csr[0]) + raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0])) if not domains: # TODO: add CN to domains instead: @@ -1143,7 +1143,7 @@ class HelpfulArgumentParser(object): csr_domains, config_domains = set(domains), set(parsed_args.domains) if csr_domains != config_domains: raise errors.ConfigurationError( - "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}" + "Inconsistent domain requests:\nFrom the CSR: {0}\nFrom command line/config: {1}" .format(", ".join(csr_domains), ", ".join(config_domains)) ) From e0cfd9f691fb9efd6197d41ecacfac4b1cf443be Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 21:10:34 -0800 Subject: [PATCH 08/10] Extra CSR sanity checking --- letsencrypt/cli.py | 15 +++++++++++---- letsencrypt/le_util.py | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 375495833..ac6e2c937 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1132,20 +1132,27 @@ class HelpfulArgumentParser(object): 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]) - for d in domains: - _process_domain(parsed_args, d) + 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)) - ) + .format(", ".join(csr_domains), ", ".join(config_domains))) def determine_verb(self): diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 35793849e..527c9bdae 100644 --- a/letsencrypt/le_util.py +++ b/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}" From 4000aa762ef3235f576b013725d95e402ef8ed48 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 21:14:45 -0800 Subject: [PATCH 09/10] Fix snauf --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7791d2819..533539684 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1202,7 +1202,7 @@ class HelpfulArgumentParser(object): _process_domain(parsed_args, d) for d in domains: - sanitised = le_util.enforce_domain_sanity(d): + 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)) From d6703f771ac158941360cce74642cc27b2e717e7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 21:28:43 -0800 Subject: [PATCH 10/10] Lint & cleanup weirdness from #2392... --- letsencrypt/cli.py | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 533539684..00d45b700 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -943,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()) @@ -954,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 @@ -1197,7 +1184,6 @@ class HelpfulArgumentParser(object): 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)