From 60dd67a60e4f17b5d46718d3ff941a2e292398c7 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 15 Jan 2018 01:22:22 +0200 Subject: [PATCH] Use static directory under workdir for HTTP challenges (#5428) * Use static directory under workdir for HTTP challenges * Handle the reverter file registration before opening file handle --- certbot-apache/certbot_apache/configurator.py | 1 - certbot-apache/certbot_apache/http_01.py | 19 ++++++++----------- .../certbot_apache/tests/configurator_test.py | 5 ----- .../certbot_apache/tests/http_01_test.py | 6 +++--- 4 files changed, 11 insertions(+), 20 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 8d6995211..b87189dc0 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1956,7 +1956,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.revert_challenge_config() self.restart() self.parser.reset_modules() - self.http_doer.cleanup() def install_ssl_options_conf(self, options_ssl, options_ssl_digest): """Copy Certbot's SSL options file into the system's config dir if required.""" diff --git a/certbot-apache/certbot_apache/http_01.py b/certbot-apache/certbot_apache/http_01.py index 87721d290..3b942823d 100644 --- a/certbot-apache/certbot_apache/http_01.py +++ b/certbot-apache/certbot_apache/http_01.py @@ -1,8 +1,6 @@ """A class that performs HTTP-01 challenges for Apache""" import logging import os -import shutil -import tempfile from certbot.plugins import common @@ -35,7 +33,9 @@ Alias /.well-known/acme-challenge {0} self.challenge_conf = os.path.join( self.configurator.conf("challenge-location"), "le_http_01_challenge.conf") - self.challenge_dir = None + self.challenge_dir = os.path.join( + self.configurator.config.work_dir, + "http_challenges") def perform(self): """Perform all HTTP-01 challenges.""" @@ -56,12 +56,6 @@ Alias /.well-known/acme-challenge {0} return responses - def cleanup(self): - """Cleanup the challenge directory.""" - if self.challenge_dir: - shutil.rmtree(self.challenge_dir, ignore_errors=True) - self.challenge_dir = None - def prepare_http01_modules(self): """Make sure that we have the needed modules available for http01""" @@ -92,8 +86,9 @@ Alias /.well-known/acme-challenge {0} new_conf.write(config_text) def _set_up_challenges(self): - self.challenge_dir = tempfile.mkdtemp() - os.chmod(self.challenge_dir, 0o755) + if not os.path.isdir(self.challenge_dir): + os.makedirs(self.challenge_dir) + os.chmod(self.challenge_dir, 0o755) responses = [] for achall in self.achalls: @@ -105,6 +100,8 @@ Alias /.well-known/acme-challenge {0} response, validation = achall.response_and_validation() name = os.path.join(self.challenge_dir, achall.chall.encode("token")) + + self.configurator.reverter.register_file_creation(True, name) with open(name, 'wb') as f: f.write(validation.encode()) os.chmod(name, 0o644) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index ea3867061..df67104da 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -747,7 +747,6 @@ class MultipleVhostsTest(util.ApacheTest): def test_cleanup(self, mock_cfg, mock_restart): mock_cfg.return_value = "" _, achalls = self.get_key_and_achalls() - self.config.http_doer = mock.MagicMock() for achall in achalls: self.config._chall_out.add(achall) # pylint: disable=protected-access @@ -756,10 +755,8 @@ class MultipleVhostsTest(util.ApacheTest): self.config.cleanup([achall]) if i == len(achalls) - 1: self.assertTrue(mock_restart.called) - self.assertTrue(self.config.http_doer.cleanup.called) else: self.assertFalse(mock_restart.called) - self.assertFalse(self.config.http_doer.cleanup.called) @mock.patch("certbot_apache.configurator.ApacheConfigurator.restart") @mock.patch("certbot_apache.parser.ApacheParser._get_runtime_cfg") @@ -773,11 +770,9 @@ class MultipleVhostsTest(util.ApacheTest): self.config.cleanup([achalls[-1]]) self.assertFalse(mock_restart.called) - self.assertFalse(self.config.http_doer.cleanup.called) self.config.cleanup(achalls) self.assertTrue(mock_restart.called) - self.assertTrue(self.config.http_doer.cleanup.called) @mock.patch("certbot.util.run_script") def test_get_version(self, mock_script): diff --git a/certbot-apache/certbot_apache/tests/http_01_test.py b/certbot-apache/certbot_apache/tests/http_01_test.py index 4e2a5faff..204d9a76c 100644 --- a/certbot-apache/certbot_apache/tests/http_01_test.py +++ b/certbot-apache/certbot_apache/tests/http_01_test.py @@ -100,6 +100,8 @@ class ApacheHttp01Test(util.ApacheTest): def common_perform_test(self, achalls): """Tests perform with the given achalls.""" + challenge_dir = self.http.challenge_dir + self.assertFalse(os.path.exists(challenge_dir)) for achall in achalls: self.http.add_chall(achall) @@ -114,9 +116,7 @@ class ApacheHttp01Test(util.ApacheTest): for achall in achalls: self._test_challenge_file(achall) - challenge_dir = self.http.challenge_dir - self.http.cleanup() - self.assertFalse(os.path.exists(challenge_dir)) + self.assertTrue(os.path.exists(challenge_dir)) def _test_challenge_conf(self): self.assertEqual(