Merge remote-tracking branch 'upstream/master' into bugfix_redirect

4 files changed, 85 insertions, 20 deletions

diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/README b/letsencrypt-apache/letsencrypt_apache/augeas_lens/README
index fc803a776..f801efd43 100644
--- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/README
+++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/README
@@ -1,2 +1,2 @@
 Let's Encrypt includes the very latest Augeas lenses in order to ship bug fixes 
-to Apacche configuration handling bugs as quickly as possible
+to Apache configuration handling bugs as quickly as possible
diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug
index 30d8ca501..0669896a0 100644
--- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug
+++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug
@@ -59,8 +59,10 @@ let empty               = Util.empty_dos
 let indent              = Util.indent
 
 (* borrowed from shellvars.aug *)
-let char_arg_dir  = /([^\\ '"\t\r\n]|[^\\ '"\t\r\n][^ '"\t\r\n]*[^\\ '"\t\r\n])|\\\\"|\\\\'/
+let char_arg_dir  = /([^\\ '"{\t\r\n]|[^ '"{\t\r\n]+[^\\ '"\t\r\n])|\\\\"|\\\\'/
 let char_arg_sec  = /[^ '"\t\r\n>]|\\\\"|\\\\'/
+let char_arg_wl   = /([^\\ '"},\t\r\n]|[^ '"},\t\r\n]+[^\\ '"},\t\r\n])/
+
 let cdot = /\\\\./
 let cl = /\\\\\n/
 let dquot =
@@ -77,11 +79,19 @@ let comp = /[<>=]?=/
 
 let arg_dir = [ label "arg" . store (char_arg_dir+|dquot|squot) ]
 let arg_sec = [ label "arg" . store (char_arg_sec+|comp|dquot|squot) ]
+let arg_wl  = [ label "arg" . store (char_arg_wl+|dquot|squot) ]
+
+(* comma-separated wordlist as permitted in the SSLRequire directive *)
+let arg_wordlist =
+     let wl_start = Util.del_str "{" in
+     let wl_end   = Util.del_str "}" in
+     let wl_sep   = del /[ \t]*,[ \t]*/ ", "
+  in [ label "wordlist" . wl_start . arg_wl . (wl_sep . arg_wl)* . wl_end ]
 
 let argv (l:lens) = l . (sep_spc . l)*
 
 let directive = [ indent . label "directive" . store word .
-                  (sep_spc . argv arg_dir)? . eol ]
+                  (sep_spc . argv (arg_dir|arg_wordlist))? . eol ]
 
 let section (body:lens) =
     (* opt_eol includes empty lines *)
@@ -91,7 +101,7 @@ let section (body:lens) =
              indent . dels "</" in
     let kword = key word in
     let dword = del word "a" in
-        [ indent . dels "<" . square kword inner dword . del ">" ">" . eol ]
+        [ indent . dels "<" . square kword inner dword . del />[ \t\n\r]*/ ">\n" ]
 
 let rec content = section (content|directive)
 
diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py
index 33a9ea9db..f72492ac2 100644
--- a/letsencrypt-apache/letsencrypt_apache/configurator.py
+++ b/letsencrypt-apache/letsencrypt_apache/configurator.py
@@ -93,7 +93,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         add("enmod", default=constants.CLI_DEFAULTS["enmod"],
             help="Path to the Apache 'a2enmod' binary.")
         add("dismod", default=constants.CLI_DEFAULTS["dismod"],
-            help="Path to the Apache 'a2enmod' binary.")
+            help="Path to the Apache 'a2dismod' binary.")
         add("le-vhost-ext", default=constants.CLI_DEFAULTS["le_vhost_ext"],
             help="SSL vhost configuration extension.")
         add("server-root", default=constants.CLI_DEFAULTS["server_root"],
@@ -120,7 +120,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         self.version = version
         self.vhosts = None
         self._enhance_func = {"redirect": self._enable_redirect,
-                "ensure-http-header": self._set_http_header}
+                              "ensure-http-header": self._set_http_header}
 
     @property
     def mod_ssl_conf(self):
@@ -545,21 +545,43 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         # Check for Listen <port>
         # Note: This could be made to also look for ip:443 combo
