diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 69b635bf0..af6dad395 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1,4 +1,5 @@ """Apache Configuration based off of Augeas Configurator.""" +# pylint: disable=too-many-lines import logging import os import re @@ -16,8 +17,6 @@ from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util -from letsencrypt.plugins import common - from letsencrypt_apache import augeas_configurator from letsencrypt_apache import constants from letsencrypt_apache import display_ops @@ -168,17 +167,21 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ vhost = self.choose_vhost(domain) + # This is done first so that ssl module is enabled and cert_path, + # cert_key... can all be parsed appropriately + self.prepare_server_https("443") + path = {} - path["cert_path"] = self.parser.find_dir(parser.case_i( - "SSLCertificateFile"), None, vhost.path) - path["cert_key"] = self.parser.find_dir(parser.case_i( - "SSLCertificateKeyFile"), None, vhost.path) + path["cert_path"] = self.parser.find_dir( + "SSLCertificateFile", None, vhost.path) + path["cert_key"] = self.parser.find_dir( + "SSLCertificateKeyFile", None, vhost.path) # Only include if a certificate chain is specified if chain_path is not None: path["chain_path"] = self.parser.find_dir( - parser.case_i("SSLCertificateChainFile"), None, vhost.path) + "SSLCertificateChainFile", None, vhost.path) if not path["cert_path"] or not path["cert_key"]: # Throw some can't find all of the directives error" @@ -186,7 +189,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "Cannot find a cert or key directive in %s. " "VirtualHost was not modified", vhost.path) # Presumably break here so that the virtualhost is not modified - return False + raise errors.PluginError( + "Unable to find cert and/or key directives") logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) @@ -235,7 +239,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): vhost = self._find_best_vhost(target_name) if vhost is not None: if not vhost.ssl: - vhost = self.make_vhost_ssl(non_ssl_vhost) + vhost = self.make_vhost_ssl(vhost) self.assoc[target_name] = vhost return vhost @@ -311,13 +315,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for vhost in self.vhosts: all_names.update(vhost.names) for addr in vhost.addrs: - name = get_name_from_ip(addr) + name = self.get_name_from_ip(addr) if name: all_names.add(name) return all_names - def get_name_from_ip(self, addr): + def get_name_from_ip(self, addr): # pylint: disable=no-self-use """Returns a reverse dns name if available. :param addr: IP Address @@ -328,7 +332,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # If it isn't a private IP, do a reverse DNS lookup - if not private_ips_regex.match(addr.get_addr()): + if not ApacheConfigurator.private_ips_regex.match(addr.get_addr()): try: socket.inet_aton(addr.get_addr()) return socket.gethostbyaddr(addr.get_addr())[0] @@ -371,12 +375,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): addrs.add(obj.Addr.fromstring(self.parser.get_arg(arg))) is_ssl = False - if self.parser.find_dir( - parser.case_i("SSLEngine"), parser.case_i("on"), path): + if self.parser.find_dir("SSLEngine", "on", path, exclude=False): is_ssl = True filename = get_file_path(path) is_enabled = self.is_site_enabled(filename) + vhost = obj.VirtualHost(filename, path, addrs, is_ssl, is_enabled) self._add_servernames(vhost) return vhost @@ -394,6 +398,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): paths = self.aug.match( ("/files%s/sites-available//*[label()=~regexp('%s')]" % (self.parser.root, parser.case_i("VirtualHost")))) + vhs = [] for path in paths: @@ -402,7 +407,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return vhs def is_name_vhost(self, target_addr): - r"""Returns if vhost is a name based vhost + """Returns if vhost is a name based vhost NameVirtualHost was deprecated in Apache 2.4 as all VirtualHosts are now NameVirtualHosts. If version is earlier than 2.4, check if addr @@ -421,9 +426,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # search for NameVirtualHost directive for ip_addr # note ip_addr can be FQDN although Apache does not recommend it return (self.version >= (2, 4) or - self.parser.find_dir( - parser.case_i("NameVirtualHost"), - parser.case_i(str(target_addr)))) + self.parser.find_dir("NameVirtualHost", str(target_addr))) def add_name_vhost(self, addr): """Adds NameVirtualHost directive for given address. @@ -432,14 +435,17 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :type addr: :class:~letsencrypt_apache.obj.Addr """ - path = self.parser.add_dir_to_ifmodssl( - parser.get_aug_path( - self.parser.loc["name"]), "NameVirtualHost", [str(addr)]) + loc = parser.get_aug_path(self.parser.loc["name"]) + if addr.get_port == "443": + path = self.parser.add_dir_to_ifmodssl( + loc, "NameVirtualHost", [str(addr)]) + else: + path = self.parser.add_dir(loc, "NameVirtualHost", [str(addr)]) self.save_notes += "Setting %s to be NameBasedVirtualHost\n" % addr self.save_notes += "\tDirective added to %s\n" % path - def _prepare_server_https(self, port): + def prepare_server_https(self, port): """Prepare the server for HTTPS. Make sure that the ssl_module is loaded and that the server @@ -454,9 +460,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Check for Listen # Note: This could be made to also look for ip:443 combo - if not self.parser.find_dir(parser.case_i("Listen"), port): - logger.debug("No Listen {0} directive found. Setting the " - "Apache Server to Listen on port {0}".format(port)) + 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] @@ -540,7 +546,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): vh_p = vh_p[0] # Update Addresses - ssl_addrs = self._update_ssl_vhosts_addrs(vh_p) + self._update_ssl_vhosts_addrs(vh_p) # Add directives self._add_dummy_ssl_directives(vh_p) @@ -552,7 +558,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # We know the length is one because of the assertion above # Create the Vhost object - ssl_vhost = self._create_vhost(vh_p[0]) + ssl_vhost = self._create_vhost(vh_p) self.vhosts.append(ssl_vhost) @@ -632,7 +638,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # See if the exact address appears in any other vhost for addr in vhost.addrs: - for test_vhost in self.vhosts: + for test_vh in self.vhosts: if (vhost.filep != test_vh.filep and addr in test_vh.addrs and not self.is_name_vhost(addr)): self.add_name_vhost(addr) @@ -746,10 +752,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: bool, int """ - rewrite_path = self.parser.find_dir( - parser.case_i("RewriteRule"), None, vhost.path) - redirect_path = self.parser.find_dir( - parser.case_i("Redirect"), None, vhost.path) + rewrite_path = self.parser.find_dir("RewriteRule", None, vhost.path) + redirect_path = self.parser.find_dir("Redirect", None, vhost.path) if redirect_path: # "Existing Redirect directive for virtualhost" @@ -942,9 +946,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for vhost in self.vhosts: if vhost.ssl: cert_path = self.parser.find_dir( - parser.case_i("SSLCertificateFile"), None, vhost.path) + "SSLCertificateFile", None, vhost.path) key_path = self.parser.find_dir( - parser.case_i("SSLCertificateKeyFile"), None, vhost.path) + "SSLCertificateKeyFile", None, vhost.path) # Can be removed once find directive can return ordered results if len(cert_path) != 1 or len(key_path) != 1: @@ -981,7 +985,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """Enables an available site, Apache restart required. .. todo:: This function should number subdomains before the domain vhost - .. todo:: Make sure link is not broken... :param vhost: vhost to enable @@ -996,9 +999,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if vhost.ssl: # TODO: Make this based on addresses - self._prepare_server_https("443") + self.prepare_server_https("443") if self.save_notes: - self.save("Enabled TLS for Apache") + self.save() if "/sites-available/" in vhost.filep: enabled_path = ("%s/sites-enabled/%s" % @@ -1034,20 +1037,21 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # 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.update_runtime_variables(self.conf("ctl")) self.parser.modules.add(mod_name + "_module") - self.parser.modules.add("mod_" + mod_name) + self.parser.modules.add("mod_" + mod_name + ".c") 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"]) + self._enable_mod_debian_files( + ["ssl.conf", "ssl.load"], "ssl_module") elif mod_name == "rewrite": - self._enable_mod_debian_files(["rewrite.load"]) + self._enable_mod_debian_files(["rewrite.load"], "rewrite_module") else: - raise NotImplemented + raise NotImplementedError def _enable_mod_debian_files(self, filenames, mod_name): """Move over all required files into mods-enabled.""" @@ -1058,11 +1062,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for filename in filenames: if not os.path.isfile(os.path.join(mods_available, filename)): raise errors.MisconfigurationError( - "Unable to enable module. Required files missing from " - "mods-available. %s" % str(filenames)) + "Unable to enable module. Required files missing from " + "mods-available. %s" % str(filenames)) # Register and symlink files - for filename in files: + for filename in filenames: enabled_path = os.path.join(mods_enabled, filename) if os.path.isfile(enabled_path): logger.debug( @@ -1131,6 +1135,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise errors.PluginError("Unable to run apache2ctl") if proc.returncode != 0: + print proc.returncode # Enter recovery routine... logger.error("Apache Configtest failed\n%s\n%s", stdout, stderr) raise errors.MisconfigurationError( diff --git a/letsencrypt-apache/letsencrypt_apache/dvsni.py b/letsencrypt-apache/letsencrypt_apache/dvsni.py index ac0a3039a..e10c0cbf9 100644 --- a/letsencrypt-apache/letsencrypt_apache/dvsni.py +++ b/letsencrypt-apache/letsencrypt_apache/dvsni.py @@ -45,7 +45,7 @@ class ApacheDvsni(common.Dvsni): """ - def __init__(self): + def __init__(self, *args, **kwargs): super(ApacheDvsni, self).__init__(*args, **kwargs) self.challenge_conf = os.path.join( @@ -60,6 +60,10 @@ class ApacheDvsni(common.Dvsni): # About to make temporary changes to the config self.configurator.save() + # Prepare the server for HTTPS + self.configurator.prepare_server_https( + str(self.configurator.config.dvsni_port)) + responses = [] # Create all of the challenge certs @@ -68,13 +72,8 @@ class ApacheDvsni(common.Dvsni): # Setup the configuration dvsni_addrs = self._mod_config() - self.configurator.make_addrs_sni_ready(dvsni_addrs) - # Prepare the server for HTTPS - self.configurator._prepare_https_server( - str(self.configurator.config.dvsni_port)) - # Save reversible changes self.configurator.save("SNI Challenge", True) @@ -96,7 +95,7 @@ class ApacheDvsni(common.Dvsni): achall_addrs = self.get_dvsni_addrs(achall) dvsni_addrs.update(achall_addrs) - config_text += self._get_config_text(self.achalls, achall_addrs) + config_text += self._get_config_text(achall, achall_addrs) config_text += "\n" @@ -114,7 +113,6 @@ class ApacheDvsni(common.Dvsni): vhost = self.configurator.choose_vhost(achall.domain) # TODO: Checkout _default_ rules. - # TODO: Need to separate out test mode and normal mode for DVSNI addrs dvsni_addrs = set() default_addr = obj.Addr(("*", self.configurator.config.dvsni_port)) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index 956b6999f..ae84f7d26 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -2,7 +2,7 @@ from letsencrypt.plugins import common class Addr(common.Addr): - + """Represents an Apache address.""" def __eq__(self, other): """This is defined as equalivalent within Apache. @@ -16,7 +16,8 @@ class Addr(common.Addr): return False def is_wildcard(self): - return tup[1] == "*" or not tup[1] + """Returns if address has a wildcard port.""" + return self.tup[1] == "*" or not self.tup[1] def get_sni_addr(self, port): """Returns the least specific address that resolves on the port. diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index ffbacc066..a0bc6fd12 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -1,5 +1,4 @@ """ApacheParser is a member object of the ApacheConfigurator class.""" -import collections import fnmatch import itertools import logging @@ -13,10 +12,6 @@ from letsencrypt import errors logger = logging.getLogger(__name__) -arg_var_interpreter = re.compile(r"\$\{[^ \}]*}") -fnmatch_chars = set(["*", "?", "\\", "[", "]"]) - - class ApacheParser(object): """Class handles the fine details of parsing the Apache Configuration. @@ -26,6 +21,9 @@ class ApacheParser(object): directory. Without trailing slash. """ + arg_var_interpreter = re.compile(r"\$\{[^ \}]*}") + fnmatch_chars = set(["*", "?", "\\", "[", "]"]) + def __init__(self, aug, root, ssl_options, ctl): # This uses the binary, so it can be done first. # https://httpd.apache.org/docs/2.4/mod/core.html#define @@ -59,7 +57,7 @@ class ApacheParser(object): the iteration issue. Else... parse and enable mods at same time. """ - matches = self.find_dir(case_i("LoadModule")) + matches = self.find_dir("LoadModule") iterator = iter(matches) # Make sure prev_size != cur_size for do: while: iteration @@ -101,7 +99,7 @@ class ApacheParser(object): self.variables = variables - def _get_runtime_cfg(self, ctl): + def _get_runtime_cfg(self, ctl): # pylint: disable=no-self-use """Get runtime configuration info. :returns: stdout from DUMP_RUN_CFG @@ -116,8 +114,7 @@ class ApacheParser(object): except (OSError, ValueError): logger.error( - "Error accessing {0} for runtime parameters!{1}".format( - ctl, os.linesep)) + "Error accessing %s for runtime parameters!%s", ctl, os.linesep) raise errors.MisconfigurationError( "Error accessing loaded Apache parameters: %s", ctl) # Small errors that do not impede @@ -129,7 +126,7 @@ class ApacheParser(object): return stdout - def _filter_args_num(self, matches, args): + def _filter_args_num(self, matches, args): # pylint: disable=no-self-use """Filter out directives with specific number of arguments. This function makes the assumption that all related arguments are given @@ -223,7 +220,7 @@ class ApacheParser(object): else: self.aug.set(aug_conf_path + "/directive[last()]/arg", args) - def find_dir(self, directive, arg=None, start=None): + def find_dir(self, directive, arg=None, start=None, exclude=True): """Finds directive in the configuration. Recursively searches through config files to find directives @@ -241,12 +238,13 @@ class ApacheParser(object): compatibility. :param str directive: Directive to look for - :param arg: Specific value directive must have, None if all should be considered :type arg: str or None :param str start: Beginning Augeas path to begin looking + :param bool exclude: Whether or not to exclude directives based on + variables and enabled modules """ # Cannot place member variable in the definition of the function so... @@ -265,32 +263,33 @@ class ApacheParser(object): # includes = self.aug.match(start + # "//* [self::directive='Include']/* [label()='arg']") - regex = "(%s)|(%s)|(%s)" % (directive, + regex = "(%s)|(%s)|(%s)" % (case_i(directive), case_i("Include"), case_i("IncludeOptional")) matches = self.aug.match( "%s//*[self::directive=~regexp('%s')]" % (start, regex)) - matches = self._exclude_dirs(matches) - + if exclude: + matches = self._exclude_dirs(matches) if arg is None: arg_suffix = "/arg" else: - arg_suffix = "/*[self::arg=~regexp('%s')]" % arg + arg_suffix = "/*[self::arg=~regexp('%s')]" % case_i(arg) 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": + 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( self.get_arg(match + "/arg")))) - else: + # This additionally allows Include + if dir_ == directive.lower(): ordered_matches.extend(self.aug.match(match + arg_suffix)) return ordered_matches @@ -302,11 +301,14 @@ class ApacheParser(object): """ value = self.aug.get(match) - variables = arg_var_interpreter.findall(value) + variables = ApacheParser.arg_var_interpreter.findall(value) for var in variables: # Strip off ${ and } - value = value.replace(var, self.variables[var[2:-1]]) + try: + value = value.replace(var, self.variables[var[2:-1]]) + except KeyError: + raise errors.PluginError("Error Parsing variable: %s" % var) return value @@ -317,14 +319,14 @@ class ApacheParser(object): valid_matches = [] for match in matches: - for filter in filters: - if not self._pass_filter(match, filter): + for filter_ in filters: + if not self._pass_filter(match, filter_): break else: valid_matches.append(match) return valid_matches - def _pass_filter(self, match, filter): + def _pass_filter(self, match, filter_): """Determine if directive passes a filter. :param str match: Augeas path @@ -333,7 +335,7 @@ class ApacheParser(object): """ match_l = match.lower() - last_match_idx = match_l.find(filter[0]) + last_match_idx = match_l.find(filter_[0]) while last_match_idx != -1: # Check args @@ -341,10 +343,10 @@ class ApacheParser(object): expression = self.aug.get(match[:end_of_if] + "/arg") expected = not expression.startswith("!") - if expected != expression in filter[1]: + if expected != (expression in filter_[1]): return False - last_match_idx = match_l.find(filter[0], end_of_if) + last_match_idx = match_l.find(filter_[0], end_of_if) return True @@ -382,7 +384,7 @@ class ApacheParser(object): # then reassemble split_arg = arg.split("/") for idx, split in enumerate(split_arg): - if any(char in fnmatch_chars for char in split): + if any(char in ApacheParser.fnmatch_chars for char in split): # Turn it into a augeas regex # TODO: Can this instead be an augeas glob instead of regex split_arg[idx] = ("* [label()=~regexp('%s')]" % @@ -529,8 +531,7 @@ class ApacheParser(object): # in hierarchy via direct include # httpd.conf was very common as a user file in Apache 2.2 if (os.path.isfile(os.path.join(self.root, "httpd.conf")) and - self.find_dir( - case_i("Include"), case_i("httpd.conf"), root)): + self.find_dir("Include", "httpd.conf", root)): return os.path.join(self.root, "httpd.conf") else: return os.path.join(self.root, "apache2.conf") diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index c156ebe66..111e82a2d 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -1,6 +1,5 @@ """Test for letsencrypt_apache.configurator.""" import os -import re import shutil import unittest @@ -12,18 +11,16 @@ from letsencrypt import achallenges from letsencrypt import errors from letsencrypt import le_util -from letsencrypt.plugins import common - from letsencrypt.tests import acme_util from letsencrypt_apache import configurator -from letsencrypt_apache import parser +from letsencrypt_apache import obj from letsencrypt_apache.tests import util class TwoVhost80Test(util.ApacheTest): - """Test two standard well configured HTTP vhosts.""" + """Test two standard well-configured HTTP vhosts.""" def setUp(self): super(TwoVhost80Test, self).setUp() @@ -48,7 +45,7 @@ class TwoVhost80Test(util.ApacheTest): """Make sure all vhosts are being properly found. .. note:: If test fails, only finding 1 Vhost... it is likely that - it is a problem with is_enabled. + it is a problem with is_enabled. If finding only 3, likely is_ssl """ vhs = self.config.get_virtual_hosts() @@ -60,6 +57,8 @@ class TwoVhost80Test(util.ApacheTest): if vhost == truth: found += 1 break + else: + raise Exception("Missed: %s" % vhost) self.assertEqual(found, 4) @@ -77,7 +76,35 @@ class TwoVhost80Test(util.ApacheTest): self.assertTrue(self.config.is_site_enabled(self.vh_truth[2].filep)) self.assertTrue(self.config.is_site_enabled(self.vh_truth[3].filep)) - def test_deploy_cert(self): + @mock.patch("letsencrypt_apache.parser.subprocess.Popen") + def test_enable_mod(self, mock_popen): + mock_popen().communicate.return_value = ("Define: DUMP_RUN_CFG", "") + mock_popen().returncode = 0 + + self.config.enable_mod("ssl") + for filename in ["ssl.conf", "ssl.load"]: + self.assertTrue( + os.path.isfile(os.path.join( + self.config.conf("server-root"), "mods-enabled", filename))) + + self.assertTrue("ssl_module" in self.config.parser.modules) + self.assertTrue("mod_ssl.c" in self.config.parser.modules) + + @mock.patch("letsencrypt_apache.parser.subprocess.Popen") + def test_enable_site(self, mock_popen): + mock_popen().returncode = 0 + mock_popen().communicate.return_value = ("Define: DUMP_RUN_CFG", "") + + # Default 443 vhost + self.assertFalse(self.vh_truth[1].enabled) + self.config.enable_site(self.vh_truth[1]) + self.assertTrue(self.vh_truth[1].enabled) + + @mock.patch("letsencrypt_apache.parser.subprocess.Popen") + def test_deploy_cert(self, mock_popen): + mock_popen().returncode = 0 + mock_popen().communicate.return_value = ("Define: DUMP_RUN_CFG", "") + # Get the default 443 vhost self.config.assoc["random.demo"] = self.vh_truth[1] self.config.deploy_cert( @@ -85,15 +112,17 @@ class TwoVhost80Test(util.ApacheTest): "example/cert.pem", "example/key.pem", "example/cert_chain.pem") self.config.save() + # Verify ssl_module was enabled. + self.assertTrue(self.vh_truth[1].enabled) + self.assertTrue("ssl_module" in self.config.parser.modules) + loc_cert = self.config.parser.find_dir( - parser.case_i("sslcertificatefile"), - re.escape("example/cert.pem"), self.vh_truth[1].path) + "sslcertificatefile", "example/cert.pem", self.vh_truth[1].path) loc_key = self.config.parser.find_dir( - parser.case_i("sslcertificateKeyfile"), - re.escape("example/key.pem"), self.vh_truth[1].path) + "sslcertificateKeyfile", "example/key.pem", self.vh_truth[1].path) loc_chain = self.config.parser.find_dir( - parser.case_i("SSLCertificateChainFile"), - re.escape("example/cert_chain.pem"), self.vh_truth[1].path) + "SSLCertificateChainFile", "example/cert_chain.pem", + self.vh_truth[1].path) # Verify one directive was found in the correct file self.assertEqual(len(loc_cert), 1) @@ -109,15 +138,15 @@ class TwoVhost80Test(util.ApacheTest): self.vh_truth[1].filep) def test_is_name_vhost(self): - addr = common.Addr.fromstring("*:80") + addr = obj.Addr.fromstring("*:80") self.assertTrue(self.config.is_name_vhost(addr)) self.config.version = (2, 2) self.assertFalse(self.config.is_name_vhost(addr)) def test_add_name_vhost(self): - self.config.add_name_vhost("*:443") + self.config.add_name_vhost(obj.Addr.fromstring("*:443")) self.assertTrue(self.config.parser.find_dir( - "NameVirtualHost", re.escape("*:443"))) + "NameVirtualHost", "*:443")) def test_make_vhost_ssl(self): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) @@ -130,17 +159,15 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(ssl_vhost.path, "/files" + ssl_vhost.filep + "/IfModule/VirtualHost") self.assertEqual(len(ssl_vhost.addrs), 1) - self.assertEqual(set([common.Addr.fromstring("*:443")]), ssl_vhost.addrs) + self.assertEqual(set([obj.Addr.fromstring("*:443")]), ssl_vhost.addrs) self.assertEqual(ssl_vhost.names, set(["encryption-example.demo"])) self.assertTrue(ssl_vhost.ssl) self.assertFalse(ssl_vhost.enabled) self.assertTrue(self.config.parser.find_dir( - "SSLCertificateFile", None, ssl_vhost.path)) + "SSLCertificateFile", None, ssl_vhost.path, False)) self.assertTrue(self.config.parser.find_dir( - "SSLCertificateKeyFile", None, ssl_vhost.path)) - self.assertTrue(self.config.parser.find_dir( - "Include", self.ssl_options, ssl_vhost.path)) + "SSLCertificateKeyFile", None, ssl_vhost.path, False)) self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]), self.config.is_name_vhost(ssl_vhost)) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py b/letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py index 933656e94..b6f9bc5e0 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py @@ -6,9 +6,9 @@ import mock from acme import challenges -from letsencrypt.plugins import common from letsencrypt.plugins import common_test +from letsencrypt_apache import obj from letsencrypt_apache.tests import util @@ -20,11 +20,9 @@ class DvsniPerformTest(util.ApacheTest): def setUp(self): super(DvsniPerformTest, self).setUp() - with mock.patch("letsencrypt_apache.configurator.ApacheConfigurator." - "mod_loaded") as mock_load: - mock_load.return_value = True - config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir) + config = util.get_apache_configurator( + self.config_path, self.config_dir, self.work_dir) + config.config.dvsni_port = 443 from letsencrypt_apache import dvsni self.sni = dvsni.ApacheDvsni(config) @@ -38,7 +36,11 @@ class DvsniPerformTest(util.ApacheTest): resp = self.sni.perform() self.assertEqual(len(resp), 0) - def test_perform1(self): + @mock.patch("letsencrypt_apache.parser.subprocess.Popen") + def test_perform1(self, mock_popen): + mock_popen().communicate.return_value = ("Define: DUMP_RUN_CFG", "") + mock_popen().returncode = 0 + achall = self.achalls[0] self.sni.add_chall(achall) mock_setup_cert = mock.MagicMock( @@ -53,12 +55,14 @@ class DvsniPerformTest(util.ApacheTest): # Check to make sure challenge config path is included in apache config. self.assertEqual( len(self.sni.configurator.parser.find_dir( - "Include", self.sni.challenge_conf)), - 1) + "Include", self.sni.challenge_conf)), 1) self.assertEqual(len(responses), 1) self.assertEqual(responses[0].s, "randomS1") def test_perform2(self): + # Avoid load module + self.sni.configurator.parser.modules.add("ssl_module") + for achall in self.achalls: self.sni.add_chall(achall) @@ -89,13 +93,8 @@ class DvsniPerformTest(util.ApacheTest): def test_mod_config(self): for achall in self.achalls: self.sni.add_chall(achall) - v_addr1 = [common.Addr(("1.2.3.4", "443")), - common.Addr(("5.6.7.8", "443"))] - v_addr2 = [common.Addr(("127.0.0.1", "443"))] - ll_addr = [] - ll_addr.append(v_addr1) - ll_addr.append(v_addr2) - self.sni._mod_config(ll_addr) # pylint: disable=protected-access + + self.sni._mod_config() # pylint: disable=protected-access self.sni.configurator.save() self.sni.configurator.parser.find_dir( @@ -109,15 +108,10 @@ class DvsniPerformTest(util.ApacheTest): vhs.append(self.sni.configurator._create_vhost(match)) self.assertEqual(len(vhs), 2) for vhost in vhs: - if vhost.addrs == set(v_addr1): - self.assertEqual( - vhost.names, - set([self.achalls[0].nonce_domain])) - else: - self.assertEqual(vhost.addrs, set(v_addr2)) - self.assertEqual( - vhost.names, - set([self.achalls[1].nonce_domain])) + self.assertEqual(vhost.addrs, set([obj.Addr.fromstring("*:443")])) + self.assertTrue( + vhost.names == set([self.achalls[0].nonce_domain]) or + vhost.names == set([self.achalls[1].nonce_domain])) if __name__ == "__main__": diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index 05224b56d..3a74055ad 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -54,11 +54,11 @@ class ApacheParserTest(util.ApacheTest): self.assertTrue(matches) def test_find_dir(self): - from letsencrypt_apache.parser import case_i - test = self.parser.find_dir(case_i("Listen"), "443") + test = self.parser.find_dir("Listen", "80") # This will only look in enabled hosts - test2 = self.parser.find_dir(case_i("documentroot")) - self.assertEqual(len(test), 2) + test2 = self.parser.find_dir("documentroot") + + self.assertEqual(len(test), 1) self.assertEqual(len(test2), 3) def test_add_dir(self): @@ -80,6 +80,9 @@ class ApacheParserTest(util.ApacheTest): """ from letsencrypt_apache.parser import get_aug_path + # This makes sure that find_dir will work + self.parser.modules.add("mod_ssl.c") + self.parser.add_dir_to_ifmodssl( get_aug_path(self.parser.loc["default"]), "FakeDirective", ["123"]) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 55349c366..ae2f25b9f 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -35,8 +35,7 @@ class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods def get_apache_configurator( - config_path, config_dir, work_dir, version=(2, 4, 7), - conf=mock.MagicMock()): + config_path, config_dir, work_dir, version=(2, 4, 7), conf=None): """Create an Apache Configurator with the specified options. :param conf: Function that returns binary paths. self.conf in Configurator @@ -44,25 +43,30 @@ def get_apache_configurator( """ backups = os.path.join(work_dir, "backups") - with mock.patch("letsencrypt_apache.configurator.subprocess.Popen") as mock_popen: - # This indicates config_test passes - mock_popen().communicate.return_value = ("Fine output", "No problems") - mock_popen.returncode.return_value = 0 - # mock_popen().communicate.return_value = ("ssl_module", "") - config = configurator.ApacheConfigurator( - config=mock.MagicMock( - apache_server_root=config_path, - apache_le_vhost_ext=constants.CLI_DEFAULTS["le_vhost_ext"], - backup_dir=backups, - config_dir=config_dir, - temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), - in_progress_dir=os.path.join(backups, "IN_PROGRESS"), - work_dir=work_dir), - name="apache", - version=version) - config.conf = conf + with mock.patch("letsencrypt_apache.configurator." + "subprocess.Popen") as mock_popen: + with mock.patch("letsencrypt_apache.parser.ApacheParser." + "update_runtime_variables"): + # This indicates config_test passes + mock_popen().communicate.return_value = ("Fine output", "No problems") + mock_popen().returncode = 0 - config.prepare() + config = configurator.ApacheConfigurator( + config=mock.MagicMock( + apache_server_root=config_path, + apache_le_vhost_ext=constants.CLI_DEFAULTS["le_vhost_ext"], + backup_dir=backups, + config_dir=config_dir, + temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), + in_progress_dir=os.path.join(backups, "IN_PROGRESS"), + work_dir=work_dir), + name="apache", + version=version) + # This allows testing scripts to set it a bit more quickly + if conf is not None: + config.conf = conf + + config.prepare() return config @@ -77,21 +81,21 @@ def get_vh_truth(temp_dir, config_name): obj.VirtualHost( os.path.join(prefix, "encryption-example.conf"), os.path.join(aug_pre, "encryption-example.conf/VirtualHost"), - set([common.Addr.fromstring("*:80")]), + set([obj.Addr.fromstring("*:80")]), False, True, set(["encryption-example.demo"])), obj.VirtualHost( os.path.join(prefix, "default-ssl.conf"), os.path.join(aug_pre, "default-ssl.conf/IfModule/VirtualHost"), - set([common.Addr.fromstring("_default_:443")]), True, False), + set([obj.Addr.fromstring("_default_:443")]), True, False), obj.VirtualHost( os.path.join(prefix, "000-default.conf"), os.path.join(aug_pre, "000-default.conf/VirtualHost"), - set([common.Addr.fromstring("*:80")]), False, True, + set([obj.Addr.fromstring("*:80")]), False, True, set(["ip-172-30-0-17"])), obj.VirtualHost( os.path.join(prefix, "letsencrypt.conf"), os.path.join(aug_pre, "letsencrypt.conf/VirtualHost"), - set([common.Addr.fromstring("*:80")]), False, True, + set([obj.Addr.fromstring("*:80")]), False, True, set(["letsencrypt.demo"])), ] return vh_truth