diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 60441f30c..3b91617e5 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -740,28 +740,40 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ - # If nonstandard port, add service definition for matching - if port != "443": + self.prepare_https_modules(temp) + self.ensure_listen(port, https=True) + + def ensure_listen(self, port, https=False): + """Make sure that Apache is listening on the port. Checks if the + Listen statement for the port already exists, and adds it to the + configuration if necessary. + + :param str port: Port number to check and add Listen for if not in + place already + :param bool https: If the port will be used for HTTPS + + """ + + # If HTTPS requested for nonstandard port, add service definition + if https and port != "443": port_service = "%s %s" % (port, "https") else: port_service = port - self.prepare_https_modules(temp) # Check for Listen # Note: This could be made to also look for ip:443 combo listens = [self.parser.get_arg(x).split()[0] for x in self.parser.find_dir("Listen")] - # In case no Listens are set (which really is a broken apache config) - if not listens: - listens = ["80"] - # Listen already in place if self._has_port_already(listens, port): return listen_dirs = set(listens) + if not listens: + listen_dirs.add(port_service) + for listen in listens: # For any listen statement, check if the machine also listens on # Port 443. If not, add such a listen statement. @@ -776,11 +788,39 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if "%s:%s" % (ip, port_service) not in listen_dirs and ( "%s:%s" % (ip, port_service) not in listen_dirs): listen_dirs.add("%s:%s" % (ip, port_service)) - self._add_listens(listen_dirs, listens, port) + if https: + self._add_listens_https(listen_dirs, listens, port) + else: + self._add_listens_http(listen_dirs, listens, port) - def _add_listens(self, listens, listens_orig, port): - """Helper method for prepare_server_https to figure out which new - listen statements need adding + def _add_listens_http(self, listens, listens_orig, port): + """Helper method for ensure_listen to figure out which new + listen statements need adding for listening HTTP on port + + :param set listens: Set of all needed Listen statements + :param list listens_orig: List of existing listen statements + :param string port: Port number we're adding + """ + + new_listens = listens.difference(listens_orig) + + if port in new_listens: + # We have wildcard, skip the rest + self.parser.add_dir(parser.get_aug_path(self.parser.loc["listen"]), + "Listen", port) + self.save_notes += "Added Listen %s directive to %s\n" % ( + port, self.parser.loc["listen"]) + else: + for listen in new_listens: + self.parser.add_dir(parser.get_aug_path( + self.parser.loc["listen"]), "Listen", listen.split(" ")) + self.save_notes += ("Added Listen %s directive to " + "%s\n") % (listen, + self.parser.loc["listen"]) + + def _add_listens_https(self, listens, listens_orig, port): + """Helper method for ensure_listen to figure out which new + listen statements need adding for listening HTTPS on port :param set listens: Set of all needed Listen statements :param list listens_orig: List of existing listen statements diff --git a/certbot-apache/certbot_apache/http_01.py b/certbot-apache/certbot_apache/http_01.py index 410e8e9a6..9c25b1f17 100644 --- a/certbot-apache/certbot_apache/http_01.py +++ b/certbot-apache/certbot_apache/http_01.py @@ -45,6 +45,10 @@ Alias /.well-known/acme-challenge {0} # About to make temporary changes to the config self.configurator.save("Changes before challenge setup", True) + self.configurator.ensure_listen(str( + self.configurator.config.http01_port)) + self.prepare_http01_modules() + responses = self._set_up_challenges() self._mod_config() # Save reversible changes @@ -57,6 +61,13 @@ Alias /.well-known/acme-challenge {0} 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""" + + if self.configurator.conf("handle-modules"): + if "alias_module" not in self.configurator.parser.modules: + self.configurator.enable_mod("alias", temp=True) + def _mod_config(self): self.configurator.parser.add_include( self.configurator.parser.loc["default"], self.challenge_conf) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 1620013c8..ea3867061 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -424,6 +424,43 @@ class MultipleVhostsTest(util.ApacheTest): self.assertTrue(self.config.parser.find_dir( "NameVirtualHost", "*:80")) + def test_add_listen_80(self): + mock_find = mock.Mock() + mock_add_dir = mock.Mock() + mock_find.return_value = [] + self.config.parser.find_dir = mock_find + self.config.parser.add_dir = mock_add_dir + self.config.ensure_listen("80") + self.assertTrue(mock_add_dir.called) + self.assertTrue(mock_find.called) + self.assertEqual(mock_add_dir.call_args[0][1], "Listen") + self.assertEqual(mock_add_dir.call_args[0][2], "80") + + def test_add_listen_80_named(self): + mock_find = mock.Mock() + mock_find.return_value = ["test1", "test2", "test3"] + mock_get = mock.Mock() + mock_get.side_effect = ["1.2.3.4:80", "[::1]:80", "1.1.1.1:443"] + mock_add_dir = mock.Mock() + + self.config.parser.find_dir = mock_find + self.config.parser.get_arg = mock_get + self.config.parser.add_dir = mock_add_dir + + self.config.ensure_listen("80") + self.assertEqual(mock_add_dir.call_count, 0) + + # Reset return lists and inputs + mock_add_dir.reset_mock() + mock_get.side_effect = ["1.2.3.4:80", "[::1]:80", "1.1.1.1:443"] + + # Test + self.config.ensure_listen("8080") + self.assertEqual(mock_add_dir.call_count, 3) + self.assertTrue(mock_add_dir.called) + self.assertEqual(mock_add_dir.call_args[0][1], "Listen") + self.assertEqual(mock_add_dir.call_args[0][2], ['1.2.3.4:8080']) + def test_prepare_server_https(self): mock_enable = mock.Mock() self.config.enable_mod = mock_enable @@ -435,7 +472,6 @@ class MultipleVhostsTest(util.ApacheTest): # This will test the Add listen self.config.parser.find_dir = mock_find 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") diff --git a/certbot-apache/certbot_apache/tests/http_01_test.py b/certbot-apache/certbot_apache/tests/http_01_test.py index 11121fae2..f52b9d0cc 100644 --- a/certbot-apache/certbot_apache/tests/http_01_test.py +++ b/certbot-apache/certbot_apache/tests/http_01_test.py @@ -1,4 +1,5 @@ """Test for certbot_apache.http_01.""" +import mock import os import unittest @@ -53,11 +54,21 @@ class ApacheHttp01Test(util.ApacheTest): account_key=self.account_key)) from certbot_apache.http_01 import ApacheHttp01 + self.config.parser.modules.add("mod_alias.c") + self.config.parser.modules.add("alias_module") self.http = ApacheHttp01(self.config) def test_empty_perform(self): self.assertFalse(self.http.perform()) + @mock.patch("certbot_apache.configurator.ApacheConfigurator.enable_mod") + def test_add_alias_module(self, mock_enmod): + self.config.parser.modules.remove("alias_module") + self.config.parser.modules.remove("mod_alias.c") + self.http.prepare_http01_modules() + self.assertTrue(mock_enmod.called) + self.assertEqual(mock_enmod.call_args[0][0], "alias") + def common_perform_test(self, achalls): """Tests perform with the given achalls.""" for achall in achalls: diff --git a/certbot-apache/certbot_apache/tests/util.py b/certbot-apache/certbot_apache/tests/util.py index ca667465c..8f5162629 100644 --- a/certbot-apache/certbot_apache/tests/util.py +++ b/certbot-apache/certbot_apache/tests/util.py @@ -103,6 +103,7 @@ def get_apache_configurator( # pylint: disable=too-many-arguments, too-many-loc apache_challenge_location=config_path, backup_dir=backups, config_dir=config_dir, + http01_port="80", temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), in_progress_dir=os.path.join(backups, "IN_PROGRESS"), work_dir=work_dir)