Skip to content

Commit

Permalink
Apache plugin: configure all matching domain names to be able to answ…
Browse files Browse the repository at this point in the history
…er HTTP challenge. (certbot#6729)

Attempts to configure all of the following VirtualHosts for answering the HTTP challenge:

* VirtualHosts that have the requested domain name in either `ServerName` or `ServerAlias` directive.
* VirtualHosts that have a wildcard name that would match the requested domain name.

This also applies to HTTPS VirtualHosts, making Apache plugin able to handle cases where HTTP redirection takes place in reverse proxy or similar, before reaching the Apache HTTPD.

Even though also HTTPS VirtualHosts are selected, Apache plugin tries to ensure that at least one of the selected VirtualHosts listens to HTTP-01 port (configured with `--http-01-port` CLI option). So in a case where only HTTPS VirtualHosts exist, but user wants to configure those, `--http-01-port` parameter needs to be set for the port configured to the HTTPS VirtualHost(s).

Fixes: certbot#6730

* Select all matching VirtualHosts for HTTP-01 challenges instead of just one

* Finalize PR and add tests

* Changelog entry
  • Loading branch information
joohoi authored and bmw committed Feb 6, 2019
1 parent 2ddaf3d commit 7e6a1f2
Showing 11 changed files with 112 additions and 38 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -18,6 +18,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).

* Lexicon-based DNS plugins are now fully compatible with Lexicon 3.x (support
on 2.x branch is maintained).
* Apache plugin now attempts to configure all VirtualHosts matching requested
domain name instead of only a single one when answering the HTTP-01 challenge.

### Fixed

9 changes: 5 additions & 4 deletions certbot-apache/certbot_apache/configurator.py
Original file line number Diff line number Diff line change
@@ -577,8 +577,9 @@ def _choose_vhost_from_list(self, target_name, temp=False):
self.assoc[target_name] = vhost
return vhost

def included_in_wildcard(self, names, target_name):
"""Is target_name covered by a wildcard?
def domain_in_names(self, names, target_name):
"""Checks if target domain is covered by one or more of the provided
names. The target name is matched by wildcard as well as exact match.
:param names: server aliases
:type names: `collections.Iterable` of `str`
@@ -649,7 +650,7 @@ def _find_best_vhost(self, target_name, vhosts=None, filter_defaults=True):
names = vhost.get_names()
if target_name in names:
points = 3
elif self.included_in_wildcard(names, target_name):
elif self.domain_in_names(names, target_name):
points = 2
elif any(addr.get_addr() == target_name for addr in vhost.addrs):
points = 1
@@ -1463,7 +1464,7 @@ def _has_matching_wildcard(self, vh_path, target_name):
matches = self.parser.find_dir(
"ServerAlias", start=vh_path, exclude=False)
aliases = (self.aug.get(match) for match in matches)
return self.included_in_wildcard(aliases, target_name)
return self.domain_in_names(aliases, target_name)