-        if not self.parser.find_dir("Listen", port):
-            logger.debug("No Listen %s directive found. Setting the "
-                         "Apache Server to Listen on port %s", port, port)
-
-            if port == "443":
-                args = [port]
+        listens = [self.parser.get_arg(x).split()[0] for x in self.parser.find_dir("Listen")]
+        # In case no Listens are set (which really is a broken apache config)
+        if not listens:
+            listens = ["80"]
+        for listen in listens:
+            # For any listen statement, check if the machine also listens on Port 443.
+            # If not, add such a listen statement.
+            if len(listen.split(":")) == 1:
+                # Its listening to all interfaces
+                if port not in listens:
+                    if port == "443":
+                        args = [port]
+                    else:
+                        # Non-standard ports should specify https protocol
+                        args = [port, "https"]
+                    self.parser.add_dir_to_ifmodssl(
+                        parser.get_aug_path(
+                            self.parser.loc["listen"]), "Listen", args)
+                    self.save_notes += "Added Listen %s directive to %s\n" % (
+                        port, self.parser.loc["listen"])
+                    listens.append(port)
             else:
-                # Non-standard ports should specify https protocol
-                args = [port, "https"]
-
-            self.parser.add_dir_to_ifmodssl(
-                parser.get_aug_path(
-                    self.parser.loc["listen"]), "Listen", args)
-            self.save_notes += "Added Listen %s directive to %s\n" % (
-                port, self.parser.loc["listen"])
+                # The Listen statement specifies an ip
+                _, ip = listen[::-1].split(":", 1)
+                ip = ip[::-1]
+                if "%s:%s" % (ip, port) not in listens:
+                    if port == "443":
+                        args = ["%s:%s" % (ip, port)]
+                    else:
+                        # Non-standard ports should specify https protocol
+                        args = ["%s:%s" % (ip, port), "https"]
+                    self.parser.add_dir_to_ifmodssl(
+                        parser.get_aug_path(
+                            self.parser.loc["listen"]), "Listen", args)
+                    self.save_notes += "Added Listen %s:%s directive to %s\n" % (
+                                       ip, port, self.parser.loc["listen"])
+                    listens.append("%s:%s" % (ip, port))
 
     def make_addrs_sni_ready(self, addrs):
         """Checks to see if the server is ready for SNI challenges.
diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py
index e05d9893f..f2bf89d2c 100644
--- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py
+++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py
@@ -391,6 +391,39 @@ class TwoVhost80Test(util.ApacheTest):
 
         self.assertEqual(mock_add_dir.call_count, 2)
 
+    def test_prepare_server_https_named_listen(self):
+        mock_find = mock.Mock()
+        mock_find.return_value = ["test1", "test2", "test3"]
+        mock_get = mock.Mock()
+        mock_get.side_effect = ["1.2.3.4:80", "[::1]:80", "1.1.1.1:443"]
+        mock_add_dir = mock.Mock()
+        mock_enable = mock.Mock()
+
+        self.config.parser.find_dir = mock_find
+        self.config.parser.get_arg = mock_get
+        self.config.parser.add_dir_to_ifmodssl = mock_add_dir
+        self.config.enable_mod = mock_enable
+
+        # Test Listen statements with specific ip listeed
+        self.config.prepare_server_https("443")
+        # Should only be 2 here, as the third interface already listens to the correct port
+        self.assertEqual(mock_add_dir.call_count, 2)
+
+        # Check argument to new Listen statements
+        self.assertEqual(mock_add_dir.call_args_list[0][0][2], ["1.2.3.4:443"])
+        self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:443"])
+
+        # Reset return lists and inputs
+        mock_add_dir.reset_mock()
+        mock_get.side_effect = ["1.2.3.4:80", "[::1]:80", "1.1.1.1:443"]
+
+        # Test
+        self.config.prepare_server_https("8080", temp=True)
+        self.assertEqual(mock_add_dir.call_count, 3)
+        self.assertEqual(mock_add_dir.call_args_list[0][0][2], ["1.2.3.4:8080", "https"])
+        self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:8080", "https"])
+        self.assertEqual(mock_add_dir.call_args_list[2][0][2], ["1.1.1.1:8080", "https"])
+
     def test_make_vhost_ssl(self):
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])