From 3a2e9ff1fa48112ead871de4f6865f47b6245455 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 12 Apr 2019 19:40:51 +0300 Subject: [PATCH] Try to restart httpd on Fedora if config check fails (#6941) This PR adds a step to Apache plugin config_test when run on Fedora. Because Fedora now creates self signed certificate and related key material upon first startup of httpd. This was causing issues for users who run certbot-auto or install certbot (and mod_ssl) and run Certbot directly after. Fixes: #6828 * Try to restart httpd on Fedora if config check fails * Update CHANGELOG.md --- CHANGELOG.md | 7 ++- .../certbot_apache/override_centos.py | 39 +++++++++++++- .../certbot_apache/tests/centos_test.py | 52 +++++++++++++++++++ 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34838bdc3..e05808edc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed -* +* Apache plugin now tries to restart httpd on Fedora using systemctl if a + configuration test error is detected. This has to be done due to the way + Fedora now generates the self signed certificate files upon first + restart. ### Fixed @@ -20,7 +23,7 @@ Despite us having broken lockstep, we are continuing to release new versions of all Certbot components during releases for the time being, however, the only package with changes other than its version number was: -* +* certbot-apache More details about these changes can be found on our GitHub repo. diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py index f0c7e7367..a42e1ac9c 100644 --- a/certbot-apache/certbot_apache/override_centos.py +++ b/certbot-apache/certbot_apache/override_centos.py @@ -4,14 +4,17 @@ import logging import pkg_resources import zope.interface +from certbot import errors +from certbot import interfaces +from certbot import util +from certbot.errors import MisconfigurationError + from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module from certbot_apache import apache_util from certbot_apache import configurator from certbot_apache import parser -from certbot import interfaces -from certbot.errors import MisconfigurationError logger = logging.getLogger(__name__) @@ -40,6 +43,38 @@ class CentOSConfigurator(configurator.ApacheConfigurator): "certbot_apache", "centos-options-ssl-apache.conf") ) + def config_test(self): + """ + Override config_test to mitigate configtest error in vanilla installation + of mod_ssl in Fedora. The error is caused by non-existent self-signed + certificates referenced by the configuration, that would be autogenerated + during the first (re)start of httpd. + """ + + os_info = util.get_os_info() + fedora = os_info[0].lower() == "fedora" + + try: + super(CentOSConfigurator, self).config_test() + except errors.MisconfigurationError: + if fedora: + self._try_restart_fedora() + else: + raise + + def _try_restart_fedora(self): + """ + Tries to restart httpd using systemctl to generate the self signed keypair. + """ + + try: + util.run_script(['systemctl', 'restart', 'httpd']) + except errors.SubprocessError as err: + raise errors.MisconfigurationError(str(err)) + + # Finish with actual config check to see if systemctl restart helped + super(CentOSConfigurator, self).config_test() + def _prepare_options(self): """ Override the options dictionary initialization in order to support diff --git a/certbot-apache/certbot_apache/tests/centos_test.py b/certbot-apache/certbot_apache/tests/centos_test.py index a27916c32..c5a51c749 100644 --- a/certbot-apache/certbot_apache/tests/centos_test.py +++ b/certbot-apache/certbot_apache/tests/centos_test.py @@ -30,6 +30,58 @@ def get_vh_truth(temp_dir, config_name): ] return vh_truth +class FedoraRestartTest(util.ApacheTest): + """Tests for Fedora specific self-signed certificate override""" + + def setUp(self): # pylint: disable=arguments-differ + test_dir = "centos7_apache/apache" + config_root = "centos7_apache/apache/httpd" + vhost_root = "centos7_apache/apache/httpd/conf.d" + super(FedoraRestartTest, self).setUp(test_dir=test_dir, + config_root=config_root, + vhost_root=vhost_root) + self.config = util.get_apache_configurator( + self.config_path, self.vhost_path, self.config_dir, self.work_dir, + os_info="fedora") + self.vh_truth = get_vh_truth( + self.temp_dir, "centos7_apache/apache") + + def _run_fedora_test(self): + with mock.patch("certbot.util.get_os_info") as mock_info: + mock_info.return_value = ["fedora"] + self.config.config_test() + + def test_non_fedora_error(self): + c_test = "certbot_apache.configurator.ApacheConfigurator.config_test" + with mock.patch(c_test) as mock_test: + mock_test.side_effect = errors.MisconfigurationError + with mock.patch("certbot.util.get_os_info") as mock_info: + mock_info.return_value = ["not_fedora"] + self.assertRaises(errors.MisconfigurationError, + self.config.config_test) + + def test_fedora_restart_error(self): + c_test = "certbot_apache.configurator.ApacheConfigurator.config_test" + with mock.patch(c_test) as mock_test: + # First call raises error, second doesn't + mock_test.side_effect = [errors.MisconfigurationError, ''] + with mock.patch("certbot.util.run_script") as mock_run: + mock_run.side_effect = errors.SubprocessError + self.assertRaises(errors.MisconfigurationError, + self._run_fedora_test) + + def test_fedora_restart(self): + c_test = "certbot_apache.configurator.ApacheConfigurator.config_test" + with mock.patch(c_test) as mock_test: + with mock.patch("certbot.util.run_script") as mock_run: + # First call raises error, second doesn't + mock_test.side_effect = [errors.MisconfigurationError, ''] + self._run_fedora_test() + self.assertEqual(mock_test.call_count, 2) + self.assertEqual(mock_run.call_args[0][0], + ['systemctl', 'restart', 'httpd']) + + class MultipleVhostsTestCentOS(util.ApacheTest): """Multiple vhost tests for CentOS / RHEL family of distros"""