diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index b87189dc0..b32eda921 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -164,9 +164,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "ensure-http-header": self._set_http_header, "staple-ocsp": self._enable_ocsp_stapling} - # This will be set during the perform function - self.http_doer = None - @property def mod_ssl_conf(self): """Full absolute path to SSL configuration file.""" @@ -439,12 +436,35 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return True return False - def _find_best_vhost(self, target_name): + def find_best_http_vhost(self, target, filter_defaults, port="80"): + """Returns non-HTTPS vhost objects found from the Apache config + + :param str target: Domain name of the desired VirtualHost + :param bool filter_defaults: whether _default_ vhosts should be + included if it is the best match + :param str port: port number the vhost should be listening on + + :returns: VirtualHost object that's the best match for target name + :rtype: `obj.VirtualHost` or None + """ + filtered_vhosts = [] + for vhost in self.vhosts: + if any(a.is_wildcard() or a.get_port() == port for a in vhost.addrs) and not vhost.ssl: + filtered_vhosts.append(vhost) + return self._find_best_vhost(target, filtered_vhosts, filter_defaults) + + def _find_best_vhost(self, target_name, vhosts=None, filter_defaults=True): """Finds the best vhost for a target_name. This does not upgrade a vhost to HTTPS... it only finds the most appropriate vhost for the given target_name. + :param str target_name: domain handled by the desired vhost + :param vhosts: vhosts to consider + :type vhosts: `collections.Iterable` of :class:`~certbot_apache.obj.VirtualHost` + :param bool filter_defaults: whether a vhost with a _default_ + addr is acceptable + :returns: VHost or None """ @@ -456,7 +476,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Points 1 - Address name with no SSL best_candidate = None best_points = 0 - for vhost in self.vhosts: + + if vhosts is None: + vhosts = self.vhosts + + for vhost in vhosts: if vhost.modmacro is True: continue names = vhost.get_names() @@ -480,8 +504,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # No winners here... is there only one reasonable vhost? if best_candidate is None: - # reasonable == Not all _default_ addrs - vhosts = self._non_default_vhosts() + if filter_defaults: + vhosts = self._non_default_vhosts(vhosts) # remove mod_macro hosts from reasonable vhosts reasonable_vhosts = [vh for vh in vhosts if vh.modmacro is False] @@ -490,9 +514,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return best_candidate - def _non_default_vhosts(self): + def _non_default_vhosts(self, vhosts): """Return all non _default_ only vhosts.""" - return [vh for vh in self.vhosts if not all( + return [vh for vh in vhosts if not all( addr.get_addr() == "_default_" for addr in vh.addrs )] @@ -1911,7 +1935,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ self._chall_out.update(achalls) responses = [None] * len(achalls) - self.http_doer = http_01.ApacheHttp01(self) + http_doer = http_01.ApacheHttp01(self) sni_doer = tls_sni_01.ApacheTlsSni01(self) for i, achall in enumerate(achalls): @@ -1919,11 +1943,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # challenge. This helps to put all of the responses back together # when they are all complete. if isinstance(achall.chall, challenges.HTTP01): - self.http_doer.add_chall(achall, i) + http_doer.add_chall(achall, i) else: # tls-sni-01 sni_doer.add_chall(achall, i) - http_response = self.http_doer.perform() + http_response = http_doer.perform() sni_response = sni_doer.perform() if http_response or sni_response: # Must reload in order to activate the challenges. @@ -1935,7 +1959,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # of identifying when the new configuration is being used. time.sleep(3) - self._update_responses(responses, http_response, self.http_doer) + self._update_responses(responses, http_response, http_doer) self._update_responses(responses, sni_response, sni_doer) return responses diff --git a/certbot-apache/certbot_apache/http_01.py b/certbot-apache/certbot_apache/http_01.py index 3b942823d..e463f3880 100644 --- a/certbot-apache/certbot_apache/http_01.py +++ b/certbot-apache/certbot_apache/http_01.py @@ -2,31 +2,33 @@ import logging import os +from certbot import errors + from certbot.plugins import common logger = logging.getLogger(__name__) class ApacheHttp01(common.TLSSNI01): - """Class that performs HTPP-01 challenges within the Apache configurator.""" - - CONFIG_TEMPLATE24 = """\ -Alias /.well-known/acme-challenge {0} - - - Require all granted - - -""" + """Class that performs HTTP-01 challenges within the Apache configurator.""" CONFIG_TEMPLATE22 = """\ -Alias /.well-known/acme-challenge {0} + RewriteEngine on + RewriteRule ^/\\.well-known/acme-challenge/([A-Za-z0-9-_=]+)$ {0}/$1 [L] - - Order allow,deny - Allow from all - + + Order Allow,Deny + Allow from all + + """ -""" + CONFIG_TEMPLATE24 = """\ + RewriteEngine on + RewriteRule ^/\\.well-known/acme-challenge/([A-Za-z0-9-_=]+)$ {0}/$1 [END] + + + Require all granted + + """ def __init__(self, *args, **kwargs): super(ApacheHttp01, self).__init__(*args, **kwargs) @@ -36,6 +38,7 @@ Alias /.well-known/acme-challenge {0} self.challenge_dir = os.path.join( self.configurator.config.work_dir, "http_challenges") + self.moded_vhosts = set() def perform(self): """Perform all HTTP-01 challenges.""" @@ -50,6 +53,7 @@ Alias /.well-known/acme-challenge {0} self.prepare_http01_modules() responses = self._set_up_challenges() + self._mod_config() # Save reversible changes self.configurator.save("HTTP Challenge", True) @@ -60,7 +64,7 @@ Alias /.well-known/acme-challenge {0} """Make sure that we have the needed modules available for http01""" if self.configurator.conf("handle-modules"): - needed_modules = ["alias"] + needed_modules = ["rewrite"] if self.configurator.version < (2, 4): needed_modules.append("authz_host") else: @@ -70,8 +74,16 @@ Alias /.well-known/acme-challenge {0} self.configurator.enable_mod(mod, temp=True) def _mod_config(self): - self.configurator.parser.add_include( - self.configurator.parser.loc["default"], self.challenge_conf) + 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_directive(vh) + else: + for vh in self._relevant_vhosts(): + self._set_up_include_directive(vh) + self.configurator.reverter.register_file_creation( True, self.challenge_conf) @@ -79,12 +91,29 @@ Alias /.well-known/acme-challenge {0} config_template = self.CONFIG_TEMPLATE22 else: config_template = self.CONFIG_TEMPLATE24 + config_text = config_template.format(self.challenge_dir) logger.debug("writing a config file with text:\n %s", config_text) with open(self.challenge_conf, "w") as new_conf: new_conf.write(config_text) + def _relevant_vhosts(self): + http01_port = str(self.configurator.config.http01_port) + relevant_vhosts = [] + for vhost in self.configurator.vhosts: + if any(a.is_wildcard() or a.get_port() == http01_port for a in vhost.addrs): + if not vhost.ssl: + relevant_vhosts.append(vhost) + if not relevant_vhosts: + raise errors.PluginError( + "Unable to find a virtual host listening on port {0} which is" + " currently needed for Certbot to prove to the CA that you" + " control your domain. Please add a virtual host for port" + " {0}.".format(http01_port)) + + return relevant_vhosts + def _set_up_challenges(self): if not os.path.isdir(self.challenge_dir): os.makedirs(self.challenge_dir) @@ -107,3 +136,15 @@ Alias /.well-known/acme-challenge {0} os.chmod(name, 0o644) return response + + def _set_up_include_directive(self, vhost): + """Includes override configuration to the beginning of VirtualHost. + Note that this include isn't added to Augeas search tree""" + + if vhost not in self.moded_vhosts: + logger.debug( + "Adding a temporary challenge validation Include for name: %s " + + "in: %s", vhost.name, vhost.filep) + self.configurator.parser.add_dir_beginning( + vhost.path, "Include", self.challenge_conf) + self.moded_vhosts.add(vhost) diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py index 7715d2c35..d7da1e55e 100644 --- a/certbot-apache/certbot_apache/parser.py +++ b/certbot-apache/certbot_apache/parser.py @@ -332,6 +332,23 @@ class ApacheParser(object): else: self.aug.set(aug_conf_path + "/directive[last()]/arg", args) + def add_dir_beginning(self, aug_conf_path, dirname, args): + """Adds the directive to the beginning of defined aug_conf_path. + + :param str aug_conf_path: Augeas configuration path to add directive + :param str dirname: Directive to add + :param args: Value of the directive. ie. Listen 443, 443 is arg + :type args: list or str + """ + first_dir = aug_conf_path + "/directive[1]" + self.aug.insert(first_dir, "directive", True) + self.aug.set(first_dir, dirname) + if isinstance(args, list): + for i, value in enumerate(args, 1): + self.aug.set(first_dir + "/arg[%d]" % (i), value) + else: + self.aug.set(first_dir + "/arg", args) + def find_dir(self, directive, arg=None, start=None, exclude=True): """Finds directive in the configuration. diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index df67104da..530d75a92 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -126,7 +126,7 @@ class MultipleVhostsTest(util.ApacheTest): names = self.config.get_all_names() self.assertEqual(names, set( ["certbot.demo", "ocspvhost.com", "encryption-example.demo", - "nonsym.link", "vhost.in.rootconf"] + "nonsym.link", "vhost.in.rootconf", "www.certbot.demo"] )) @certbot_util.patch_get_utility() @@ -146,7 +146,7 @@ class MultipleVhostsTest(util.ApacheTest): names = self.config.get_all_names() # Names get filtered, only 5 are returned - self.assertEqual(len(names), 7) + self.assertEqual(len(names), 8) self.assertTrue("zombo.com" in names) self.assertTrue("google.com" in names) self.assertTrue("certbot.demo" in names) @@ -260,6 +260,20 @@ class MultipleVhostsTest(util.ApacheTest): self.assertRaises( errors.PluginError, self.config.choose_vhost, "none.com") + def test_find_best_http_vhost_default(self): + vh = obj.VirtualHost( + "fp", "ap", set([obj.Addr.fromstring("_default_:80")]), False, True) + self.config.vhosts = [vh] + self.assertEqual(self.config.find_best_http_vhost("foo.bar", False), vh) + + def test_find_best_http_vhost_port(self): + port = "8080" + vh = obj.VirtualHost( + "fp", "ap", set([obj.Addr.fromstring("*:" + port)]), + False, True, "encryption-example.demo") + self.config.vhosts.append(vh) + self.assertEqual(self.config.find_best_http_vhost("foo.bar", False, port), vh) + def test_findbest_continues_on_short_domain(self): # pylint: disable=protected-access chosen_vhost = self.config._find_best_vhost("purple.com") @@ -305,7 +319,8 @@ class MultipleVhostsTest(util.ApacheTest): def test_non_default_vhosts(self): # pylint: disable=protected-access - self.assertEqual(len(self.config._non_default_vhosts()), 8) + vhosts = self.config._non_default_vhosts(self.config.vhosts) + self.assertEqual(len(vhosts), 8) def test_deploy_cert_enable_new_vhost(self): # Create diff --git a/certbot-apache/certbot_apache/tests/http_01_test.py b/certbot-apache/certbot_apache/tests/http_01_test.py index 204d9a76c..ee8566396 100644 --- a/certbot-apache/certbot_apache/tests/http_01_test.py +++ b/certbot-apache/certbot_apache/tests/http_01_test.py @@ -6,6 +6,7 @@ import unittest from acme import challenges from certbot import achallenges +from certbot import errors from certbot.tests import acme_util @@ -22,8 +23,9 @@ class ApacheHttp01TestMeta(type): def _gen_test(num_achalls, minor_version): def _test(self): achalls = self.achalls[:num_achalls] + vhosts = self.vhosts[:num_achalls] self.config.version = (2, minor_version) - self.common_perform_test(achalls) + self.common_perform_test(achalls, vhosts) return _test for i in range(1, NUM_ACHALLS + 1): @@ -43,16 +45,30 @@ class ApacheHttp01Test(util.ApacheTest): self.account_key = self.rsa512jwk self.achalls = [] + self.vhosts = [] + vhost_index = 0 for i in range(NUM_ACHALLS): + domain = None + # Find a vhost with a name/alias we can use + for j in range(vhost_index + 1, len(self.config.vhosts)): + vhost = self.config.vhosts[j] + domain = vhost.name if vhost.name else next(iter(vhost.aliases), None) + if domain: + self.vhosts.append(vhost) + vhost_index = j + 1 + break + else: # pragma: no cover + # If we didn't find a domain, we shouldn't continue the test. + self.fail("No usable vhost found") + self.achalls.append( achallenges.KeyAuthorizationAnnotatedChallenge( challb=acme_util.chall_to_challb( - challenges.HTTP01(token=((chr(ord('a') + i) * 16))), + challenges.HTTP01(token=((chr(ord('a') + i).encode() * 16))), "pending"), - domain="example{0}.com".format(i), - account_key=self.account_key)) + domain=domain, account_key=self.account_key)) - modules = ["alias", "authz_core", "authz_host"] + modules = ["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") @@ -81,9 +97,9 @@ class ApacheHttp01Test(util.ApacheTest): self.assertEqual(enmod_calls[0][0][0], "authz_core") def common_enable_modules_test(self, mock_enmod): - """Tests enabling mod_alias and other modules.""" - self.config.parser.modules.remove("alias_module") - self.config.parser.modules.remove("mod_alias.c") + """Tests enabling mod_rewrite and other modules.""" + self.config.parser.modules.remove("rewrite_module") + self.config.parser.modules.remove("mod_rewrite.c") self.http.prepare_http01_modules() @@ -91,14 +107,46 @@ class ApacheHttp01Test(util.ApacheTest): calls = mock_enmod.call_args_list other_calls = [] for call in calls: - if "alias" != call[0][0]: + if "rewrite" != call[0][0]: other_calls.append(call) - # If these lists are equal, we never enabled mod_alias + # If these lists are equal, we never enabled mod_rewrite self.assertNotEqual(calls, other_calls) return other_calls - def common_perform_test(self, achalls): + def test_same_vhost(self): + vhost = next(v for v in self.config.vhosts if v.name == "certbot.demo") + achalls = [ + achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.chall_to_challb( + challenges.HTTP01(token=((b'a' * 16))), + "pending"), + domain=vhost.name, account_key=self.account_key), + achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.chall_to_challb( + challenges.HTTP01(token=((b'b' * 16))), + "pending"), + domain=next(iter(vhost.aliases)), account_key=self.account_key) + ] + self.common_perform_test(achalls, [vhost]) + + def test_anonymous_vhost(self): + vhosts = [v for v in self.config.vhosts if not v.ssl] + achalls = [ + achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.chall_to_challb( + challenges.HTTP01(token=((b'a' * 16))), + "pending"), + domain="something.nonexistent", 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) + self.config.config.http01_port = 12345 + self.assertRaises(errors.PluginError, self.http.perform) + + def common_perform_test(self, achalls, vhosts): """Tests perform with the given achalls.""" challenge_dir = self.http.challenge_dir self.assertFalse(os.path.exists(challenge_dir)) @@ -116,19 +164,22 @@ class ApacheHttp01Test(util.ApacheTest): for achall in achalls: self._test_challenge_file(achall) + for vhost in vhosts: + if not vhost.ssl: + matches = self.config.parser.find_dir("Include", + self.http.challenge_conf, + vhost.path) + self.assertEqual(len(matches), 1) + self.assertTrue(os.path.exists(challenge_dir)) def _test_challenge_conf(self): - self.assertEqual( - len(self.config.parser.find_dir( - "Include", self.http.challenge_conf)), 1) - with open(self.http.challenge_conf) as f: conf_contents = f.read() - alias_fmt = "Alias /.well-known/acme-challenge {0}" - alias = alias_fmt.format(self.http.challenge_dir) - self.assertTrue(alias in conf_contents) + self.assertTrue("RewriteEngine on" in conf_contents) + self.assertTrue("RewriteRule" in conf_contents) + self.assertTrue(self.http.challenge_dir in conf_contents) if self.config.version < (2, 4): self.assertTrue("Allow from all" in conf_contents) else: diff --git a/certbot-apache/certbot_apache/tests/parser_test.py b/certbot-apache/certbot_apache/tests/parser_test.py index a9eb129c2..4496781c9 100644 --- a/certbot-apache/certbot_apache/tests/parser_test.py +++ b/certbot-apache/certbot_apache/tests/parser_test.py @@ -66,6 +66,23 @@ class BasicParserTest(util.ParserTest): for i, match in enumerate(matches): self.assertEqual(self.parser.aug.get(match), str(i + 1)) + def test_add_dir_beginning(self): + aug_default = "/files" + self.parser.loc["default"] + self.parser.add_dir_beginning(aug_default, + "AddDirectiveBeginning", + "testBegin") + + self.assertTrue( + self.parser.find_dir("AddDirectiveBeginning", "testBegin", aug_default)) + + self.assertEqual( + self.parser.aug.get(aug_default+"/directive[1]"), + "AddDirectiveBeginning") + self.parser.add_dir_beginning(aug_default, "AddList", ["1", "2", "3", "4"]) + matches = self.parser.find_dir("AddList", None, aug_default) + for i, match in enumerate(matches): + self.assertEqual(self.parser.aug.get(match), str(i + 1)) + def test_empty_arg(self): self.assertEquals(None, self.parser.get_arg("/files/whatever/nonexistent")) diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/certbot.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/certbot.conf index b3147a523..965ca2222 100644 --- a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/certbot.conf +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/certbot.conf @@ -1,5 +1,6 @@ ServerName certbot.demo +ServerAlias www.certbot.demo ServerAdmin webmaster@localhost DocumentRoot /var/www-certbot-reworld/static/ diff --git a/certbot-apache/certbot_apache/tests/util.py b/certbot-apache/certbot_apache/tests/util.py index 1ba1e2c34..1daaa00c5 100644 --- a/certbot-apache/certbot_apache/tests/util.py +++ b/certbot-apache/certbot_apache/tests/util.py @@ -170,7 +170,7 @@ def get_vh_truth(temp_dir, config_name): os.path.join(prefix, "certbot.conf"), os.path.join(aug_pre, "certbot.conf/VirtualHost"), set([obj.Addr.fromstring("*:80")]), False, True, - "certbot.demo"), + "certbot.demo", aliases=["www.certbot.demo"]), obj.VirtualHost( os.path.join(prefix, "mod_macro-example.conf"), os.path.join(aug_pre,