From 707bb55c81a47c9305b39ff55ce3adf9c40d981f Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 18 Nov 2015 18:59:22 -0800 Subject: [PATCH 01/11] change to not make a permanent file if just doing dvsni --- letsencrypt-apache/letsencrypt_apache/configurator.py | 4 +++- letsencrypt-apache/letsencrypt_apache/dvsni.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index f10f0c241..64449302a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -234,7 +234,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if not vhost.enabled: self.enable_site(vhost) - def choose_vhost(self, target_name): + def choose_vhost(self, target_name, dvsni=False): """Chooses a virtual host based on the given domain name. If there is no clear virtual host to be selected, the user is prompted @@ -255,6 +255,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Try to find a reasonable vhost vhost = self._find_best_vhost(target_name) if vhost is not None: + if dvsni: + return vhost if not vhost.ssl: vhost = self.make_vhost_ssl(vhost) diff --git a/letsencrypt-apache/letsencrypt_apache/dvsni.py b/letsencrypt-apache/letsencrypt_apache/dvsni.py index 2f9e9ed18..0dd411e4f 100644 --- a/letsencrypt-apache/letsencrypt_apache/dvsni.py +++ b/letsencrypt-apache/letsencrypt_apache/dvsni.py @@ -110,7 +110,7 @@ class ApacheDvsni(common.TLSSNI01): def get_dvsni_addrs(self, achall): """Return the Apache addresses needed for DVSNI.""" - vhost = self.configurator.choose_vhost(achall.domain) + vhost = self.configurator.choose_vhost(achall.domain, dvsni=True) # TODO: Checkout _default_ rules. dvsni_addrs = set() From 793f2b4f9016bfe14933fe32beb0f7103f06d9ca Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Fri, 20 Nov 2015 11:42:08 -0800 Subject: [PATCH 02/11] fixed lint scoping issue --- letsencrypt-apache/letsencrypt_apache/configurator.py | 4 ++-- letsencrypt-apache/letsencrypt_apache/dvsni.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 64449302a..d88480d0a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -234,7 +234,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if not vhost.enabled: self.enable_site(vhost) - def choose_vhost(self, target_name, dvsni=False): + def choose_vhost(self, target_name, temp=False): """Chooses a virtual host based on the given domain name. If there is no clear virtual host to be selected, the user is prompted @@ -255,7 +255,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 dvsni: + if temp: return vhost if not vhost.ssl: vhost = self.make_vhost_ssl(vhost) diff --git a/letsencrypt-apache/letsencrypt_apache/dvsni.py b/letsencrypt-apache/letsencrypt_apache/dvsni.py index 0dd411e4f..3e1bc87b7 100644 --- a/letsencrypt-apache/letsencrypt_apache/dvsni.py +++ b/letsencrypt-apache/letsencrypt_apache/dvsni.py @@ -110,7 +110,7 @@ class ApacheDvsni(common.TLSSNI01): def get_dvsni_addrs(self, achall): """Return the Apache addresses needed for DVSNI.""" - vhost = self.configurator.choose_vhost(achall.domain, dvsni=True) + vhost = self.configurator.choose_vhost(achall.domain, temp=True) # TODO: Checkout _default_ rules. dvsni_addrs = set() From 0017e4887045139baaff7f1ae538d037bc2b79c2 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Sat, 21 Nov 2015 08:53:11 -0800 Subject: [PATCH 03/11] added docstring for temp --- letsencrypt-apache/letsencrypt_apache/configurator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index e6f7ed270..6edffcbe6 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -237,6 +237,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): with all available choices. :param str target_name: domain name + :param bool temp: whether or not self.make_vhost_ssl shouldn't be called :returns: ssl vhost associated with name :rtype: :class:`~letsencrypt_apache.obj.VirtualHost` From 699cdac8eacfec825d960e16edaa78ddb9267ba1 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Mon, 23 Nov 2015 10:08:43 -0800 Subject: [PATCH 04/11] remove error if can't parse config --- letsencrypt-apache/letsencrypt_apache/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index ec5211ae4..b48551d00 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -101,7 +101,7 @@ class ApacheParser(object): try: matches.remove("DUMP_RUN_CFG") except ValueError: - raise errors.PluginError("Unable to parse runtime variables") + #raise errors.PluginError("Unable to parse runtime variables") for match in matches: if match.count("=") > 1: From 2738290b98ef289d0800e380e5b5b323b65f8f36 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Mon, 23 Nov 2015 10:21:31 -0800 Subject: [PATCH 05/11] fix issue with empty matchers array --- letsencrypt-apache/letsencrypt_apache/parser.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index b48551d00..5c18208bb 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -101,6 +101,7 @@ class ApacheParser(object): try: matches.remove("DUMP_RUN_CFG") except ValueError: + return #raise errors.PluginError("Unable to parse runtime variables") for match in matches: From d4ee483662e345bfc9a6e106806e790facdb99b2 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Mon, 23 Nov 2015 10:52:39 -0800 Subject: [PATCH 06/11] revert changes made to wrong branch --- letsencrypt-apache/letsencrypt_apache/parser.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 5c18208bb..ec5211ae4 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -101,8 +101,7 @@ class ApacheParser(object): try: matches.remove("DUMP_RUN_CFG") except ValueError: - return - #raise errors.PluginError("Unable to parse runtime variables") + raise errors.PluginError("Unable to parse runtime variables") for match in matches: if match.count("=") > 1: From bc6064addd34126708ec44aac4d209d4d5bff3da Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 24 Nov 2015 14:58:03 -0800 Subject: [PATCH 07/11] propogate temp and fix docstring --- .../letsencrypt_apache/configurator.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 6edffcbe6..c4305f724 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -236,8 +236,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): 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. + :param str target_name: domain name - :param bool temp: whether or not self.make_vhost_ssl shouldn't be called + :param bool temp: whether the vhost is only used temporarily :returns: ssl vhost associated with name :rtype: :class:`~letsencrypt_apache.obj.VirtualHost` @@ -260,9 +264,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.assoc[target_name] = vhost return vhost - return self._choose_vhost_from_list(target_name) + return self._choose_vhost_from_list(target_name, temp) - def _choose_vhost_from_list(self, target_name): + def _choose_vhost_from_list(self, target_name, temp=False): # Select a vhost from a list vhost = display_ops.select_vhost(target_name, self.vhosts) if vhost is None: @@ -275,7 +279,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): elif not vhost.ssl: addrs = self._get_proposed_addrs(vhost, "443") # TODO: Conflicts is too conservative - if not any(vhost.enabled and vhost.conflicts(addrs) for vhost in self.vhosts): + if not any(vhost.enabled and vhost.conflicts(addrs) for vhost in self.vhosts)\ + and not temp: vhost = self.make_vhost_ssl(vhost) else: logger.error( From 739fd6614b7e60b3451c41d7cffdc515c55f050f Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 1 Dec 2015 09:50:13 -0800 Subject: [PATCH 08/11] moved temp check --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index c4305f724..34c64b87b 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -276,11 +276,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "in the Apache config", target_name) raise errors.PluginError("No vhost selected") + elif temp: + return vhost elif not vhost.ssl: addrs = self._get_proposed_addrs(vhost, "443") # TODO: Conflicts is too conservative - if not any(vhost.enabled and vhost.conflicts(addrs) for vhost in self.vhosts)\ - and not temp: + if not any(vhost.enabled and vhost.conflicts(addrs) for vhost in self.vhosts): vhost = self.make_vhost_ssl(vhost) else: logger.error( From e846b157edc45130ec4b34739a1ac5fb0f3e9564 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 1 Dec 2015 09:59:45 -0800 Subject: [PATCH 09/11] removed space --- letsencrypt-apache/letsencrypt_apache/configurator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 34c64b87b..b1feca5d5 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -275,7 +275,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "No vhost was selected. Please specify servernames " "in the Apache config", target_name) raise errors.PluginError("No vhost selected") - elif temp: return vhost elif not vhost.ssl: From ab32e2fd26747f8f029af8901d6f2204e5c38a3f Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 1 Dec 2015 10:46:13 -0800 Subject: [PATCH 10/11] fix docstring --- letsencrypt-apache/letsencrypt_apache/tls_sni_01.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index ff7972e1d..4284e240c 100644 --- a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -110,7 +110,7 @@ class ApacheTlsSni01(common.TLSSNI01): return addrs def _get_addrs(self, achall): - """Return the Apache addresses needed for DVSNI.""" + """Return the Apache addresses needed for TLS-SNI-01.""" vhost = self.configurator.choose_vhost(achall.domain, temp=True) # TODO: Checkout _default_ rules. addrs = set() From ec28094ae2fecb2ce964ad0ee2f01de075300b54 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 1 Dec 2015 16:28:15 -0800 Subject: [PATCH 11/11] added test for new temp elif --- .../letsencrypt_apache/tests/configurator_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 0b6170e1d..db5f2e340 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -139,6 +139,12 @@ class TwoVhost80Test(util.ApacheTest): self.assertFalse(self.vh_truth[0].ssl) self.assertTrue(chosen_vhost.ssl) + @mock.patch("letsencrypt_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) + self.assertEqual(self.vh_truth[0], chosen_vhost) + @mock.patch("letsencrypt_apache.display_ops.select_vhost") def test_choose_vhost_select_vhost_conflicting_non_ssl(self, mock_select): mock_select.return_value = self.vh_truth[3]