From 685fa7768481585d6637efda38981c0832bbb425 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 15:02:52 -0800 Subject: [PATCH 01/28] Add --dry-run flag --- letsencrypt/cli.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 61bc85e72..bd77e23e0 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1207,6 +1207,10 @@ def _paths_parser(helpful): add("testing", "--test-cert", "--staging", action='store_true', dest='staging', help='Use the staging server to obtain test (invalid) certs; equivalent' ' to --server ' + constants.STAGING_URI) + add("testing", "--dry-run", action="store_true", dest="dry_run", + help="Perform a test run of the client, obtaining test (invalid) certs" + " but not saving them to disk. This can currently only be used" + " with the 'certonly' subcommand.") def _plugins_parsing(helpful, plugins): From 6797009dd06da2765b6cbcd30e126f7993791a53 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 16:48:43 -0800 Subject: [PATCH 02/28] Simplified calls to prepare_and_parse_args --- letsencrypt/tests/cli_test.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ca5830ed0..2a40a199a 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -1,5 +1,6 @@ """Tests for letsencrypt.cli.""" import argparse +import functools import itertools import os import shutil @@ -349,45 +350,49 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call, ['-d', '*.wildcard.tld']) - def test_parse_domains(self): + def _get_argument_parser(self): plugins = disco.PluginsRegistry.find_all() + return functools.partial(cli.prepare_and_parse_args, plugins) + + def test_parse_domains(self): + parse = self._get_argument_parser() short_args = ['-d', 'example.com'] - namespace = cli.prepare_and_parse_args(plugins, short_args) + namespace = parse(short_args) self.assertEqual(namespace.domains, ['example.com']) short_args = ['-d', 'example.com,another.net,third.org,example.com'] - namespace = cli.prepare_and_parse_args(plugins, short_args) + namespace = parse(short_args) self.assertEqual(namespace.domains, ['example.com', 'another.net', 'third.org']) long_args = ['--domains', 'example.com'] - namespace = cli.prepare_and_parse_args(plugins, long_args) + namespace = parse(long_args) self.assertEqual(namespace.domains, ['example.com']) long_args = ['--domains', 'example.com,another.net,example.com'] - namespace = cli.prepare_and_parse_args(plugins, long_args) + namespace = parse(long_args) self.assertEqual(namespace.domains, ['example.com', 'another.net']) def test_parse_server(self): - plugins = disco.PluginsRegistry.find_all() + parse = self._get_argument_parser() short_args = ['--server', 'example.com'] - namespace = cli.prepare_and_parse_args(plugins, short_args) + namespace = parse(short_args) self.assertEqual(namespace.server, 'example.com') short_args = ['--staging'] - namespace = cli.prepare_and_parse_args(plugins, short_args) + namespace = parse(short_args) self.assertEqual(namespace.server, constants.STAGING_URI) short_args = ['--staging', '--server', 'example.com'] - self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args) + self.assertRaises(errors.Error, parse, short_args) def test_parse_webroot(self): - plugins = disco.PluginsRegistry.find_all() + parse = self._get_argument_parser() webroot_args = ['--webroot', '-w', '/var/www/example', '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous', '-d', 'superfluo.us', '-d', 'www.superfluo.us'] - namespace = cli.prepare_and_parse_args(plugins, webroot_args) + namespace = parse(webroot_args) self.assertEqual(namespace.webroot_map, { 'example.com': '/var/www/example', 'www.example.com': '/var/www/example', @@ -395,10 +400,10 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods 'superfluo.us': '/var/www/superfluous'}) webroot_args = ['-d', 'stray.example.com'] + webroot_args - self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args) + self.assertRaises(errors.Error, parse, webroot_args) webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}'] - namespace = cli.prepare_and_parse_args(plugins, webroot_map_args) + namespace = parse(webroot_map_args) self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"}) @mock.patch('letsencrypt.cli._suggest_donate') From 5c528849393157b85f8574b404858182285fa22c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 17:41:47 -0800 Subject: [PATCH 03/28] Refactor --server/--staging tests to simplify --dry-run tests --- letsencrypt/tests/cli_test.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 2a40a199a..e9bb8971d 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -374,18 +374,30 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = parse(long_args) self.assertEqual(namespace.domains, ['example.com', 'another.net']) - def test_parse_server(self): + def test_server_flag(self): parse = self._get_argument_parser() - short_args = ['--server', 'example.com'] - namespace = parse(short_args) + namespace = parse('--server example.com'.split()) self.assertEqual(namespace.server, 'example.com') + def _check_server_conflict_message(self, parser_args, conflicting_args): + parse = self._get_argument_parser() + try: + parse(parser_args) + self.fail("The following flags didn't conflict with " + '--server: {0}'.format(', '.join(conflicting_args))) + except errors.Error as error: + self.assertTrue('--server' in error.message) + for arg in conflicting_args: + self.assertTrue(arg in error.message) + + def test_staging_flag(self): + parse = self._get_argument_parser() short_args = ['--staging'] namespace = parse(short_args) self.assertEqual(namespace.server, constants.STAGING_URI) - short_args = ['--staging', '--server', 'example.com'] - self.assertRaises(errors.Error, parse, short_args) + short_args += '--server example.com'.split() + self._check_server_conflict_message(short_args, '--staging') def test_parse_webroot(self): parse = self._get_argument_parser() From 3d840dc11d087ec8ca8e9cc9e475ba1630bc93cf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 18:00:28 -0800 Subject: [PATCH 04/28] Add --dry-run parsing --- letsencrypt/cli.py | 19 ++++++++++++++----- letsencrypt/tests/cli_test.py | 23 +++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bd77e23e0..25f82f7b4 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -827,15 +827,24 @@ class HelpfulArgumentParser(object): parsed_args.verb = self.verb # Do any post-parsing homework here + if parsed_args.staging or parsed_args.dry_run: + if (parsed_args.server not in + (flag_default("server"), constants.STAGING_URI)): + conflicts = ["--staging"] if parsed_args.staging else [] + if parsed_args.dry_run: + conflicts.append("--dry-run") + raise errors.Error("--server value conflicts with {0}".format( + " and ".join(conflicts))) - # argparse seemingly isn't flexible enough to give us this behaviour easily... - if parsed_args.staging: - if parsed_args.server not in (flag_default("server"), constants.STAGING_URI): - raise errors.Error("--server value conflicts with --staging") parsed_args.server = constants.STAGING_URI - return parsed_args + if parsed_args.dry_run: + if self.verb != "certonly": + raise errors.Error("--dry-run currently only works with the " + "'certonly' subcommand") + parsed_args.staging = True + return parsed_args def determine_verb(self): """Determines the verb/subcommand provided by the user. diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index e9bb8971d..24c5c5c82 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -394,11 +394,34 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods parse = self._get_argument_parser() short_args = ['--staging'] namespace = parse(short_args) + self.assertTrue(namespace.staging) self.assertEqual(namespace.server, constants.STAGING_URI) short_args += '--server example.com'.split() self._check_server_conflict_message(short_args, '--staging') + def _assert_dry_run_flag_worked(self, namespace): + self.assertTrue(namespace.dry_run) + self.assertTrue(namespace.staging) + self.assertEqual(namespace.server, constants.STAGING_URI) + + def test_dry_run_flag(self): + parse = self._get_argument_parser() + short_args = ['--dry-run'] + self.assertRaises(errors.Error, parse, short_args) + + self._assert_dry_run_flag_worked(parse(short_args + ['auth'])) + short_args += ['certonly'] + self._assert_dry_run_flag_worked(parse(short_args)) + + short_args += '--server example.com'.split() + conflicts = ['--dry-run'] + self._check_server_conflict_message(short_args, '--dry-run') + + short_args += ['--staging'] + conflicts += ['--staging'] + self._check_server_conflict_message(short_args, conflicts) + def test_parse_webroot(self): parse = self._get_argument_parser() webroot_args = ['--webroot', '-w', '/var/www/example', From d56d15225ec4ced0ea2a2b1b3ee3bfe10ea8c88a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 18:09:59 -0800 Subject: [PATCH 05/28] Add --dry-run support to obtain_and_enroll_cert --- letsencrypt/client.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index c2dfca1bf..54bf21d87 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -276,8 +276,8 @@ class Client(object): :param plugins: A PluginsFactory object. :returns: A new :class:`letsencrypt.storage.RenewableCert` instance - referred to the enrolled cert lineage, or False if the cert could - not be obtained. + referred to the enrolled cert lineage, False if the cert could not + be obtained, or None if doing a successful dry run. """ certr, chain, key, _ = self.obtain_certificate(domains) @@ -298,12 +298,14 @@ class Client(object): "Non-standard path(s), might not work with crontab installed " "by your operating system package manager") - lineage = storage.RenewableCert.new_lineage( - domains[0], OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped), - key.pem, crypto_util.dump_pyopenssl_chain(chain), - params, config, cli_config) - return lineage + if cli_config.dry_run: + return None + else: + return storage.RenewableCert.new_lineage( + domains[0], OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped), + key.pem, crypto_util.dump_pyopenssl_chain(chain), + params, config, cli_config) def save_certificate(self, certr, chain_cert, cert_path, chain_path, fullchain_path): From 0db36afa09b6b8cf90c1d1d674fdcb23e3a6443f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 18:14:13 -0800 Subject: [PATCH 06/28] Add --dry-run support when getting a new cert --- letsencrypt/cli.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 25f82f7b4..badeebd8f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -416,13 +416,15 @@ def _auth_from_domains(le_client, config, domains): elif action == "newcert": # TREAT AS NEW REQUEST lineage = le_client.obtain_and_enroll_certificate(domains) - if not lineage: + if lineage is False: raise errors.Error("Certificate could not be obtained") - _report_new_cert(lineage.cert, lineage.fullchain) + if lineage is not None: + _report_new_cert(lineage.cert, lineage.fullchain) return lineage, action + def _avoid_invalidating_lineage(config, lineage, original_server): "Do not renew a valid cert with one from a staging server!" def _is_staging(srv): From 688b92f52874c12e557a19990f27eb3cd8e824ef Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 18:19:01 -0800 Subject: [PATCH 07/28] Add --dry-run support when renewing a lineage --- letsencrypt/cli.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index badeebd8f..cc0c99d86 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -404,12 +404,13 @@ def _auth_from_domains(le_client, config, domains): # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) # TODO: Check whether it worked! <- or make sure errors are thrown (jdk) - lineage.save_successor( - lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), - new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) + if not config.dry_run: + lineage.save_successor( + lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), + new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) - lineage.update_all_links_to(lineage.latest_common_version()) + lineage.update_all_links_to(lineage.latest_common_version()) # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant # configuration values from this attempt? <- Absolutely (jdkasten) @@ -419,7 +420,7 @@ def _auth_from_domains(le_client, config, domains): if lineage is False: raise errors.Error("Certificate could not be obtained") - if lineage is not None: + if not config.dry_run: _report_new_cert(lineage.cert, lineage.fullchain) return lineage, action From c816bfd0b7feb07411896bd8095dfe19b70bf822 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 28 Jan 2016 18:24:47 -0800 Subject: [PATCH 08/28] --dry-run implies --break-my-certs --- letsencrypt/cli.py | 2 +- letsencrypt/tests/cli_test.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index cc0c99d86..41e123a52 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -845,7 +845,7 @@ class HelpfulArgumentParser(object): if self.verb != "certonly": raise errors.Error("--dry-run currently only works with the " "'certonly' subcommand") - parsed_args.staging = True + parsed_args.break_my_certs = parsed_args.staging = True return parsed_args diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 24c5c5c82..ab7d8b025 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -402,6 +402,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def _assert_dry_run_flag_worked(self, namespace): self.assertTrue(namespace.dry_run) + self.assertTrue(namespace.break_my_certs) self.assertTrue(namespace.staging) self.assertEqual(namespace.server, constants.STAGING_URI) From a2cca2050046a2ed3e9f6646553adae460da8589 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 29 Jan 2016 10:47:58 -0800 Subject: [PATCH 09/28] Add --dry-run support when using custom csr --- letsencrypt/cli.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 41e123a52..9295144a5 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -634,9 +634,10 @@ def obtain_cert(args, config, plugins): if args.csr is not None: certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR( file=args.csr[0], data=args.csr[1], form="der")) - cert_path, _, cert_fullchain = le_client.save_certificate( - certr, chain, args.cert_path, args.chain_path, args.fullchain_path) - _report_new_cert(cert_path, cert_fullchain) + if not args.dry_run: + cert_path, _, cert_fullchain = le_client.save_certificate( + certr, chain, args.cert_path, args.chain_path, args.fullchain_path) + _report_new_cert(cert_path, cert_fullchain) else: domains = _find_domains(args, installer) _auth_from_domains(le_client, config, domains) From 639444bf5c3243627c2d4737d64e02f50377b837 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 29 Jan 2016 12:07:38 -0800 Subject: [PATCH 10/28] Add message on successful dry run --- letsencrypt/cli.py | 11 ++++++++++- letsencrypt/tests/cli_test.py | 32 +++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 9295144a5..15a483b68 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -383,6 +383,12 @@ def _suggest_donate(): reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) +def _report_successful_dry_run(): + reporter_util = zope.component.getUtility(interfaces.IReporter) + reporter_util.add_message("The dry run was successful.", + reporter_util.HIGH_PRIORITY, on_crash=False) + + def _auth_from_domains(le_client, config, domains): """Authenticate and enroll certificate.""" # Note: This can raise errors... caught above us though. This is now @@ -642,7 +648,10 @@ def obtain_cert(args, config, plugins): domains = _find_domains(args, installer) _auth_from_domains(le_client, config, domains) - _suggest_donate() + if args.dry_run: + _report_successful_dry_run() + else: + _suggest_donate() def install(args, config, plugins): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ab7d8b025..b95e47987 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -383,8 +383,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods parse = self._get_argument_parser() try: parse(parser_args) - self.fail("The following flags didn't conflict with " - '--server: {0}'.format(', '.join(conflicting_args))) + self.fail( # pragma: no cover + "The following flags didn't conflict with " + '--server: {0}'.format(', '.join(conflicting_args))) except errors.Error as error: self.assertTrue('--server' in error.message) for arg in conflicting_args: @@ -442,6 +443,26 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = parse(webroot_map_args) self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"}) + def _certonly_new_request_common(self, mock_client, args=None): + with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal: + mock_renewal.return_value = ("newcert", None) + with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + mock_init.return_value = mock_client + if args is None: + args = [] + args += '-d foo.bar -a standalone certonly'.split() + self._call(args) + + @mock.patch('letsencrypt.cli.zope.component.getUtility') + def test_certonly_dry_run_success(self, mock_get_utility): + mock_client = mock.MagicMock() + mock_client.obtain_and_enroll_certificate.return_value = None + self._certonly_new_request_common(mock_client, ['--dry-run']) + self.assertEqual( + mock_client.obtain_and_enroll_certificate.call_count, 1) + self.assertTrue( + 'dry run' in mock_get_utility().add_message.call_args[0][0]) + @mock.patch('letsencrypt.cli._suggest_donate') @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.zope.component.getUtility') @@ -467,13 +488,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, self._certonly_new_request_common, mock_client) - def _certonly_new_request_common(self, mock_client): - with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal: - mock_renewal.return_value = ("newcert", None) - with mock.patch('letsencrypt.cli._init_le_client') as mock_init: - mock_init.return_value = mock_client - self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly']) - @mock.patch('letsencrypt.cli._suggest_donate') @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From 08698a3de2da1ad1c77771073e47054c3e16ce64 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 29 Jan 2016 17:25:33 -0800 Subject: [PATCH 11/28] Log when skipping functions due to --dry-run in cli.py --- letsencrypt/cli.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 15a483b68..db8cd722d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -410,7 +410,10 @@ def _auth_from_domains(le_client, config, domains): # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) # TODO: Check whether it worked! <- or make sure errors are thrown (jdk) - if not config.dry_run: + if config.dry_run: + logger.info("Dry run: skipping updating lineage at %s", + os.path.dirname(lineage.cert)) + else: lineage.save_successor( lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), @@ -640,7 +643,10 @@ def obtain_cert(args, config, plugins): if args.csr is not None: certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR( file=args.csr[0], data=args.csr[1], form="der")) - if not args.dry_run: + if args.dry_run: + logger.info( + "Dry run: skipping saving certificate to %s", args.cert_path) + else: cert_path, _, cert_fullchain = le_client.save_certificate( certr, chain, args.cert_path, args.chain_path, args.fullchain_path) _report_new_cert(cert_path, cert_fullchain) From 5c363b5b984c88f62716e7643eaa629bd227c2fd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 29 Jan 2016 17:43:21 -0800 Subject: [PATCH 12/28] Log when skipping functions due to --dry-run in client.py --- letsencrypt/client.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 54bf21d87..0b258b7eb 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -299,6 +299,8 @@ class Client(object): "by your operating system package manager") if cli_config.dry_run: + logger.info("Dry run: Skipping creating new lineage for %s", + domains[0]) return None else: return storage.RenewableCert.new_lineage( From 29bed26aa14bdb71b8ce388a01eadb14a5e1dda5 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 29 Jan 2016 18:04:51 -0800 Subject: [PATCH 13/28] Make addition of conflicting flags look similar --- letsencrypt/cli.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index db8cd722d..6f9d9dbc7 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -850,8 +850,7 @@ class HelpfulArgumentParser(object): if (parsed_args.server not in (flag_default("server"), constants.STAGING_URI)): conflicts = ["--staging"] if parsed_args.staging else [] - if parsed_args.dry_run: - conflicts.append("--dry-run") + conflicts += ["--dry-run"] if parsed_args.dry_run else [] raise errors.Error("--server value conflicts with {0}".format( " and ".join(conflicts))) From f9ac25cd7e333cc250d5e397160e71e461a0c3d7 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 29 Jan 2016 18:14:53 -0800 Subject: [PATCH 14/28] Only suggest donations sometimes --- letsencrypt/cli.py | 21 +++++++++++---------- letsencrypt/tests/cli_test.py | 12 ++++++------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 6f9d9dbc7..1fd209d69 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -374,13 +374,15 @@ def _report_new_cert(cert_path, fullchain_path): .format(and_chain, path, expiry)) reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY) -def _suggest_donate(): - "Suggest a donation to support Let's Encrypt" - reporter_util = zope.component.getUtility(interfaces.IReporter) - msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" - "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" - "Donating to EFF: https://eff.org/donate-le\n\n") - reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) + +def _suggest_donation_if_appropriate(config): + """Potentially suggest a donation to support Let's Encrypt.""" + if not config.staging: # --dry-run implies --staging + reporter_util = zope.component.getUtility(interfaces.IReporter) + msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" + "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" + "Donating to EFF: https://eff.org/donate-le\n\n") + reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) def _report_successful_dry_run(): @@ -620,7 +622,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo else: display_ops.success_renewal(domains, action) - _suggest_donate() + _suggest_donation_if_appropriate(config) def obtain_cert(args, config, plugins): @@ -656,8 +658,7 @@ def obtain_cert(args, config, plugins): if args.dry_run: _report_successful_dry_run() - else: - _suggest_donate() + _suggest_donation_if_appropriate(config) def install(args, config, plugins): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index b95e47987..bb593437b 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -50,7 +50,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def _call(self, args): "Run the cli with output streams and actual client mocked out" - with mock.patch('letsencrypt.cli._suggest_donate'): + with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'): with mock.patch('letsencrypt.cli.client') as client: ret, stdout, stderr = self._call_no_clientmock(args) return ret, stdout, stderr, client @@ -58,7 +58,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def _call_no_clientmock(self, args): "Run the client with output streams mocked out" args = self.standard_args + args - with mock.patch('letsencrypt.cli._suggest_donate'): + with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'): with mock.patch('letsencrypt.cli.sys.stdout') as stdout: with mock.patch('letsencrypt.cli.sys.stderr') as stderr: ret = cli.main(args[:]) # NOTE: parser can alter its args! @@ -70,7 +70,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods caller. """ args = self.standard_args + args - with mock.patch('letsencrypt.cli._suggest_donate'): + with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'): with mock.patch('letsencrypt.cli.sys.stderr') as stderr: with mock.patch('letsencrypt.cli.client') as client: ret = cli.main(args[:]) # NOTE: parser can alter its args! @@ -463,7 +463,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue( 'dry run' in mock_get_utility().add_message.call_args[0][0]) - @mock.patch('letsencrypt.cli._suggest_donate') + @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate') @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.zope.component.getUtility') def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter, _suggest): @@ -488,7 +488,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, self._certonly_new_request_common, mock_client) - @mock.patch('letsencrypt.cli._suggest_donate') + @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate') @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') @@ -514,7 +514,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue( chain_path in mock_get_utility().add_message.call_args[0][0]) - @mock.patch('letsencrypt.cli._suggest_donate') + @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate') @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.display_ops.pick_installer') @mock.patch('letsencrypt.cli.zope.component.getUtility') From ae6e938744be95b7cf9647272c3004216eb3c955 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 29 Jan 2016 18:50:38 -0800 Subject: [PATCH 15/28] --dry-run forces simulated renewal --- letsencrypt/cli.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1fd209d69..6d55e9674 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -401,11 +401,20 @@ def _auth_from_domains(le_client, config, domains): # (which results in treating the request as a new certificate request). action, lineage = _treat_as_renewal(config, domains) - if action == "reinstall": + if action == "reinstall" and not config.dry_run: # The lineage already exists; allow the caller to try installing # it without getting a new certificate at all. return lineage, "reinstall" - elif action == "renew": + elif action == "newcert": + # TREAT AS NEW REQUEST + lineage = le_client.obtain_and_enroll_certificate(domains) + if lineage is False: + raise errors.Error("Certificate could not be obtained") + else: + assert action == "renew" or config.dry_run, "invalid auth command" + if config.dry_run and action == "reinstall": + logger.info("Cert not due for renewal, but " + "simulating renewal for dry run") original_server = lineage.configuration["renewalparams"]["server"] _avoid_invalidating_lineage(config, lineage, original_server) # TODO: schoen wishes to reuse key - discussion @@ -425,11 +434,6 @@ def _auth_from_domains(le_client, config, domains): # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant # configuration values from this attempt? <- Absolutely (jdkasten) - elif action == "newcert": - # TREAT AS NEW REQUEST - lineage = le_client.obtain_and_enroll_certificate(domains) - if lineage is False: - raise errors.Error("Certificate could not be obtained") if not config.dry_run: _report_new_cert(lineage.cert, lineage.fullchain) From 8bb7ed9a69341ba82ab9ed4ced8740b45b3680a9 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 1 Feb 2016 11:04:02 -0800 Subject: [PATCH 16/28] Document quirks of webroot-map in conf files --- letsencrypt/cli.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index dc095a42e..e55d4ec87 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1266,7 +1266,9 @@ def _plugins_parsing(helpful, plugins): help="JSON dictionary mapping domains to webroot paths; this implies -d " "for each entry. You may need to escape this from your shell. " """Eg: --webroot-map '{"eg1.is,m.eg1.is":"/www/eg1/", "eg2.is":"/www/eg2"}' """ - "This option is merged with, but takes precedence over, -w / -d entries") + "This option is merged with, but takes precedence over, -w / -d entries." + " At present, if you put webroot-map in a config file, it needs to be " + ' on a single line, like: webroot-map = {"example.com":"/var/www"}.') class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring def __init__(self, *args, **kwargs): From f5fa64ee9a23fc8a6dded29c2ea895653ebf30e6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 12:01:12 -0800 Subject: [PATCH 17/28] Test _suggest_donation_if_appropriate --- letsencrypt/tests/cli_test.py | 50 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index c94336e66..b52ca6163 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -50,18 +50,16 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def _call(self, args): "Run the cli with output streams and actual client mocked out" - with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'): - with mock.patch('letsencrypt.cli.client') as client: - ret, stdout, stderr = self._call_no_clientmock(args) - return ret, stdout, stderr, client + with mock.patch('letsencrypt.cli.client') as client: + ret, stdout, stderr = self._call_no_clientmock(args) + return ret, stdout, stderr, client def _call_no_clientmock(self, args): "Run the client with output streams mocked out" args = self.standard_args + args - with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'): - with mock.patch('letsencrypt.cli.sys.stdout') as stdout: - with mock.patch('letsencrypt.cli.sys.stderr') as stderr: - ret = cli.main(args[:]) # NOTE: parser can alter its args! + with mock.patch('letsencrypt.cli.sys.stdout') as stdout: + with mock.patch('letsencrypt.cli.sys.stderr') as stderr: + ret = cli.main(args[:]) # NOTE: parser can alter its args! return ret, stdout, stderr def _call_stdout(self, args): @@ -70,10 +68,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods caller. """ args = self.standard_args + args - with mock.patch('letsencrypt.cli._suggest_donation_if_appropriate'): - with mock.patch('letsencrypt.cli.sys.stderr') as stderr: - with mock.patch('letsencrypt.cli.client') as client: - ret = cli.main(args[:]) # NOTE: parser can alter its args! + with mock.patch('letsencrypt.cli.sys.stderr') as stderr: + with mock.patch('letsencrypt.cli.client') as client: + ret = cli.main(args[:]) # NOTE: parser can alter its args! return ret, None, stderr, client def test_no_flags(self): @@ -505,11 +502,12 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_client.obtain_and_enroll_certificate.call_count, 1) self.assertTrue( 'dry run' in mock_get_utility().add_message.call_args[0][0]) + # Asserts we don't suggest donating after a successful dry run + self.assertEqual(mock_get_utility().add_message.call_count, 1) - @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate') @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.zope.component.getUtility') - def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter, _suggest): + def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter): cert_path = '/etc/letsencrypt/live/foo.bar' date = '1970-01-01' mock_notAfter().date.return_value = date @@ -520,10 +518,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._certonly_new_request_common(mock_client) self.assertEqual( mock_client.obtain_and_enroll_certificate.call_count, 1) + cert_msg = mock_get_utility().add_message.call_args_list[0][0][0] + self.assertTrue(cert_path in cert_msg) + self.assertTrue(date in cert_msg) self.assertTrue( - cert_path in mock_get_utility().add_message.call_args[0][0]) - self.assertTrue( - date in mock_get_utility().add_message.call_args[0][0]) + 'donate' in mock_get_utility().add_message.call_args[0][0]) def test_certonly_new_request_failure(self): mock_client = mock.MagicMock() @@ -531,11 +530,10 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, self._certonly_new_request_common, mock_client) - @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate') @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') - def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility, _suggest): + def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility): cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' @@ -554,17 +552,18 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(mock_lineage.save_successor.call_count, 1) mock_lineage.update_all_links_to.assert_called_once_with( mock_lineage.latest_common_version()) + cert_msg = mock_get_utility().add_message.call_args_list[0][0][0] + self.assertTrue(chain_path in cert_msg) self.assertTrue( - chain_path in mock_get_utility().add_message.call_args[0][0]) + 'donate' in mock_get_utility().add_message.call_args[0][0]) - @mock.patch('letsencrypt.cli._suggest_donation_if_appropriate') @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.display_ops.pick_installer') @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._init_le_client') @mock.patch('letsencrypt.cli.record_chosen_plugins') def test_certonly_csr(self, _rec, mock_init, mock_get_utility, - mock_pick_installer, mock_notAfter, _suggest): + mock_pick_installer, mock_notAfter): cert_path = '/etc/letsencrypt/live/blahcert.pem' date = '1970-01-01' mock_notAfter().date.return_value = date @@ -583,10 +582,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(mock_pick_installer.call_args[0][1], installer) mock_client.save_certificate.assert_called_once_with( 'certr', 'chain', cert_path, '/', '/') + cert_msg = mock_get_utility().add_message.call_args_list[0][0][0] + self.assertTrue(cert_path in cert_msg) + self.assertTrue(date in cert_msg) self.assertTrue( - cert_path in mock_get_utility().add_message.call_args[0][0]) - self.assertTrue( - date in mock_get_utility().add_message.call_args[0][0]) + 'donate' in mock_get_utility().add_message.call_args[0][0]) @mock.patch('letsencrypt.cli.client.acme_client') def test_revoke_with_key(self, mock_acme_client): From 32034552fd69e732617a582e75a6ef9943125a80 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 12:18:43 -0800 Subject: [PATCH 18/28] Test reinstall --- letsencrypt/tests/cli_test.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index b52ca6163..cce7ca201 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -494,7 +494,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call(args) @mock.patch('letsencrypt.cli.zope.component.getUtility') - def test_certonly_dry_run_success(self, mock_get_utility): + def test_certonly_dry_run_new_request_success(self, mock_get_utility): mock_client = mock.MagicMock() mock_client.obtain_and_enroll_certificate.return_value = None self._certonly_new_request_common(mock_client, ['--dry-run']) @@ -557,6 +557,18 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue( 'donate' in mock_get_utility().add_message.call_args[0][0]) + @mock.patch('letsencrypt.cli.zope.component.getUtility') + @mock.patch('letsencrypt.cli._treat_as_renewal') + @mock.patch('letsencrypt.cli._init_le_client') + def test_certonly_reinstall(self, mock_init, mock_renewal, mock_get_utility): + mock_renewal.return_value = ('reinstall', mock.MagicMock()) + mock_init.return_value = mock_client = mock.MagicMock() + self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly']) + self.assertFalse(mock_client.obtain_certificate.called) + self.assertFalse(mock_client.obtain_and_enroll_certificate.called) + self.assertTrue( + 'donate' in mock_get_utility().add_message.call_args[0][0]) + @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.display_ops.pick_installer') @mock.patch('letsencrypt.cli.zope.component.getUtility') From 204f8ba0f232634aade7b512bffc9d685ebedb09 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 12:45:13 -0800 Subject: [PATCH 19/28] Refactor test_certonly_renewal --- letsencrypt/tests/cli_test.py | 36 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index cce7ca201..b065a4590 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -530,30 +530,40 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, self._certonly_new_request_common, mock_client) - @mock.patch('letsencrypt.cli.zope.component.getUtility') - @mock.patch('letsencrypt.cli._treat_as_renewal') - @mock.patch('letsencrypt.cli._init_le_client') - def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility): + def _test_certonly_renewal_common(self, renewal_verb, extra_args=None): cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' - mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) mock_certr = mock.MagicMock() mock_key = mock.MagicMock(pem='pem_key') - mock_renewal.return_value = ("renew", mock_lineage) - mock_client = mock.MagicMock() - mock_client.obtain_certificate.return_value = (mock_certr, 'chain', - mock_key, 'csr') - mock_init.return_value = mock_client - with mock.patch('letsencrypt.cli.OpenSSL'): - with mock.patch('letsencrypt.cli.crypto_util'): - self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly']) + with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal: + mock_renewal.return_value = (renewal_verb, mock_lineage) + mock_client = mock.MagicMock() + mock_client.obtain_certificate.return_value = (mock_certr, 'chain', + mock_key, 'csr') + with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + mock_init.return_value = mock_client + get_utility_path = 'letsencrypt.cli.zope.component.getUtility' + with mock.patch(get_utility_path) as mock_get_utility: + with mock.patch('letsencrypt.cli.OpenSSL'): + with mock.patch('letsencrypt.cli.crypto_util'): + args = ['-d', 'foo.bar', '-a', + 'standalone', 'certonly'] + if extra_args: + args += extra_args + self._call(args) + mock_client.obtain_certificate.assert_called_once_with(['foo.bar']) self.assertEqual(mock_lineage.save_successor.call_count, 1) mock_lineage.update_all_links_to.assert_called_once_with( mock_lineage.latest_common_version()) cert_msg = mock_get_utility().add_message.call_args_list[0][0][0] self.assertTrue(chain_path in cert_msg) + + return mock_get_utility + + def test_certonly_renewal(self): + mock_get_utility = self._test_certonly_renewal_common("renew") self.assertTrue( 'donate' in mock_get_utility().add_message.call_args[0][0]) From f6a3355d28798c21abfedaec15c66c760d1878c1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 12:55:41 -0800 Subject: [PATCH 20/28] test reinstall + dry-run = renew --- letsencrypt/tests/cli_test.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index b065a4590..35a0153a4 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -554,18 +554,23 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call(args) mock_client.obtain_certificate.assert_called_once_with(['foo.bar']) - self.assertEqual(mock_lineage.save_successor.call_count, 1) - mock_lineage.update_all_links_to.assert_called_once_with( - mock_lineage.latest_common_version()) - cert_msg = mock_get_utility().add_message.call_args_list[0][0][0] - self.assertTrue(chain_path in cert_msg) - return mock_get_utility + return mock_lineage, mock_get_utility def test_certonly_renewal(self): - mock_get_utility = self._test_certonly_renewal_common("renew") - self.assertTrue( - 'donate' in mock_get_utility().add_message.call_args[0][0]) + lineage, get_utility = self._test_certonly_renewal_common('renew') + self.assertEqual(lineage.save_successor.call_count, 1) + lineage.update_all_links_to.assert_called_once_with( + lineage.latest_common_version()) + cert_msg = get_utility().add_message.call_args_list[0][0][0] + self.assertTrue('fullchain.pem' in cert_msg) + self.assertTrue('donate' in get_utility().add_message.call_args[0][0]) + + def test_certonly_dry_run_reinstall_is_renewal(self): + _, get_utility = self._test_certonly_renewal_common('reinstall', + ['--dry-run']) + self.assertEqual(get_utility().add_message.call_count, 1) + self.assertTrue('dry run' in get_utility().add_message.call_args[0][0]) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') From 5e9d5efdcbfad75fe2a95b4dd22786ca6d9d2bfe Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 13:20:46 -0800 Subject: [PATCH 21/28] Refactor test_certonly_csr --- letsencrypt/tests/cli_test.py | 48 ++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 35a0153a4..84c976177 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -584,34 +584,36 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue( 'donate' in mock_get_utility().add_message.call_args[0][0]) - @mock.patch('letsencrypt.crypto_util.notAfter') - @mock.patch('letsencrypt.cli.display_ops.pick_installer') - @mock.patch('letsencrypt.cli.zope.component.getUtility') - @mock.patch('letsencrypt.cli._init_le_client') - @mock.patch('letsencrypt.cli.record_chosen_plugins') - def test_certonly_csr(self, _rec, mock_init, mock_get_utility, - mock_pick_installer, mock_notAfter): - cert_path = '/etc/letsencrypt/live/blahcert.pem' - date = '1970-01-01' - mock_notAfter().date.return_value = date - + def _test_certonly_csr_common(self, extra_args=None): + certr = 'certr' + chain = 'chain' mock_client = mock.MagicMock() - mock_client.obtain_certificate_from_csr.return_value = ('certr', - 'chain') + mock_client.obtain_certificate_from_csr.return_value = (certr, chain) + cert_path = '/etc/letsencrypt/live/example.com/cert.pem' mock_client.save_certificate.return_value = cert_path, None, None - mock_init.return_value = mock_client + with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + mock_init.return_value = mock_client + get_utility_path = 'letsencrypt.cli.zope.component.getUtility' + with mock.patch(get_utility_path) as mock_get_utility: + chain_path = '/etc/letsencrypt/live/example.com/chain.pem' + full_path = '/etc/letsencrypt/live/example.com/fullchain.pem' + args = ('-a standalone certonly --csr {0} --cert-path {1} ' + '--chain-path {2} --fullchain-path {3}').format( + CSR, cert_path, chain_path, full_path).split() + if extra_args: + args += extra_args + with mock.patch('letsencrypt.cli.crypto_util'): + self._call(args) - installer = 'installer' - self._call( - ['-a', 'standalone', '-i', installer, 'certonly', '--csr', CSR, - '--cert-path', cert_path, '--fullchain-path', '/', - '--chain-path', '/']) - self.assertEqual(mock_pick_installer.call_args[0][1], installer) mock_client.save_certificate.assert_called_once_with( - 'certr', 'chain', cert_path, '/', '/') + certr, chain, cert_path, chain_path, full_path) + + return mock_get_utility + + def test_certonly_csr(self): + mock_get_utility = self._test_certonly_csr_common() cert_msg = mock_get_utility().add_message.call_args_list[0][0][0] - self.assertTrue(cert_path in cert_msg) - self.assertTrue(date in cert_msg) + self.assertTrue('cert.pem' in cert_msg) self.assertTrue( 'donate' in mock_get_utility().add_message.call_args[0][0]) From 149ac79b8f4ceb7469bd0663c32425d2d5033124 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 13:29:18 -0800 Subject: [PATCH 22/28] Test certonly with --csr and --dry-run --- letsencrypt/tests/cli_test.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 84c976177..506e19ba8 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -605,8 +605,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.cli.crypto_util'): self._call(args) - mock_client.save_certificate.assert_called_once_with( - certr, chain, cert_path, chain_path, full_path) + if '--dry-run' in args: + self.assertFalse(mock_client.save_certificate.called) + else: + mock_client.save_certificate.assert_called_once_with( + certr, chain, cert_path, chain_path, full_path) return mock_get_utility @@ -617,6 +620,12 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue( 'donate' in mock_get_utility().add_message.call_args[0][0]) + def test_certonly_csr_dry_run(self): + mock_get_utility = self._test_certonly_csr_common(['--dry-run']) + self.assertEqual(mock_get_utility().add_message.call_count, 1) + self.assertTrue( + 'dry run' in mock_get_utility().add_message.call_args[0][0]) + @mock.patch('letsencrypt.cli.client.acme_client') def test_revoke_with_key(self, mock_acme_client): server = 'foo.bar' From c4ce168001199d2793e416a143b12281622a69d9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 17:42:43 -0800 Subject: [PATCH 23/28] Revert "--dry-run forces simulated renewal" This reverts commit ae6e938744be95b7cf9647272c3004216eb3c955. --- letsencrypt/cli.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f762076d4..db381715d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -405,20 +405,11 @@ def _auth_from_domains(le_client, config, domains): # (which results in treating the request as a new certificate request). action, lineage = _treat_as_renewal(config, domains) - if action == "reinstall" and not config.dry_run: + if action == "reinstall": # The lineage already exists; allow the caller to try installing # it without getting a new certificate at all. return lineage, "reinstall" - elif action == "newcert": - # TREAT AS NEW REQUEST - lineage = le_client.obtain_and_enroll_certificate(domains) - if lineage is False: - raise errors.Error("Certificate could not be obtained") - else: - assert action == "renew" or config.dry_run, "invalid auth command" - if config.dry_run and action == "reinstall": - logger.info("Cert not due for renewal, but " - "simulating renewal for dry run") + elif action == "renew": original_server = lineage.configuration["renewalparams"]["server"] _avoid_invalidating_lineage(config, lineage, original_server) # TODO: schoen wishes to reuse key - discussion @@ -438,6 +429,11 @@ def _auth_from_domains(le_client, config, domains): # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant # configuration values from this attempt? <- Absolutely (jdkasten) + elif action == "newcert": + # TREAT AS NEW REQUEST + lineage = le_client.obtain_and_enroll_certificate(domains) + if lineage is False: + raise errors.Error("Certificate could not be obtained") if not config.dry_run: _report_new_cert(lineage.cert, lineage.fullchain) From d18ec15165623c333563c71cfbb8653152bcdcce Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 17:46:55 -0800 Subject: [PATCH 24/28] Change action just in case --- letsencrypt/cli.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index db381715d..60580cff9 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -405,6 +405,11 @@ def _auth_from_domains(le_client, config, domains): # (which results in treating the request as a new certificate request). action, lineage = _treat_as_renewal(config, domains) + if config.dry_run and action == "reinstall": + logger.info( + "Cert not due for renewal, but simulating renewal for dry run") + action = "renew" + if action == "reinstall": # The lineage already exists; allow the caller to try installing # it without getting a new certificate at all. From 3c34fd80c7c220c6ace04135a9e13411fa12a4ca Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 18:09:47 -0800 Subject: [PATCH 25/28] Change enable_mod order --- letsencrypt-apache/letsencrypt_apache/configurator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 5ca2ddcb6..0ab16ff06 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -644,11 +644,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ if self.conf("handle-modules"): - if "ssl_module" not in self.parser.modules: - self.enable_mod("ssl", temp=temp) if self.version >= (2, 4) and ("socache_shmcb_module" not in self.parser.modules): self.enable_mod("socache_shmcb", temp=temp) + if "ssl_module" not in self.parser.modules: + self.enable_mod("ssl", temp=temp) def make_addrs_sni_ready(self, addrs): """Checks to see if the server is ready for SNI challenges. From 8ddebe3d12dd0386d9564295c8c8b7d846f6d4a1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 1 Feb 2016 18:21:02 -0800 Subject: [PATCH 26/28] Create tests to prevent future regressions --- .../letsencrypt_apache/tests/configurator_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index a04f7904c..5b15a20d1 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -429,9 +429,15 @@ class TwoVhost80Test(util.ApacheTest): self.config.parser.add_dir_to_ifmodssl = mock_add_dir self.config.prepare_server_https("443") + # Changing the order these modules are enabled breaks the reverter + self.assertEqual(mock_enable.call_args_list[0][0][0], "socache_shmcb") + self.assertEqual(mock_enable.call_args[0][0], "ssl") self.assertEqual(mock_enable.call_args[1], {"temp": False}) self.config.prepare_server_https("8080", temp=True) + # Changing the order these modules are enabled breaks the reverter + self.assertEqual(mock_enable.call_args_list[2][0][0], "socache_shmcb") + self.assertEqual(mock_enable.call_args[0][0], "ssl") # Enable mod is temporary self.assertEqual(mock_enable.call_args[1], {"temp": True}) From ea9478ebc1c39a7c232a541bed614b7f568bb292 Mon Sep 17 00:00:00 2001 From: Prayag Verma Date: Tue, 2 Feb 2016 11:23:15 +0530 Subject: [PATCH 27/28] Fix typo in docs/ciphers.rst Remove extra `as` --- docs/ciphers.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ciphers.rst b/docs/ciphers.rst index fb854f307..c8ff26117 100644 --- a/docs/ciphers.rst +++ b/docs/ciphers.rst @@ -107,7 +107,7 @@ and the version implemented by the Let's Encrypt client will be the version that was most current as of the release date of each client version. Mozilla offers three separate sets of cryptographic options, which trade off security and compatibility differently. These are -referred to as as the "Modern", "Intermediate", and "Old" configurations +referred to as the "Modern", "Intermediate", and "Old" configurations (in order from most secure to least secure, and least-backwards compatible to most-backwards compatible). The client will follow the Mozilla defaults for the *Intermediate* configuration by default, at least with regards to From d85883d55ad7be08ac3a94881ce9d873bf6ce7dc Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Tue, 2 Feb 2016 13:05:15 -0500 Subject: [PATCH 28/28] Add 2.6 dependencies that were missing from le-auto. Fix #2334. ConfigArgParse has a conditional dependency for Pythons < 2.7. On my local machine, I had a cached ConfigArgParse wheel built under 2.7, so it didn't carry those dependencies, and the pip freeze I used to determine the le-auto requirements thus missed it. From now on, we'll do those passes with --no-cache-dir. --- letsencrypt-auto-source/letsencrypt-auto | 16 ++++++++++++++-- .../pieces/letsencrypt-auto-requirements.txt | 16 ++++++++++++++-- tools/release.sh | 6 +++++- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 7800a5eb6..0755081c8 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -438,8 +438,8 @@ if [ "$NO_SELF_UPGRADE" = 1 ]; then # ------------------------------------------------------------------------- cat << "UNLIKELY_EOF" > "$TEMP_DIR/letsencrypt-auto-requirements.txt" # This is the flattened list of packages letsencrypt-auto installs. To generate -# this, do `pip install -e acme -e . -e letsencrypt-apache`, `pip freeze`, -# and then gather the hashes. +# this, do `pip install --no-cache-dir -e acme -e . -e letsencrypt-apache`, and +# then use `hashin` or a more secure method to gather the hashes. # sha256: wxZH7baf09RlqEfqMVfTe-0flfGXYLEaR6qRwEtmYxQ # sha256: YrCJpVvh2JSc0rx-DfC9254Cj678jDIDjMhIYq791uQ @@ -508,6 +508,10 @@ idna==2.0 # sha256: WjGCsyKnBlJcRigspvBk0noCz_vUSfn0dBbx3JaqcbA ipaddress==1.0.16 +# sha256: 54vpwKDfy6xxL-BPv5K5bN2ugLG4QvJCSCFMhJbwBu8 +# sha256: Syb_TnEQ23butvWntkqCYjg51ZXCA47tpmLyott46Xw +linecache2==1.0.0 + # sha256: 6MFV_evZxLywgQtO0BrhmHVUse4DTddTLXuP2uOKYnQ ndg-httpsclient==0.4.0 @@ -599,6 +603,14 @@ requests==2.9.1 # sha256: EF-NaGFvgkjiS_DpNy7wTTzBAQTxmA9U1Xss5zpa1Wo six==1.10.0 +# sha256: glPOvsSxkJTWfMXtWvmb8duhKFKSIm6Yoxkp-HpdayM +# sha256: BazGegmYDC7P7dNCP3rgEEg57MtV_GRXc-HKoJUcMDA +traceback2==1.4.0 + +# sha256: E_d9CHXbbZtDXh1PQedK1MwutuHVyCSZYJKzQw8Ii7g +# sha256: IogqDkGMKE4fcYqCKzsCKUTVPS2QjhaQsxmp0-ssBXk +unittest2==1.1.0 + # sha256: aUkbUwUVfDxuDwSnAZhNaud_1yn8HJrNJQd_HfOFMms # sha256: 619wCpv8lkILBVY1r5AC02YuQ9gMP_0x8iTCW8DV9GI Werkzeug==0.11.3 diff --git a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt index 739e19f20..c83396de2 100644 --- a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt +++ b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt @@ -1,6 +1,6 @@ # This is the flattened list of packages letsencrypt-auto installs. To generate -# this, do `pip install -e acme -e . -e letsencrypt-apache`, `pip freeze`, -# and then gather the hashes. +# this, do `pip install --no-cache-dir -e acme -e . -e letsencrypt-apache`, and +# then use `hashin` or a more secure method to gather the hashes. # sha256: wxZH7baf09RlqEfqMVfTe-0flfGXYLEaR6qRwEtmYxQ # sha256: YrCJpVvh2JSc0rx-DfC9254Cj678jDIDjMhIYq791uQ @@ -69,6 +69,10 @@ idna==2.0 # sha256: WjGCsyKnBlJcRigspvBk0noCz_vUSfn0dBbx3JaqcbA ipaddress==1.0.16 +# sha256: 54vpwKDfy6xxL-BPv5K5bN2ugLG4QvJCSCFMhJbwBu8 +# sha256: Syb_TnEQ23butvWntkqCYjg51ZXCA47tpmLyott46Xw +linecache2==1.0.0 + # sha256: 6MFV_evZxLywgQtO0BrhmHVUse4DTddTLXuP2uOKYnQ ndg-httpsclient==0.4.0 @@ -160,6 +164,14 @@ requests==2.9.1 # sha256: EF-NaGFvgkjiS_DpNy7wTTzBAQTxmA9U1Xss5zpa1Wo six==1.10.0 +# sha256: glPOvsSxkJTWfMXtWvmb8duhKFKSIm6Yoxkp-HpdayM +# sha256: BazGegmYDC7P7dNCP3rgEEg57MtV_GRXc-HKoJUcMDA +traceback2==1.4.0 + +# sha256: E_d9CHXbbZtDXh1PQedK1MwutuHVyCSZYJKzQw8Ii7g +# sha256: IogqDkGMKE4fcYqCKzsCKUTVPS2QjhaQsxmp0-ssBXk +unittest2==1.1.0 + # sha256: aUkbUwUVfDxuDwSnAZhNaud_1yn8HJrNJQd_HfOFMms # sha256: 619wCpv8lkILBVY1r5AC02YuQ9gMP_0x8iTCW8DV9GI Werkzeug==0.11.3 diff --git a/tools/release.sh b/tools/release.sh index 9d625191e..83b57657f 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -133,8 +133,12 @@ virtualenv --no-site-packages ../venv . ../venv/bin/activate pip install -U setuptools pip install -U pip -# Now, use our local PyPI +# Now, use our local PyPI. Disable cache so we get the correct KGS even if we +# (or our dependencies) have conditional dependencies implemented with if +# statements in setup.py and we have cached wheels lying around that would +# cause those ifs to not be evaluated. pip install \ + --no-cache-dir \ --extra-index-url http://localhost:$PORT \ letsencrypt $SUBPKGS # stop local PyPI