def _add_name_vhost_if_necessary(self, vhost):
"""Add NameVirtualHost Directives if necessary for new vhost.
44 changes: 35 additions & 9 deletions certbot-apache/certbot_apache/http_01.py
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@
import logging
import os

from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module
from acme.magic_typing import List, Set # pylint: disable=unused-import, no-name-in-module
from certbot import errors
from certbot.plugins import common
from certbot_apache.obj import VirtualHost # pylint: disable=unused-import
@@ -89,15 +89,27 @@ def prepare_http01_modules(self):
self.configurator.enable_mod(mod, temp=True)

def _mod_config(self):
selected_vhosts = [] # type: List[VirtualHost]
http_port = str(self.configurator.config.http01_port)
for chall in self.achalls:
vh = self.configurator.find_best_http_vhost(
chall.domain, filter_defaults=False,
port=str(self.configurator.config.http01_port))
if vh:
self._set_up_include_directives(vh)
else:
for vh in self._relevant_vhosts():
self._set_up_include_directives(vh)
# Search for matching VirtualHosts
for vh in self._matching_vhosts(chall.domain):
selected_vhosts.append(vh)

# Ensure that we have one or more VirtualHosts that we can continue
# with. (one that listens to port configured with --http-01-port)
found = False
for vhost in selected_vhosts:
if any(a.is_wildcard() or a.get_port() == http_port for a in vhost.addrs):
found = True

if not found:
for vh in self._relevant_vhosts():
selected_vhosts.append(vh)

# Add the challenge configuration
for vh in selected_vhosts:
self._set_up_include_directives(vh)

self.configurator.reverter.register_file_creation(
True, self.challenge_conf_pre)
@@ -121,6 +133,20 @@ def _mod_config(self):
with open(self.challenge_conf_post, "w") as new_conf:
new_conf.write(config_text_post)

def _matching_vhosts(self, domain):
"""Return all VirtualHost objects that have the requested domain name or
a wildcard name that would match the domain in ServerName or ServerAlias
directive.
"""
matching_vhosts = []
for vhost in self.configurator.vhosts:
if self.configurator.domain_in_names(vhost.get_names(), domain):
# domain_in_names also matches the exact names, so no need
# to check "domain in vhost.get_names()" explicitly here
matching_vhosts.append(vhost)

return matching_vhosts

def _relevant_vhosts(self):
http01_port = str(self.configurator.config.http01_port)
relevant_vhosts = []
22 changes: 11 additions & 11 deletions certbot-apache/certbot_apache/tests/configurator_test.py
Original file line number Diff line number Diff line change
@@ -139,7 +139,8 @@ def test_get_all_names(self, mock_getutility):
names = self.config.get_all_names()
self.assertEqual(names, set(
["certbot.demo", "ocspvhost.com", "encryption-example.demo",
"nonsym.link", "vhost.in.rootconf", "www.certbot.demo"]
"nonsym.link", "vhost.in.rootconf", "www.certbot.demo",
"duplicate.example.com"]
))

@certbot_util.patch_get_utility()
@@ -158,8 +159,7 @@ def test_get_all_names_addrs(self, mock_gethost, mock_getutility):
self.config.vhosts.append(vhost)

names = self.config.get_all_names()
# Names get filtered, only 5 are returned
self.assertEqual(len(names), 8)
self.assertEqual(len(names), 9)
self.assertTrue("zombo.com" in names)
self.assertTrue("google.com" in names)
self.assertTrue("certbot.demo" in names)
@@ -200,7 +200,7 @@ def test_add_servernames_alias(self):
def test_get_virtual_hosts(self):
"""Make sure all vhosts are being properly found."""
vhs = self.config.get_virtual_hosts()
self.assertEqual(len(vhs), 10)
self.assertEqual(len(vhs), 12)
found = 0

for vhost in vhs:
@@ -211,15 +211,15 @@ def test_get_virtual_hosts(self):
else:
raise Exception("Missed: %s" % vhost) # pragma: no cover

self.assertEqual(found, 10)
self.assertEqual(found, 12)

# Handle case of non-debian layout get_virtual_hosts
with mock.patch(
"certbot_apache.configurator.ApacheConfigurator.conf"
) as mock_conf:
mock_conf.return_value = False
vhs = self.config.get_virtual_hosts()
self.assertEqual(len(vhs), 10)
self.assertEqual(len(vhs), 12)

@mock.patch("certbot_apache.display_ops.select_vhost")
def test_choose_vhost_none_avail(self, mock_select):
@@ -322,7 +322,7 @@ def test_find_best_vhost_default(self):
self.config.vhosts = [
vh for vh in self.config.vhosts
if vh.name not in ["certbot.demo", "nonsym.link",
"encryption-example.demo",
"encryption-example.demo", "duplicate.example.com",
"ocspvhost.com", "vhost.in.rootconf"]
and "*.blue.purple.com" not in vh.aliases
]
@@ -333,7 +333,7 @@ def test_find_best_vhost_default(self):
def test_non_default_vhosts(self):
# pylint: disable=protected-access
vhosts = self.config._non_default_vhosts(self.config.vhosts)
self.assertEqual(len(vhosts), 8)
self.assertEqual(len(vhosts), 10)

def test_deploy_cert_enable_new_vhost(self):
# Create
@@ -688,7 +688,7 @@ def test_make_vhost_ssl(self):
self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]),
self.config.is_name_vhost(ssl_vhost))

self.assertEqual(len(self.config.vhosts), 11)
self.assertEqual(len(self.config.vhosts), 13)

def test_clean_vhost_ssl(self):
# pylint: disable=protected-access
@@ -1269,7 +1269,7 @@ def test_create_own_redirect(self):

# pylint: disable=protected-access
self.config._enable_redirect(self.vh_truth[1], "")
self.assertEqual(len(self.config.vhosts), 11)
self.assertEqual(len(self.config.vhosts), 13)

def test_create_own_redirect_for_old_apache_version(self):
self.config.parser.modules.add("rewrite_module")
@@ -1280,7 +1280,7 @@ def test_create_own_redirect_for_old_apache_version(self):

# pylint: disable=protected-access
self.config._enable_redirect(self.vh_truth[1], "")
self.assertEqual(len(self.config.vhosts), 11)
self.assertEqual(len(self.config.vhosts), 13)

def test_sift_rewrite_rule(self):
# pylint: disable=protected-access
34 changes: 22 additions & 12 deletions certbot-apache/certbot_apache/tests/http_01_test.py
Original file line number Diff line number Diff line change
@@ -27,8 +27,8 @@ def setUp(self, *args, **kwargs):
self.achalls = [] # type: List[achallenges.KeyAuthorizationAnnotatedChallenge]
vh_truth = util.get_vh_truth(
self.temp_dir, "debian_apache_2_4/multiple_vhosts")
# Takes the vhosts for encryption-example.demo, certbot.demo, and
# vhost.in.rootconf
# Takes the vhosts for encryption-example.demo, certbot.demo
# and vhost.in.rootconf
self.vhosts = [vh_truth[0], vh_truth[3], vh_truth[10]]

for i in range(NUM_ACHALLS):
@@ -39,7 +39,7 @@ def setUp(self, *args, **kwargs):
"pending"),
domain=self.vhosts[i].name, account_key=self.account_key))

modules = ["rewrite", "authz_core", "authz_host"]
modules = ["ssl", "rewrite", "authz_core", "authz_host"]
for mod in modules:
self.config.parser.modules.add("mod_{0}.c".format(mod))
self.config.parser.modules.add(mod + "_module")
@@ -111,6 +111,17 @@ def test_anonymous_vhost(self):
domain="something.nonexistent", account_key=self.account_key)]
self.common_perform_test(achalls, vhosts)

def test_configure_multiple_vhosts(self):
vhosts = [v for v in self.config.vhosts if "duplicate.example.com" in v.get_names()]
self.assertEqual(len(vhosts), 2)
achalls = [
achallenges.KeyAuthorizationAnnotatedChallenge(
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'a' * 16))),
"pending"),
domain="duplicate.example.com", account_key=self.account_key)]
self.common_perform_test(achalls, vhosts)

def test_no_vhost(self):
for achall in self.achalls:
self.http.add_chall(achall)
@@ -176,15 +187,14 @@ def common_perform_test(self, achalls, vhosts):
self._test_challenge_file(achall)

for vhost in vhosts:
if not vhost.ssl:
matches = self.config.parser.find_dir("Include",
self.http.challenge_conf_pre,
vhost.path)
self.assertEqual(len(matches), 1)
matches = self.config.parser.find_dir("Include",
self.http.challenge_conf_post,
vhost.path)
self.assertEqual(len(matches), 1)
matches = self.config.parser.find_dir("Include",
self.http.challenge_conf_pre,
vhost.path)
self.assertEqual(len(matches), 1)
matches = self.config.parser.find_dir("Include",
self.http.challenge_conf_post,
vhost.path)
self.assertEqual(len(matches), 1)

self.assertTrue(os.path.exists(challenge_dir))

2 changes: 1 addition & 1 deletion certbot-apache/certbot_apache/tests/parser_test.py
Original file line number Diff line number Diff line change
@@ -52,7 +52,7 @@ def test_find_dir(self):
test2 = self.parser.find_dir("documentroot")

self.assertEqual(len(test), 1)
self.assertEqual(len(test2), 7)
self.assertEqual(len(test2), 8)

def test_add_dir(self):
aug_default = "/files" + self.parser.loc["default"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<VirtualHost 10.2.3.4:80>
ServerName duplicate.example.com

ServerAdmin [email protected]
DocumentRoot /var/www/html

ErrorLog ${APACHE_LOG_DIR}/error.log
CustomLog ${APACHE_LOG_DIR}/access.log combined
</VirtualHost>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<IfModule mod_ssl.c>
<VirtualHost 10.2.3.4:443>
ServerName duplicate.example.com

ServerAdmin [email protected]
DocumentRoot /var/www/html

ErrorLog ${APACHE_LOG_DIR}/error.log
CustomLog ${APACHE_LOG_DIR}/access.log combined

SSLCertificateFile /etc/apache2/certs/certbot-cert_5.pem
SSLCertificateKeyFile /etc/apache2/ssl/key-certbot_15.pem
</VirtualHost>
</IfModule>
12 changes: 11 additions & 1 deletion certbot-apache/certbot_apache/tests/util.py
Original file line number Diff line number Diff line change
@@ -196,7 +196,17 @@ def get_vh_truth(temp_dir, config_name):
"/files" + os.path.join(temp_dir, config_name,
"apache2/apache2.conf/VirtualHost"),
set([obj.Addr.fromstring("*:80")]), False, True,
"vhost.in.rootconf")]
"vhost.in.rootconf"),
obj.VirtualHost(
os.path.join(prefix, "duplicatehttp.conf"),
os.path.join(aug_pre, "duplicatehttp.conf/VirtualHost"),
set([obj.Addr.fromstring("10.2.3.4:80")]), False, True,
"duplicate.example.com"),
obj.VirtualHost(
os.path.join(prefix, "duplicatehttps.conf"),
os.path.join(aug_pre, "duplicatehttps.conf/IfModule/VirtualHost"),
set([obj.Addr.fromstring("10.2.3.4:443")]), True, True,
"duplicate.example.com")]
return vh_truth
if config_name == "debian_apache_2_4/multi_vhosts":
prefix = os.path.join(

0 comments on commit 7e6a1f2

Please sign in to comment.