From acb4cbd43216425dbfbbb0796374b16c63b312c1 Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Fri, 5 Feb 2016 18:57:52 +0200 Subject: [PATCH 01/15] Fixing parameter type for get_authorizations domains parameter --- letsencrypt/auth_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index c63d8c8d4..45c51a020 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -57,7 +57,7 @@ class AuthHandler(object): def get_authorizations(self, domains, best_effort=False): """Retrieve all authorizations for challenges. - :param set domains: Domains for authorization + :param list domains: Domains for authorization :param bool best_effort: Whether or not all authorizations are required (this is useful in renewal) From f675c57242be352682b49aaf740e3d7d0c5870fd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 18:48:33 -0800 Subject: [PATCH 02/15] Test no exception on empty config file --- letsencrypt/tests/cli_test.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a5757399e..a411b7b8c 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -602,19 +602,26 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods print "Logs:" print lf.read() - - def test_renewal_verb(self): + def test_renew_verb(self): with open(test_util.vector_path('sample-renewal.conf')) as src: # put the correct path for cert.pem, chain.pem etc in the renewal conf renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path()) rd = os.path.join(self.config_dir, "renewal") - os.makedirs(rd) + if not os.path.exists(rd): + os.makedirs(rd) rc = os.path.join(rd, "sample-renewal.conf") with open(rc, "w") as dest: dest.write(renewal_conf) args = ["renew", "--dry-run", "-tvv"] self._test_renewal_common(True, [], args=args, renew=True) + def test_renew_verb_empty_config(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'empty.conf'), 'w'): + pass # leave the file empty + self.test_renew_verb() + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From ad2b6b2047abf9a154ec826e24f61a964bf01f14 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 18:59:16 -0800 Subject: [PATCH 03/15] Test config file without renewal params --- letsencrypt/tests/cli_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a411b7b8c..b2db89cd4 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -622,6 +622,20 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods pass # leave the file empty self.test_renew_verb() + def test_renew_sparse_config(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = ["not renewalparams"] + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From d8c0eb6d7f68113abb13643845cdc9f938d72866 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 19:02:34 -0800 Subject: [PATCH 04/15] Test no authenticator --- letsencrypt/tests/cli_test.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index b2db89cd4..876e262ac 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -630,7 +630,12 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage - mock_lineage.configuration = ["not renewalparams"] + mock_lineage.configuration = ['not renewalparams'] + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + mock_lineage.configuration = {'renewalparams': ['no auth']} with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) From d6e207e912f20dc9e016f030f2ac3626ec839454 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 19:11:36 -0800 Subject: [PATCH 05/15] Test renewal with bad int value in config --- letsencrypt/tests/cli_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 876e262ac..00588618a 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -641,6 +641,22 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_with_bad_int(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = { + 'renewalparams': {'authenticator': None, + 'rsa_key_size': 'over 9000'}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 0ba4b0c0b5dc67d07042738f6d6543f2c7ffc7c2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 11:42:55 -0800 Subject: [PATCH 06/15] Add bad domain renew test --- letsencrypt/tests/cli_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 00588618a..aebfd42a6 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -657,6 +657,22 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_with_bad_domain(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_rc.names.return_value = ['*.example.com'] + mock_lineage.configuration = { + 'renewalparams': {'authenticator': None}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 12f1ec685054fc09ee1ae59122420ab194c985c8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:11:45 -0800 Subject: [PATCH 07/15] Fix test and bad domain error handling --- letsencrypt/cli.py | 6 +++--- letsencrypt/tests/cli_test.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 838da4015..c335d8d5b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -856,10 +856,10 @@ def _reconstitute(config, full_path): try: domains = [le_util.enforce_domain_sanity(x) for x in renewal_candidate.names()] - except (UnicodeError, ValueError): + except errors.ConfigurationError as error: logger.warning("Renewal configuration file %s references a cert " - "that mentions a domain name that we regarded as " - "invalid. Skipping.", full_path) + "that contains an invalid domain name. The problem " + "was: %s. Skipping.", full_path, error) return None setattr(config.namespace, "domains", domains) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index aebfd42a6..d0c2f3b91 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -650,7 +650,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage mock_lineage.configuration = { - 'renewalparams': {'authenticator': None, + 'renewalparams': {'authenticator': 'webroot', 'rsa_key_size': 'over 9000'}} with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, @@ -665,9 +665,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage - mock_rc.names.return_value = ['*.example.com'] mock_lineage.configuration = { - 'renewalparams': {'authenticator': None}} + 'renewalparams': {'authenticator': 'webroot'}} + mock_lineage.names.return_value = ['*.example.com'] with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) From c0715d168bf32b6e4b3cec2e31ce06bf63be15cf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:29:04 -0800 Subject: [PATCH 08/15] test _restore_plugin_configs --- letsencrypt/tests/cli_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index d0c2f3b91..2276bad97 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -673,6 +673,24 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_plugin_config_restoration(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = { + 'renewalparams': + {'authenticator': 'webroot', + 'webroot_path': 'None', + 'webroot_imaginary_flag': '42'}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertEqual(mock_obtain_cert.call_count, 1) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 93ca160a1b84afed2bee67810eeb846bfe8d2af4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:34:58 -0800 Subject: [PATCH 09/15] test renew with -d/--csr --- letsencrypt/tests/cli_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 2276bad97..171cc0aaa 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -691,6 +691,13 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertEqual(mock_obtain_cert.call_count, 1) + def test_renew_with_bad_cli_args(self): + self.assertRaises(errors.Error, self._test_renewal_common, True, None, + args='renew -d example.com'.split(), renew=False) + self.assertRaises(errors.Error, self._test_renewal_common, True, None, + args='renew --csr {0}'.format(CSR).split(), + renew=False) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From d7be27fd847dcc6fb8e6ab829c8de9418bb45f3a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:38:18 -0800 Subject: [PATCH 10/15] Test renew catches all exceptions from reconstitute --- letsencrypt/tests/cli_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 171cc0aaa..a9c885f27 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -691,6 +691,19 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertEqual(mock_obtain_cert.call_count, 1) + def test_renew_reconstitute_error(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + # pylint: disable=protected-access + with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: + mock_reconstitute.side_effect = [Exception] + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + def test_renew_with_bad_cli_args(self): self.assertRaises(errors.Error, self._test_renewal_common, True, None, args='renew -d example.com'.split(), renew=False) From fdb9857dd8b16743286e5f28ad624a56ad242c00 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:54:46 -0800 Subject: [PATCH 11/15] test obtain_cert error is caught by renew --- letsencrypt/tests/cli_test.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a9c885f27..3647060d3 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -698,12 +698,27 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods f.write("My contents don't matter") # pylint: disable=protected-access with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: - mock_reconstitute.side_effect = [Exception] + mock_reconstitute.side_effect = Exception with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_obtain_cert_error(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = { + 'renewalparams': {'authenticator': 'webroot'}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + mock_obtain_cert.side_effect = Exception + self._test_renewal_common(True, None, + args=['renew'], renew=False) + def test_renew_with_bad_cli_args(self): self.assertRaises(errors.Error, self._test_renewal_common, True, None, args='renew -d example.com'.split(), renew=False) From 71faa50820db415186fdd0f59a08de4bef00e350 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 13:12:18 -0800 Subject: [PATCH 12/15] duplication-- --- letsencrypt/tests/cli_test.py | 99 +++++++++++++---------------------- 1 file changed, 35 insertions(+), 64 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 3647060d3..c41f45116 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -622,93 +622,64 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods pass # leave the file empty self.test_renew_verb() - def test_renew_sparse_config(self): + def _make_dummy_renewal_config(self): renewer_configs_dir = os.path.join(self.config_dir, 'renewal') os.makedirs(renewer_configs_dir) with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: f.write("My contents don't matter") + + def _test_renew_common(self, renewalparams=None, + names=None, assert_oc_called=None): + self._make_dummy_renewal_config() with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() + if renewalparams is not None: + mock_lineage.configuration = {'renewalparams': renewalparams} + if names is not None: + mock_lineage.names.return_value = names mock_rc.return_value = mock_lineage - mock_lineage.configuration = ['not renewalparams'] with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) - mock_lineage.configuration = {'renewalparams': ['no auth']} - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + if assert_oc_called is not None: + if assert_oc_called: + self.assertTrue(mock_obtain_cert.called) + else: + self.assertFalse(mock_obtain_cert.called) + + def test_renew_no_renewalparams(self): + self._test_renew_common(assert_oc_called=False) + + def test_renew_no_authenticator(self): + self._test_renew_common(renewalparams={}, assert_oc_called=False) def test_renew_with_bad_int(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") - with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: - mock_lineage = mock.MagicMock() - mock_rc.return_value = mock_lineage - mock_lineage.configuration = { - 'renewalparams': {'authenticator': 'webroot', - 'rsa_key_size': 'over 9000'}} - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + renewalparams = {'authenticator': 'webroot', + 'rsa_key_size': 'over 9000'} + self._test_renew_common(renewalparams=renewalparams, + assert_oc_called=False) def test_renew_with_bad_domain(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") - with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: - mock_lineage = mock.MagicMock() - mock_rc.return_value = mock_lineage - mock_lineage.configuration = { - 'renewalparams': {'authenticator': 'webroot'}} - mock_lineage.names.return_value = ['*.example.com'] - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + renewalparams = {'authenticator': 'webroot'} + names = ['*.example.com'] + self._test_renew_common(renewalparams=renewalparams, + names=names, assert_oc_called=False) def test_renew_plugin_config_restoration(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") - with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: - mock_lineage = mock.MagicMock() - mock_rc.return_value = mock_lineage - mock_lineage.configuration = { - 'renewalparams': - {'authenticator': 'webroot', - 'webroot_path': 'None', - 'webroot_imaginary_flag': '42'}} - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertEqual(mock_obtain_cert.call_count, 1) + renewalparams = {'authenticator': 'webroot', + 'webroot_path': 'None', + 'webroot_imaginary_flag': '42'} + self._test_renew_common(renewalparams=renewalparams, + assert_oc_called=True) def test_renew_reconstitute_error(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") # pylint: disable=protected-access with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: mock_reconstitute.side_effect = Exception - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + self._test_renew_common(assert_oc_called=False) def test_renew_obtain_cert_error(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") + self._make_dummy_renewal_config() with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage From 72dfaea434723245ff58e802ebc7d07324609fee Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 13:35:54 -0800 Subject: [PATCH 13/15] Fix ACME error reporting regression --- letsencrypt/auth_handler.py | 7 ++++++- letsencrypt/tests/auth_handler_test.py | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 45c51a020..3b8c5e393 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -480,6 +480,9 @@ def is_preferred(offered_challb, satisfied, return True +_ACME_PREFIX = "urn:acme:error:" + + _ERROR_HELP_COMMON = ( "To fix these errors, please make sure that your domain name was entered " "correctly and the DNS A record(s) for that domain contain(s) the " @@ -540,11 +543,13 @@ def _generate_failed_chall_msg(failed_achalls): """ typ = failed_achalls[0].error.typ + if typ.startswith(_ACME_PREFIX): + typ = typ[len(_ACME_PREFIX):] msg = ["The following errors were reported by the server:"] for achall in failed_achalls: msg.append("\n\nDomain: %s\nType: %s\nDetail: %s" % ( - achall.domain, achall.error.typ, achall.error.detail)) + achall.domain, typ, achall.error.detail)) if typ in _ERROR_HELP: msg.append("\n\n") diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 5b4c2bfc7..5a6199ca3 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -437,9 +437,12 @@ class ReportFailedChallsTest(unittest.TestCase): "chall": acme_util.HTTP01, "uri": "uri", "status": messages.STATUS_INVALID, - "error": messages.Error(typ="tls", detail="detail"), + "error": messages.Error(typ="urn:acme:error:tls", detail="detail"), } + # Prevent future regressions if the error type changes + self.assertTrue(kwargs["error"].description is not None) + self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge( # pylint: disable=star-args challb=messages.ChallengeBody(**kwargs), From 5ef3e5399d578e8cb587392be821a6a9df74d565 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 14:06:34 -0800 Subject: [PATCH 14/15] Add webroot help to connection message --- letsencrypt/auth_handler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 3b8c5e393..ffbd70ced 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -493,7 +493,9 @@ _ERROR_HELP = { "connection": _ERROR_HELP_COMMON + " Additionally, please check that your computer " "has a publicly routable IP address and that no firewalls are preventing " - "the server from communicating with the client.", + "the server from communicating with the client. If you're using the " + "webroot plugin, you should also verify that you are serving files " + "from the webroot path you provided.", "dnssec": _ERROR_HELP_COMMON + " Additionally, if you have DNSSEC enabled for " "your domain, please ensure that the signature is valid.", From c23aa37f4b910e5357c191b729d50e2d7042a715 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Feb 2016 18:06:46 -0800 Subject: [PATCH 15/15] 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()