Parse variables and fnmatch

letsencrypt-apache/letsencrypt_apache/configurator.py

@@ -64,7 +64,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
     This class can adequately configure most typical configurations but
     is not ready to handle very complex configurations.
 
-    .. todo:: Always use self.parser.aug_get rather than self.aug.get
     .. todo:: Verify permissions on configuration root... it is easier than
         checking permissions on each of the relative directories and less error
         prone.
@@ -332,7 +331,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         for name in name_match:
             args = self.aug.match(name + "/*")
             for arg in args:
-                host.add_name(self.aug.get(arg))
+                host.add_name(self.parser.get_arg(arg))
 
     def _create_vhost(self, path):
         """Used by get_virtual_hosts to create vhost objects
@@ -346,7 +345,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         addrs = set()
         args = self.aug.match(path + "/arg")
         for arg in args:
-            addrs.add(common.Addr.fromstring(self.aug.get(arg)))
+            addrs.add(common.Addr.fromstring(self.parser.get_arg(arg)))
         is_ssl = False
 
         if self.parser.find_dir(
@@ -514,7 +513,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         for addr in ssl_addr_p:
             old_addr = common.Addr.fromstring(
-                str(self.aug.get(addr)))
+                str(self.parser.get_arg(addr)))
             ssl_addr = old_addr.get_addr_obj("443")
             self.aug.set(addr, str(ssl_addr))
             ssl_addrs.add(ssl_addr)
@@ -868,8 +867,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                     errors.MisconfigurationError(
                         "Too many cert/key directives in vhost")
 
-                cert = os.path.abspath(self.aug.get(cert_path[0]))
+                cert = os.path.abspath(self.parser.get_arg(cert_path[0]))
-                key = os.path.abspath(self.aug.get(key_path[0]))
+                key = os.path.abspath(self.parser.get_arg(key_path[0]))
                 c_k.add((cert, key, get_file_path(cert_path[0])))
 
         return c_k
@@ -942,13 +941,19 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 "and try again." % mod_name)
 
         self._enable_mod_debian(mod_name)
+        self.save_notes += "Enabled %s module in Apache" % mod_name
+        logger.debug("Enabled Apache %s module", mod_name)
+
+        # Modules can enable additional config files. Variables may be defined
+        # within these new configuration sections.
+        # Restart is not necessary as DUMP_RUN_CFG uses latest config.
+        self.parser.update_runtime_variables()
 
         self.parser.modules.add(mod_name + "_module")
         self.parser.modules.add("mod_" + mod_name)
 
     def _enable_mod_debian(self, mod_name):
         """Assumes mods-available, mods-enabled layout."""
-
         # TODO: This can be further updated to not require all files.
         if mod_name == "ssl":
             self._enable_mod_debian_files(["ssl.conf", "ssl.load"])
@@ -957,7 +962,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         else:
             raise NotImplemented
 
-    def _enable_mod_debian_files(self, filenames):
+    def _enable_mod_debian_files(self, filenames, mod_name):
         """Move over all required files into mods-enabled."""
         mods_available = os.path.join(self.parser.root, "mods-available")
         mods_enabled = os.path.join(self.parser.root, "mods-enabled")
@@ -972,10 +977,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Register and symlink files
         for filename in files:
             enabled_path = os.path.join(mods_enabled, filename)
+            if os.path.isfile(enabled_path):
+                logger.debug(
+                    "Error - enabling module %s, filepath already exists "
+                    "%s", mod_name, enabled_path)
+                raise errors.PluginError("Error enabling module %s" % mod_name)
             self.reverter.register_file_creation(False, enabled_path)
             os.symlink(os.path.join(mods_available, filename), enabled_path)
 
-
     def mod_loaded(self, module):
         """Checks to see if mod_ssl is loaded
 

letsencrypt-apache/letsencrypt_apache/parser.py

@@ -1,5 +1,6 @@
 """ApacheParser is a member object of the ApacheConfigurator class."""
 import collections
+import fnmatch
 import itertools
 import logging
 import os
@@ -13,6 +14,7 @@ logger = logging.getLogger(__name__)
 
 
 arg_var_interpreter = re.compile(r"\$\{[^ \}]*}")
+fnmatch_chars = set(["*", "?", "\\", "[", "]"])
 
 
 class ApacheParser(object):
@@ -29,7 +31,8 @@ class ApacheParser(object):
         # https://httpd.apache.org/docs/2.4/mod/core.html#define
         # https://httpd.apache.org/docs/2.4/mod/core.html#ifdefine
         # This only handles invocation parameters and Define directives!
-        self.variables = self._init_runtime_variables(ctl)
+        self.variables = {}
+        self.update_runtime_variables(ctl)
 
         # Find configuration root and make sure augeas can parse it.
         self.aug = aug
@@ -67,11 +70,11 @@ class ApacheParser(object):
 
             for match_name, match_filename in itertools.izip(
                     iterator, iterator):
-                self.modules.add(self.aug.get(match_name))
+                self.modules.add(self.get_arg(match_name))
                 self.modules.add(
-                    os.path.basename(self.aug.get(match_filename))[:-2] + "c")
+                    os.path.basename(self.get_arg(match_filename))[:-2] + "c")
 
-    def _init_runtime_variables(self, ctl):
+    def update_runtime_variables(self, ctl):
         """"
 
         .. note:: Compile time variables (apache2ctl -V) are not used within the
@@ -94,7 +97,7 @@ class ApacheParser(object):
             parts = match.partition("=")
             variables[parts[0]] = parts[2]
 
-        return variables
+        self.variables = variables
 
     def _get_runtime_cfg(self, ctl):
         """Get runtime configuration info.
