From e034b50363a75d61d1a7e7add2b8aca52684acd1 Mon Sep 17 00:00:00 2001 From: Daniel Huang Date: Sat, 18 Mar 2017 13:42:54 -0700 Subject: [PATCH] Don't save keys/csr on dry run (#4380) * Don't save keys/csr on dry run (#2495) * Replace assertIsNone for py26 * Fix config defaults for compat tests --- certbot-nginx/certbot_nginx/tests/util.py | 1 + certbot/constants.py | 1 + certbot/crypto_util.py | 30 ++++++++++-------- certbot/tests/crypto_util_test.py | 37 +++++++++++++++++++++-- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py index 259dc1f10..4ab95374e 100644 --- a/certbot-nginx/certbot_nginx/tests/util.py +++ b/certbot-nginx/certbot_nginx/tests/util.py @@ -70,6 +70,7 @@ def get_nginx_configurator( in_progress_dir=os.path.join(backups, "IN_PROGRESS"), server="https://acme-server.org:443/new", tls_sni_01_port=5001, + dry_run=False, ), name="nginx", version=version) diff --git a/certbot/constants.py b/certbot/constants.py index b286ca26a..c85992fc1 100644 --- a/certbot/constants.py +++ b/certbot/constants.py @@ -18,6 +18,7 @@ CLI_DEFAULTS = dict( os.path.join(os.environ.get("XDG_CONFIG_HOME", "~/.config"), "letsencrypt", "cli.ini"), ], + dry_run=False, verbose_count=-int(logging.INFO / 10), server="https://acme-v01.api.letsencrypt.org/directory", rsa_key_size=2048, diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 65e3de345..1ad76d503 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -53,12 +53,15 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): # Save file util.make_or_verify_dir(key_dir, 0o700, os.geteuid(), config.strict_permissions) - key_f, key_path = util.unique_file( - os.path.join(key_dir, keyname), 0o600, "wb") - with key_f: - key_f.write(key_pem) - - logger.info("Generating key (%d bits): %s", key_size, key_path) + if config.dry_run: + key_path = None + logger.info("Generating key (%d bits), not saving to file", key_size) + else: + key_f, key_path = util.unique_file( + os.path.join(key_dir, keyname), 0o600, "wb") + with key_f: + key_f.write(key_pem) + logger.info("Generating key (%d bits): %s", key_size, key_path) return util.Key(key_path, key_pem) @@ -85,12 +88,15 @@ def init_save_csr(privkey, names, path, csrname="csr-certbot.pem"): # Save CSR util.make_or_verify_dir(path, 0o755, os.geteuid(), config.strict_permissions) - csr_f, csr_filename = util.unique_file( - os.path.join(path, csrname), 0o644, "wb") - csr_f.write(csr_pem) - csr_f.close() - - logger.info("Creating CSR: %s", csr_filename) + if config.dry_run: + csr_filename = None + logger.info("Creating CSR: not saving to file") + else: + csr_f, csr_filename = util.unique_file( + os.path.join(path, csrname), 0o644, "wb") + with csr_f: + csr_f.write(csr_pem) + logger.info("Creating CSR: %s", csr_filename) return util.CSR(csr_filename, csr_der, "der") diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index 946e772c1..a832d0494 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -1,5 +1,6 @@ """Tests for certbot.crypto_util.""" import logging +import os import shutil import tempfile import unittest @@ -26,7 +27,8 @@ class InitSaveKeyTest(unittest.TestCase): def setUp(self): logging.disable(logging.CRITICAL) zope.component.provideUtility( - mock.Mock(strict_permissions=True), interfaces.IConfig) + mock.Mock(strict_permissions=True, dry_run=False), + interfaces.IConfig) self.key_dir = tempfile.mkdtemp('key_dir') def tearDown(self): @@ -44,6 +46,17 @@ class InitSaveKeyTest(unittest.TestCase): key = self._call(1024, self.key_dir) self.assertEqual(key.pem, b'key_pem') self.assertTrue('key-certbot.pem' in key.file) + self.assertTrue(os.path.exists(os.path.join(self.key_dir, key.file))) + + @mock.patch('certbot.crypto_util.make_key') + def test_success_dry_run(self, mock_make): + zope.component.provideUtility( + mock.Mock(strict_permissions=True, dry_run=True), + interfaces.IConfig) + mock_make.return_value = b'key_pem' + key = self._call(1024, self.key_dir) + self.assertEqual(key.pem, b'key_pem') + self.assertTrue(key.file is None) @mock.patch('certbot.crypto_util.make_key') def test_key_failure(self, mock_make): @@ -56,7 +69,8 @@ class InitSaveCSRTest(unittest.TestCase): def setUp(self): zope.component.provideUtility( - mock.Mock(strict_permissions=True), interfaces.IConfig) + mock.Mock(strict_permissions=True, dry_run=False), + interfaces.IConfig) self.csr_dir = tempfile.mkdtemp('csr_dir') def tearDown(self): @@ -64,7 +78,7 @@ class InitSaveCSRTest(unittest.TestCase): @mock.patch('certbot.crypto_util.make_csr') @mock.patch('certbot.crypto_util.util.make_or_verify_dir') - def test_it(self, unused_mock_verify, mock_csr): + def test_success(self, unused_mock_verify, mock_csr): from certbot.crypto_util import init_save_csr mock_csr.return_value = (b'csr_pem', b'csr_der') @@ -76,6 +90,23 @@ class InitSaveCSRTest(unittest.TestCase): self.assertEqual(csr.data, b'csr_der') self.assertTrue('csr-certbot.pem' in csr.file) + @mock.patch('certbot.crypto_util.make_csr') + @mock.patch('certbot.crypto_util.util.make_or_verify_dir') + def test_success_dry_run(self, unused_mock_verify, mock_csr): + from certbot.crypto_util import init_save_csr + + zope.component.provideUtility( + mock.Mock(strict_permissions=True, dry_run=True), + interfaces.IConfig) + mock_csr.return_value = (b'csr_pem', b'csr_der') + + csr = init_save_csr( + mock.Mock(pem='dummy_key'), 'example.com', self.csr_dir, + 'csr-certbot.pem') + + self.assertEqual(csr.data, b'csr_der') + self.assertTrue(csr.file is None) + class MakeCSRTest(unittest.TestCase): """Tests for certbot.crypto_util.make_csr."""