filter by port in http01

certbot-apache/certbot_apache/configurator.py

@@ -436,7 +436,24 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 return True
         return False
 
-    def _find_best_vhost(self, target_name, vhosts=None):
+    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
@@ -445,6 +462,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :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
 
@@ -485,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(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]

certbot-apache/certbot_apache/http_01.py

@@ -7,26 +7,26 @@ 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}
-
-<Directory {0} >
-    Require all granted
-</Directory>
-
-"""
+    """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]
 
-<Directory {0} >
-    Order allow,deny
-    Allow from all
-</Directory>
+        <Directory {0}>
+            Order Allow,Deny
+            Allow from all
+        </Directory>
+    """
 
-"""
+    CONFIG_TEMPLATE24 = """\
+        RewriteEngine on
+        RewriteRule ^/\\.well-known/acme-challenge/([A-Za-z0-9-_=]+)$ {0}/$1 [END]
+
+        <Directory {0}>
+            Require all granted
+        </Directory>
+    """
 
     def __init__(self, *args, **kwargs):
         super(ApacheHttp01, self).__init__(*args, **kwargs)
@@ -50,6 +50,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 +61,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 +71,15 @@ 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)
+        moded_vhosts = set()
+        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 and vh not in moded_vhosts:
+                self._set_up_include_directive(vh)
+                moded_vhosts.add(vh)
+
         self.configurator.reverter.register_file_creation(
             True, self.challenge_conf)
 
@@ -79,12 +87,14 @@ 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 _set_up_challenges(self):
         if not os.path.isdir(self.challenge_dir):
             os.makedirs(self.challenge_dir)
@@ -107,3 +117,9 @@ 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"""
+        self.configurator.parser.add_dir_beginning(vhost.path, "Include",
+                                                   self.challenge_conf)

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.
 

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")

certbot-apache/certbot_apache/tests/http_01_test.py

@@ -22,8 +22,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 +44,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))),
                         "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 +96,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 +106,30 @@ 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 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 +147,21 @@ class ApacheHttp01Test(util.ApacheTest):
         for achall in achalls:
             self._test_challenge_file(achall)
 
+        for vhost in vhosts:
+            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:

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"))

certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/certbot.conf

@@ -1,5 +1,6 @@
 <VirtualHost *:80>
 ServerName certbot.demo
+ServerAlias www.certbot.demo
 ServerAdmin webmaster@localhost
 
 DocumentRoot /var/www-certbot-reworld/static/

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,