@@ -269,13 +272,15 @@ class ApacheParser(object):
 
         ordered_matches = []
 
+        # TODO: Wildcards should be included in alphabetical order
+        # https://httpd.apache.org/docs/2.4/mod/core.html#include
         for match in matches:
             dir = self.aug.get(match).lower()
             if dir == "include" or dir == "includeoptional":
                 # start[6:] to strip off /files
                 ordered_matches.extend(self.find_dir(
                     directive, arg, self._get_include_path(
-                        strip_dir(start[6:]), self.aug.get(match + "/arg"))))
+                        self.get_arg(match + "/arg"))))
             else:
                 ordered_matches.extend(self.aug.match(match + arg_suffix))
 
@@ -334,7 +339,7 @@ class ApacheParser(object):
 
         return True
 
-    def _get_include_path(self, cur_dir, arg):
+    def _get_include_path(self, arg):
         """Converts an Apache Include directive into Augeas path.
 
         Converts an Apache Include directive argument into an Augeas
@@ -342,29 +347,12 @@ class ApacheParser(object):
 
         .. todo:: convert to use os.path.join()
 
-        :param str cur_dir: current working directory
-
         :param str arg: Argument of Include directive
 
         :returns: Augeas path string
         :rtype: str
 
         """
-        # Sanity check argument - maybe
-        # Question: what can the attacker do with control over this string
-        # Effect parse file... maybe exploit unknown errors in Augeas
-        # If the attacker can Include anything though... and this function
-        # only operates on Apache real config data... then the attacker has
-        # already won.
-        # Perhaps it is better to simply check the permissions on all
-        # included files?
-        # check_config to validate apache config doesn't work because it
-        # would create a race condition between the check and this input
-
-        # TODO: Maybe... although I am convinced we have lost if
-        # Apache files can't be trusted.  The augeas include path
-        # should be made to be exact.
-
         # Check to make sure only expected characters are used <- maybe remove
         # validChars = re.compile("[a-zA-Z0-9.*?_-/]*")
         # matchObj = validChars.match(arg)
@@ -374,11 +362,8 @@ class ApacheParser(object):
 
         # Standardize the include argument based on server root
         if not arg.startswith("/"):
-            arg = cur_dir + arg
+            # Normpath will condense ../
-        # conf/ is a special variable for ServerRoot in Apache
+            arg = os.path.normpath(os.path.join(self.root, arg))
-        elif arg.startswith("conf/"):
-            arg = self.root + arg[4:]
-        # TODO: Test if Apache allows ../ or ~/ for Includes
 
         # Attempts to add a transform to the file if one does not already exist
         self._parse_file(arg)
@@ -386,46 +371,38 @@ class ApacheParser(object):
         # Argument represents an fnmatch regular expression, convert it
         # Split up the path and convert each into an Augeas accepted regex
         # then reassemble
-        if "*" in arg or "?" in arg:
+        split_arg = arg.split("/")
-            split_arg = arg.split("/")
+        for idx, split in enumerate(split_arg):
-            for idx, split in enumerate(split_arg):
+            if any(char in fnmatch_chars for char in split):
-                # * and ? are the two special fnmatch characters
+                # Turn it into a augeas regex
-                if "*" in split or "?" in split:
+                # TODO: Can this instead be an augeas glob instead of regex
-                    # Turn it into a augeas regex
+                split_arg[idx] = ("* [label()=~regexp('%s')]" %
-                    # TODO: Can this instead be an augeas glob instead of regex
+                                  self.fnmatch_to_re(split))
-                    split_arg[idx] = ("* [label()=~regexp('%s')]" %
+        # Reassemble the argument
-                                      self.fnmatch_to_re(split))
+        arg = "/".join(split_arg)
-            # Reassemble the argument
-            arg = "/".join(split_arg)
 
         # If the include is a directory, just return the directory as a file
         if arg.endswith("/"):
-            return get_aug_path(arg[:len(arg)-1])
+            return get_aug_path(arg[:-1])
         return get_aug_path(arg)
 
     def fnmatch_to_re(self, clean_fn_match):  # pylint: disable=no-self-use
         """Method converts Apache's basic fnmatch to regular expression.
 
+        Assumption - Configs are assumed to be well-formed and only writable by
+        privileged users.
+
+        https://apr.apache.org/docs/apr/2.0/apr__fnmatch_8h_source.html
+        http://apache2.sourcearchive.com/documentation/2.2.16-6/apr__fnmatch_8h_source.html
+
         :param str clean_fn_match: Apache style filename match, similar to globs
 
         :returns: regex suitable for augeas
         :rtype: str
 
         """
-        # Checkout fnmatch.py in venv/local/lib/python2.7/fnmatch.py
+        # This strips off final /Z(?ms)
-        regex = ""
+        return fnmatch.translate(clean_fn_match)[:-7]
-        for letter in clean_fn_match:
-            if letter == ".":
-                regex = regex + r"\."
-            elif letter == "*":
-                regex = regex + ".*"
-            # According to apache.org ? shouldn't appear
-            # but in case it is valid...
-            elif letter == "?":
-                regex = regex + "."
-            else:
-                regex = regex + letter
-        return regex
 
     def _parse_file(self, filepath):
         """Parse file with Augeas