diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 6377bb114..0ecbf88b3 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -323,7 +323,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Returned objects are guaranteed to be ssl vhosts return self._choose_vhosts_wildcard(domain, create_if_no_ssl) else: - return [self.choose_vhost(domain)] + return [self.choose_vhost(domain, create_if_no_ssl)] def _vhosts_for_wildcard(self, domain): """ @@ -469,20 +469,21 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if chain_path is not None: self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path - def choose_vhost(self, target_name, temp=False): + def choose_vhost(self, target_name, create_if_no_ssl=True): """Chooses a virtual host based on the given domain name. If there is no clear virtual host to be selected, the user is prompted with all available choices. - The returned vhost is guaranteed to have TLS enabled unless temp is - True. If temp is True, there is no such guarantee and the result is - not cached. + The returned vhost is guaranteed to have TLS enabled unless + create_if_no_ssl is set to False, in which case there is no such guarantee + and the result is not cached. :param str target_name: domain name - :param bool temp: whether the vhost is only used temporarily + :param bool create_if_no_ssl: If found VirtualHost doesn't have a HTTPS + counterpart, should one get created - :returns: ssl vhost associated with name + :returns: vhost associated with name :rtype: :class:`~certbot_apache.obj.VirtualHost` :raises .errors.PluginError: If no vhost is available or chosen @@ -495,7 +496,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Try to find a reasonable vhost vhost = self._find_best_vhost(target_name) if vhost is not None: - if temp: + if not create_if_no_ssl: return vhost if not vhost.ssl: vhost = self.make_vhost_ssl(vhost) @@ -504,7 +505,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.assoc[target_name] = vhost return vhost - return self._choose_vhost_from_list(target_name, temp) + # Negate create_if_no_ssl value to indicate if we want a SSL vhost + # to get created if a non-ssl vhost is selected. + return self._choose_vhost_from_list(target_name, temp=not create_if_no_ssl) def _choose_vhost_from_list(self, target_name, temp=False): # Select a vhost from a list @@ -1499,7 +1502,20 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise errors.PluginError( "Unsupported enhancement: {0}".format(enhancement)) - vhosts = self.choose_vhosts(domain, create_if_no_ssl=False) + matched_vhosts = self.choose_vhosts(domain, create_if_no_ssl=False) + # We should be handling only SSL vhosts for enhancements + vhosts = [vhost for vhost in matched_vhosts if vhost.ssl] + + if not vhosts: + msg_tmpl = ("Certbot was not able to find SSL VirtualHost for a " + "domain {0} for enabling enhancement \"{1}\". The requested " + "enhancement was not configured.") + msg_enhancement = enhancement + if options: + msg_enhancement += ": " + options + msg = msg_tmpl.format(domain, msg_enhancement) + logger.warning(msg) + raise errors.PluginError(msg) try: for vhost in vhosts: func(vhost, options) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index d002a2c10..2d683d6ef 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -246,7 +246,7 @@ class MultipleVhostsTest(util.ApacheTest): @mock.patch("certbot_apache.display_ops.select_vhost") def test_choose_vhost_select_vhost_with_temp(self, mock_select): mock_select.return_value = self.vh_truth[0] - chosen_vhost = self.config.choose_vhost("none.com", temp=True) + chosen_vhost = self.config.choose_vhost("none.com", create_if_no_ssl=False) self.assertEqual(self.vh_truth[0], chosen_vhost) @mock.patch("certbot_apache.display_ops.select_vhost") @@ -914,14 +914,16 @@ class MultipleVhostsTest(util.ApacheTest): def test_enhance_no_ssl_vhost(self): with mock.patch("certbot_apache.configurator.logger.warning") as mock_log: - self.config.enhance("certbot.demo", "redirect") + self.assertRaises(errors.PluginError, self.config.enhance, + "certbot.demo", "redirect") # Check that correct logger.warning was printed self.assertTrue("not able to find" in mock_log.call_args[0][0]) self.assertTrue("\"redirect\"" in mock_log.call_args[0][0]) mock_log.reset_mock() - self.config.enhance("certbot.demo", "ensure-http-header", "Test") + self.assertRaises(errors.PluginError, self.config.enhance, + "certbot.demo", "ensure-http-header", "Test") # Check that correct logger.warning was printed self.assertTrue("not able to find" in mock_log.call_args[0][0]) self.assertTrue("Test" in mock_log.call_args[0][0]) @@ -1448,6 +1450,7 @@ class MultipleVhostsTest(util.ApacheTest): # pylint: disable=protected-access self.config.parser.modules.add("mod_ssl.c") self.config.parser.modules.add("headers_module") + self.vh_truth[3].ssl = True self.config._wildcard_vhosts["*.certbot.demo"] = [self.vh_truth[3]] self.config.enhance("*.certbot.demo", "ensure-http-header", "Upgrade-Insecure-Requests") @@ -1455,6 +1458,7 @@ class MultipleVhostsTest(util.ApacheTest): @mock.patch("certbot_apache.configurator.ApacheConfigurator._choose_vhosts_wildcard") def test_enhance_wildcard_no_install(self, mock_choose): + self.vh_truth[3].ssl = True mock_choose.return_value = [self.vh_truth[3]] self.config.parser.modules.add("mod_ssl.c") self.config.parser.modules.add("headers_module") diff --git a/certbot-apache/certbot_apache/tls_sni_01.py b/certbot-apache/certbot_apache/tls_sni_01.py index 5ce96ac5f..549feb17d 100644 --- a/certbot-apache/certbot_apache/tls_sni_01.py +++ b/certbot-apache/certbot_apache/tls_sni_01.py @@ -123,7 +123,8 @@ class ApacheTlsSni01(common.TLSSNI01): self.configurator.config.tls_sni_01_port))) try: - vhost = self.configurator.choose_vhost(achall.domain, temp=True) + vhost = self.configurator.choose_vhost(achall.domain, + create_if_no_ssl=False) except (PluginError, MissingCommandlineFlag): # We couldn't find the virtualhost for this domain, possibly # because it's a new vhost that's not configured yet