diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py index 7c70cf933..2e5ef9a45 100644 --- a/certbot-apache/certbot_apache/override_centos.py +++ b/certbot-apache/certbot_apache/override_centos.py @@ -12,7 +12,7 @@ from certbot import interfaces from certbot_apache import apache_util from certbot_apache import configurator from certbot_apache import parser -from certbot.errors import ConfigurationError +from certbot.errors import MisconfigurationError logger = logging.getLogger(__name__) @@ -82,26 +82,25 @@ class CentOSConfigurator(configurator.ApacheConfigurator): path_args = self.parser.get_all_args(noarg_path) if loadmod_args: if loadmod_args != path_args: - msg = ("Certbot encountered multiple different LoadModule directives " - "for LoadModule ssl_module. Please remove or comment out the " - "one(s) that are not in use.") - raise ConfigurationError(msg) + msg = ("Certbot encountered multiple LoadModule directives " + "for LoadModule ssl_module with differing library paths. " + "Please remove or comment out the one(s) that are not in " + "use, and run Certbot again.") + raise MisconfigurationError(msg) else: loadmod_args = path_args - loadmod_paths.append(noarg_path) - if self.parser.not_modssl_ifmodule(noarg_path): # pylint: disable=no-member - # Populate the list of known !mod_ssl.c IfModules - nodir_path = noarg_path.rpartition("/directive")[0] - correct_ifmods.append(nodir_path) - - if self.parser.loc["default"] in noarg_path: - if self.parser.not_modssl_ifmodule(noarg_path): #pylint: disable=no-member + if self.parser.loc["default"] in noarg_path: # LoadModule already in the main configuration file if "ifmodule/" in noarg_path or "ifmodule[1]" in noarg_path.lower(): # It's the first or only IfModule in the file return + # Populate the list of known !mod_ssl.c IfModules + nodir_path = noarg_path.rpartition("/directive")[0] + correct_ifmods.append(nodir_path) + else: + loadmod_paths.append(noarg_path) if not loadmod_args: # Do not try to enable mod_ssl @@ -109,9 +108,9 @@ class CentOSConfigurator(configurator.ApacheConfigurator): # Force creation as the directive wasn't found from the beginning of # httpd.conf - rootconf_ifmod = self.parser.get_ifmod( + rootconf_ifmod = self.parser.create_ifmod( parser.get_aug_path(self.parser.loc["default"]), - "!mod_ssl.c", beginning=True, force_create=True) + "!mod_ssl.c", beginning=True) # parser.get_ifmod returns a path postfixed with "/", remove that self.parser.add_dir(rootconf_ifmod[:-1], "LoadModule", loadmod_args) correct_ifmods.append(rootconf_ifmod[:-1]) @@ -121,19 +120,17 @@ class CentOSConfigurator(configurator.ApacheConfigurator): # configured like this already. for loadmod_path in loadmod_paths: nodir_path = loadmod_path.split("/directive")[0] - # Only create if !mod_ssl.c ifmodule not in path - if not self.parser.not_modssl_ifmodule(loadmod_path): # pylint: disable=no-member - # Remove the old LoadModule directive - self.aug.remove(loadmod_path) + # Remove the old LoadModule directive + self.aug.remove(loadmod_path) - # Create a new IfModule !mod_ssl.c if not already found on path - ssl_ifmod = self.parser.get_ifmod(nodir_path, "!mod_ssl.c", - beginning=True)[:-1] - if ssl_ifmod not in correct_ifmods: - self.parser.add_dir(ssl_ifmod, "LoadModule", loadmod_args) - correct_ifmods.append(ssl_ifmod) - self.save_notes += ("Wrapped pre-existing LoadModule ssl_module " - "inside of block.\n") + # Create a new IfModule !mod_ssl.c if not already found on path + ssl_ifmod = self.parser.get_ifmod(nodir_path, "!mod_ssl.c", + beginning=True)[:-1] + if ssl_ifmod not in correct_ifmods: + self.parser.add_dir(ssl_ifmod, "LoadModule", loadmod_args) + correct_ifmods.append(ssl_ifmod) + self.save_notes += ("Wrapped pre-existing LoadModule ssl_module " + "inside of block.\n") class CentOSParser(parser.ApacheParser): @@ -175,7 +172,7 @@ class CentOSParser(parser.ApacheParser): if parts[2].startswith("["): # Append the index from tail ifmod_path += parts[2].partition("/")[0] - # Get the origianl path trimmed to correct length + # Get the original path trimmed to correct length # This is required to preserve cases ifmod_real_path = path[0:len(ifmod_path)] if "!mod_ssl.c" in self.get_all_args(ifmod_real_path): diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py index b622379f5..085ede166 100644 --- a/certbot-apache/certbot_apache/parser.py +++ b/certbot-apache/certbot_apache/parser.py @@ -292,7 +292,7 @@ class ApacheParser(object): for i, arg in enumerate(args): self.aug.set("%s/arg[%d]" % (nvh_path, i + 1), arg) - def get_ifmod(self, aug_conf_path, mod, beginning=False, force_create=False): + def get_ifmod(self, aug_conf_path, mod, beginning=False): """Returns the path to and creates one if it doesn't exist. :param str aug_conf_path: Augeas configuration path @@ -303,21 +303,34 @@ class ApacheParser(object): """ if_mods = self.aug.match(("%s/IfModule/*[self::arg='%s']" % (aug_conf_path, mod))) - if len(if_mods) == 0 or force_create: - if beginning: - c_path_arg = "{}/IfModule[1]/arg".format(aug_conf_path) - # Insert IfModule before the first directive - self.aug.insert("{}/directive[1]".format(aug_conf_path), - "IfModule", True) - else: - c_path = "{}/IfModule[last() + 1]".format(aug_conf_path) - c_path_arg = "{}/IfModule[last()]/arg".format(aug_conf_path) - self.aug.set(c_path, "") - self.aug.set(c_path_arg, mod) - if_mods = self.aug.match(("%s/IfModule/*[self::arg='%s']" % - (aug_conf_path, mod))) + if not if_mods: + return self.create_ifmod(aug_conf_path, mod, beginning) + # Strip off "arg" at end of first ifmod path - return if_mods[0][:len(if_mods[0]) - 3] + return if_mods[0].rpartition("arg")[0] + + def create_ifmod(self, aug_conf_path, mod, beginning=False): + """Creates a new and returs it path. + + :param str aug_conf_path: Augeas configuration path + :param str mod: module ie. mod_ssl.c + :param bool beginning: If the IfModule should be created to the beginning + of augeas path DOM tree. + + """ + if beginning: + c_path_arg = "{}/IfModule[1]/arg".format(aug_conf_path) + # Insert IfModule before the first directive + self.aug.insert("{}/directive[1]".format(aug_conf_path), + "IfModule", True) + retpath = "{}/IfModule[1]/".format(aug_conf_path) + else: + c_path = "{}/IfModule[last() + 1]".format(aug_conf_path) + c_path_arg = "{}/IfModule[last()]/arg".format(aug_conf_path) + self.aug.set(c_path, "") + retpath = "{}/IfModule[last()]/".format(aug_conf_path) + self.aug.set(c_path_arg, mod) + return retpath def add_dir(self, aug_conf_path, directive, args): """Appends directive to the end fo the file given by aug_conf_path. diff --git a/certbot-apache/certbot_apache/tests/centos6_test.py b/certbot-apache/certbot_apache/tests/centos6_test.py index e8976b153..ea8a85ed7 100644 --- a/certbot-apache/certbot_apache/tests/centos6_test.py +++ b/certbot-apache/certbot_apache/tests/centos6_test.py @@ -6,7 +6,7 @@ from certbot_apache import obj from certbot_apache import override_centos from certbot_apache import parser from certbot_apache.tests import util -from certbot.errors import ConfigurationError +from certbot.errors import MisconfigurationError def get_vh_truth(temp_dir, config_name): """Return the ground truth for the specified directory.""" @@ -179,7 +179,7 @@ class CentOS6Tests(util.ApacheTest): pre_matches = self.config.parser.find_dir("LoadModule", "ssl_module", exclude=False) - self.assertRaises(ConfigurationError, self.config.deploy_cert, + self.assertRaises(MisconfigurationError, self.config.deploy_cert, "random.demo", "example/cert.pem", "example/key.pem", "example/cert_chain.pem", "example/fullchain.pem")