From 3964357eb3417b4bb3e3c61b611852983f8f08d4 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Feb 2016 15:48:36 -0800 Subject: [PATCH 1/5] rewrite generic files --- .../letsencrypt_apache/configurator.py | 7 +++++++ letsencrypt-apache/letsencrypt_apache/obj.py | 17 +++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index cbc451ac9..694767f2a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1074,6 +1074,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if not self._is_rewrite_engine_on(general_vh): self.parser.add_dir(general_vh.path, "RewriteEngine", "on") + for name in ssl_vhost.get_names(): + self.parser.add_dir(general_vh.path, "RewriteCond", + ["%{SERVER_NAME}", "={0}".format(name), "[OR]"] if self.get_version() >= (2, 3, 9): self.parser.add_dir(general_vh.path, "RewriteRule", constants.REWRITE_HTTPS_ARGS_WITH_END) @@ -1243,6 +1246,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for http_vh in candidate_http_vhs: if http_vh.same_server(ssl_vhost): return http_vh + # Third filter - if none with same names, return generic + for http_vh in candidate_http_vhs: + if http_vh.same_server(ssl_vhost, generic=True): + return http_vh return None diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index 175ce3f92..c98ca4b99 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -189,7 +189,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods return True return False - def same_server(self, vhost): + def same_server(self, vhost, generic=False): """Determines if the vhost is the same 'server'. Used in redirection - indicates whether or not the two virtual hosts @@ -199,12 +199,17 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods """ - if vhost.get_names() != self.get_names(): - return False + if not generic: + if vhost.get_names() != self.get_names(): + return False - # If equal and set is not empty... assume same server - if self.name is not None or self.aliases: - return True + # If equal and set is not empty... assume same server + if self.name is not None or self.aliases: + return True + # If we're looking for a generic vhost, don't return one with a ServerName + else: + if self.name: + return False # Both sets of names are empty. From bf30e54a32f91178d8d84305e2db4fb11592bb70 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Feb 2016 15:58:53 -0800 Subject: [PATCH 2/5] fix syntax and don't have unneeded ors --- letsencrypt-apache/letsencrypt_apache/configurator.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 694767f2a..2198cbdfb 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1073,10 +1073,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # even with save() and load() if not self._is_rewrite_engine_on(general_vh): self.parser.add_dir(general_vh.path, "RewriteEngine", "on") - - for name in ssl_vhost.get_names(): + cond = "[OR]" + names = ssl_vhost.get_names() + for idx, name in enumerate(names): + if idx == len(names) - 1: + cond = "" self.parser.add_dir(general_vh.path, "RewriteCond", - ["%{SERVER_NAME}", "={0}".format(name), "[OR]"] + ["%{SERVER_NAME}", "={0}".format(name), cond]) if self.get_version() >= (2, 3, 9): self.parser.add_dir(general_vh.path, "RewriteRule", constants.REWRITE_HTTPS_ARGS_WITH_END) From 109d6caf6525e1673e4d126bd1a73530645be9bf Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Feb 2016 16:07:03 -0800 Subject: [PATCH 3/5] fix how OR is added --- letsencrypt-apache/letsencrypt_apache/configurator.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 2198cbdfb..9cff002f1 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1073,13 +1073,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # even with save() and load() if not self._is_rewrite_engine_on(general_vh): self.parser.add_dir(general_vh.path, "RewriteEngine", "on") - cond = "[OR]" names = ssl_vhost.get_names() for idx, name in enumerate(names): + args = ["%{SERVER_NAME}", "={0}".format(name), "[OR]"] if idx == len(names) - 1: - cond = "" - self.parser.add_dir(general_vh.path, "RewriteCond", - ["%{SERVER_NAME}", "={0}".format(name), cond]) + args.pop() + self.parser.add_dir(general_vh.path, "RewriteCond", args) if self.get_version() >= (2, 3, 9): self.parser.add_dir(general_vh.path, "RewriteRule", constants.REWRITE_HTTPS_ARGS_WITH_END) From c8a9da844294829d8f7c3f5a882f7d9ce5f2a4e1 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Feb 2016 16:47:29 -0800 Subject: [PATCH 4/5] update test to hit new line in configurator --- .../letsencrypt_apache/tests/configurator_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 5b15a20d1..58ec3fe21 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -746,9 +746,9 @@ class TwoVhost80Test(util.ApacheTest): self.config.parser.modules.add("rewrite_module") mock_exe.return_value = True ssl_vh = obj.VirtualHost( - "fp", "ap", set([obj.Addr(("*", "443")), - obj.Addr(("satoshi.com",))]), + "fp", "ap", set([obj.Addr(("*", "443"))]), True, False) + ssl_vh.name = "satoshi.com" self.config.vhosts.append(ssl_vh) self.assertRaises( errors.PluginError, From b81079be3b373bb49304b7a61c04f0ab8835433b Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Mon, 22 Feb 2016 18:08:06 -0800 Subject: [PATCH 5/5] brad nits --- letsencrypt-apache/letsencrypt_apache/obj.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index c98ca4b99..b2a21ef5d 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -194,6 +194,8 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods Used in redirection - indicates whether or not the two virtual hosts serve on the exact same IP combinations, but different ports. + The generic flag indicates that that we're trying to match to a + default or generic vhost .. todo:: Handle _default_ @@ -207,8 +209,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods if self.name is not None or self.aliases: return True # If we're looking for a generic vhost, don't return one with a ServerName - else: - if self.name: + elif self.name: return False # Both sets of names are empty.