From 46984689ae95f637951a58a03f2a5aea265c18d4 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 6 Feb 2016 13:19:55 -0800 Subject: [PATCH 1/6] Attempt to get --csr and -w to play together --- letsencrypt/cli.py | 3 +-- letsencrypt/client.py | 23 ++++++++++++++++------- letsencrypt/tests/client_test.py | 23 ++++++++++++++++------- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 838da4015..3b614d4b4 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -689,8 +689,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..b4d6c5b56 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -228,21 +228,30 @@ 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? + try: + domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) + except: + raise TypeError("Failed %r %r %r" % (self.config.csr, csr, csr.data)) + for d in domains: + domain_callback(self.config, d) + + return self._obtain_certificate(domains, csr) def obtain_certificate(self, domains): """Obtains a certificate from the ACME server. diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 2f117f80c..f051b6618 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,20 @@ 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.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(self.eg_domains)) + + # 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 89df062a1c6be00153807b4fb015b04e63f1d318 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 6 Feb 2016 13:38:35 -0800 Subject: [PATCH 2/6] Allow config.domains to exist in CSR mode --- letsencrypt/cli.py | 5 ----- letsencrypt/client.py | 12 ++++++++---- letsencrypt/tests/client_test.py | 3 ++- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3b614d4b4..99ee7884a 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") diff --git a/letsencrypt/client.py b/letsencrypt/client.py index b4d6c5b56..046c58cc7 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -244,13 +244,17 @@ class Client(object): #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? - try: - domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) - except: - raise TypeError("Failed %r %r %r" % (self.config.csr, csr, csr.data)) + 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}\n:cli config{1}" + .format(", ".join(csr_domains), ", ".join(config_domains)) + ) + return self._obtain_certificate(domains, csr) def obtain_certificate(self, domains): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index f051b6618..5e8fd57a7 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -115,13 +115,14 @@ class ClientTest(unittest.TestCase): 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(self.eg_domains)) + self.assertEqual(set(cli_processed), set(("example.com", "www.example.com"))) # and that the cert was obtained correctly self._check_obtain_certificate() From dd20788e1cc37e5a1ec80ac8de65c8b790fbe8d1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 6 Feb 2016 13:39:32 -0800 Subject: [PATCH 3/6] lint --- letsencrypt/tests/client_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 5e8fd57a7..6a8899c3b 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -115,7 +115,7 @@ class ClientTest(unittest.TestCase): 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.client.config.domains = self.eg_domains self.assertEqual( (mock.sentinel.certr, mock.sentinel.chain), self.client.obtain_certificate_from_csr(mock_process_domain)) From 6df94bf68dff22f2dc91dac7f2d8f772de2e5793 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 6 Feb 2016 13:47:52 -0800 Subject: [PATCH 4/6] Better webroot configuration error Fixes: #2377 --- letsencrypt/plugins/webroot.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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") From 2ba2dde9ed4740db196fffc095acd8928475bcf8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 7 Feb 2016 18:47:50 -0800 Subject: [PATCH 5/6] Fix some broken tests --- letsencrypt/tests/cli_test.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a5757399e..1a86fb99b 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') From 7b0e70173126f46d5b8be57a48ec45672681db4b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 7 Feb 2016 19:08:26 -0800 Subject: [PATCH 6/6] Fix error formatting --- letsencrypt/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 046c58cc7..413409ded 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -251,7 +251,7 @@ class Client(object): csr_domains, config_domains = set(domains), set(self.config.domains) if csr_domains != config_domains: raise errors.ConfigurationError( - "Inconsistent domain requests:\ncsr:{0}\n:cli config{1}" + "Inconsistent domain requests:\ncsr: {0}\ncli config: {1}" .format(", ".join(csr_domains), ", ".join(config_domains)) )