diff --git a/certbot-apache/certbot_apache/augeas_configurator.py b/certbot-apache/certbot_apache/augeas_configurator.py index 3735284ef..e053d0468 100644 --- a/certbot-apache/certbot_apache/augeas_configurator.py +++ b/certbot-apache/certbot_apache/augeas_configurator.py @@ -47,7 +47,9 @@ class AugeasConfigurator(common.Plugin): loadpath=constants.AUGEAS_LENS_DIR, # Do not save backup (we do it ourselves), do not load # anything by default - flags=(augeas.Augeas.NONE | augeas.Augeas.NO_MODL_AUTOLOAD)) + flags=(augeas.Augeas.NONE | + augeas.Augeas.NO_MODL_AUTOLOAD | + augeas.Augeas.ENABLE_SPAN)) self.recovery_routine() def check_parsing_errors(self, lens): diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index b82506dd2..3dd9aae5f 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -502,6 +502,32 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return "" + def _get_vhost_names(self, path): + """Helper method for getting the ServerName and + ServerAlias values from vhost in path + + :param path: Path to read ServerName and ServerAliases from + + :returns: Tuple including ServerName and `list` of ServerAlias strings + """ + + servername_match = self.parser.find_dir( + "ServerName", None, start=path, exclude=False) + serveralias_match = self.parser.find_dir( + "ServerAlias", None, start=path, exclude=False) + + serveraliases = [] + for alias in serveralias_match: + serveralias = self.parser.get_arg(alias) + serveraliases.append(serveralias) + + servername = None + if servername_match: + # Get last ServerName as each overwrites the previous + servername = self.parser.get_arg(servername_match[-1]) + + return (servername, serveraliases) + def _add_servernames(self, host): """Helper function for get_virtual_hosts(). @@ -509,22 +535,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :type host: :class:`~certbot_apache.obj.VirtualHost` """ - # Take the final ServerName as each overrides the previous - servername_match = self.parser.find_dir( - "ServerName", None, start=host.path, exclude=False) - serveralias_match = self.parser.find_dir( - "ServerAlias", None, start=host.path, exclude=False) - for alias in serveralias_match: - serveralias = self.parser.get_arg(alias) - if not host.modmacro: - host.aliases.add(serveralias) + servername, serveraliases = self._get_vhost_names(host.path) - if servername_match: - # Get last ServerName as each overwrites the previous - servername = self.parser.get_arg(servername_match[-1]) + for alias in serveraliases: if not host.modmacro: - host.name = servername + host.aliases.add(alias) + + if not host.modmacro: + host.name = servername def _create_vhost(self, path): """Used by get_virtual_hosts to create vhost objects @@ -581,30 +600,46 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Search base config, and all included paths for VirtualHosts + file_paths = {} + internal_paths = defaultdict(set) vhs = [] - vhost_paths = {} for vhost_path in self.parser.parser_paths.keys(): paths = self.aug.match( ("/files%s//*[label()=~regexp('%s')]" % (vhost_path, parser.case_i("VirtualHost")))) paths = [path for path in paths if - os.path.basename(path.lower()) == "virtualhost"] + "virtualhost" in os.path.basename(path).lower()] for path in paths: new_vhost = self._create_vhost(path) if not new_vhost: continue + internal_path = get_internal_aug_path(new_vhost.path) realpath = os.path.realpath(new_vhost.filep) - if realpath not in vhost_paths.keys(): + if realpath not in file_paths: + file_paths[realpath] = new_vhost.filep + internal_paths[realpath].add(internal_path) vhs.append(new_vhost) - vhost_paths[realpath] = new_vhost.filep - elif realpath == new_vhost.filep: + elif (realpath == new_vhost.filep and + realpath != file_paths[realpath]): # Prefer "real" vhost paths instead of symlinked ones # ex: sites-enabled/vh.conf -> sites-available/vh.conf # remove old (most likely) symlinked one - vhs = [v for v in vhs if v.filep != vhost_paths[realpath]] + new_vhs = [] + for v in vhs: + if v.filep == file_paths[realpath]: + internal_paths[realpath].remove( + get_internal_aug_path(v.path)) + else: + new_vhs.append(v) + vhs = new_vhs + + file_paths[realpath] = realpath + internal_paths[realpath].add(internal_path) + vhs.append(new_vhost) + elif internal_path not in internal_paths[realpath]: + internal_paths[realpath].add(internal_path) vhs.append(new_vhost) - vhost_paths[realpath] = realpath return vhs @@ -800,24 +835,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): avail_fp = nonssl_vhost.filep ssl_fp = self._get_ssl_vhost_path(avail_fp) - self._copy_create_ssl_vhost_skeleton(avail_fp, ssl_fp) + orig_matches = self.aug.match("/files%s//* [label()=~regexp('%s')]" % + (self._escape(ssl_fp), + parser.case_i("VirtualHost"))) + + self._copy_create_ssl_vhost_skeleton(nonssl_vhost, ssl_fp) # Reload augeas to take into account the new vhost self.aug.load() # Get Vhost augeas path for new vhost - vh_p = self.aug.match("/files%s//* [label()=~regexp('%s')]" % - (self._escape(ssl_fp), parser.case_i("VirtualHost"))) - if len(vh_p) != 1: - logger.error("Error: should only be one vhost in %s", avail_fp) - raise errors.PluginError("Currently, we only support " - "configurations with one vhost per file") - else: - # This simplifies the process - vh_p = vh_p[0] + new_matches = self.aug.match("/files%s//* [label()=~regexp('%s')]" % + (self._escape(ssl_fp), + parser.case_i("VirtualHost"))) + + vh_p = self._get_new_vh_path(orig_matches, new_matches) + + if not vh_p: + raise errors.PluginError( + "Could not reverse map the HTTPS VirtualHost to the original") # Update Addresses self._update_ssl_vhosts_addrs(vh_p) - # Add directives self._add_dummy_ssl_directives(vh_p) self.save() @@ -830,6 +868,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) + ssl_vhost.ancestor = nonssl_vhost self.vhosts.append(ssl_vhost) # NOTE: Searches through Augeas seem to ruin changes to directives @@ -843,6 +882,20 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return ssl_vhost + def _get_new_vh_path(self, orig_matches, new_matches): + """ Helper method for make_vhost_ssl for matching augeas paths. Returns + VirtualHost path from new_matches that's not present in orig_matches. + + Paths are normalized, because augeas leaves indices out for paths + with only single directive with a similar key """ + + orig_matches = [i.replace("[1]", "") for i in orig_matches] + for match in new_matches: + if match.replace("[1]", "") not in orig_matches: + # Return the unmodified path + return match + return None + def _get_ssl_vhost_path(self, non_ssl_vh_fp): # Get filepath of new ssl_vhost if non_ssl_vh_fp.endswith(".conf"): @@ -867,7 +920,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: bool """ - if not line.lstrip().startswith("RewriteRule"): + if not line.lower().lstrip().startswith("rewriterule"): return False # According to: http://httpd.apache.org/docs/2.4/rewrite/flags.html @@ -883,10 +936,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Sift line if it redirects the request to a HTTPS site return target.startswith("https://") - def _copy_create_ssl_vhost_skeleton(self, avail_fp, ssl_fp): + def _copy_create_ssl_vhost_skeleton(self, vhost, ssl_fp): """Copies over existing Vhost with IfModule mod_ssl.c> skeleton. - :param str avail_fp: Pointer to the original available non-ssl vhost + :param obj.VirtualHost vhost: Original VirtualHost object :param str ssl_fp: Full path where the new ssl_vhost will reside. A new file is created on the filesystem. @@ -894,70 +947,25 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # First register the creation so that it is properly removed if # configuration is rolled back - self.reverter.register_file_creation(False, ssl_fp) + if os.path.exists(ssl_fp): + notes = "Appended new VirtualHost directive to file %s" % ssl_fp + files = set() + files.add(ssl_fp) + self.reverter.add_to_checkpoint(files, notes) + else: + self.reverter.register_file_creation(False, ssl_fp) sift = False try: - with open(avail_fp, "r") as orig_file: - with open(ssl_fp, "w") as new_file: - new_file.write("\n") + orig_contents = self._get_vhost_block(vhost) + ssl_vh_contents, sift = self._sift_rewrite_rules(orig_contents) - comment = ("# Some rewrite rules in this file were " - "disabled on your HTTPS site,\n" - "# because they have the potential to create " - "redirection loops.\n") - - for line in orig_file: - A = line.lstrip().startswith("RewriteCond") - B = line.lstrip().startswith("RewriteRule") - - if not (A or B): - new_file.write(line) - continue - - # A RewriteRule that doesn't need filtering - if B and not self._sift_rewrite_rule(line): - new_file.write(line) - continue - - # A RewriteRule that does need filtering - if B and self._sift_rewrite_rule(line): - if not sift: - new_file.write(comment) - sift = True - new_file.write("# " + line) - continue - - # We save RewriteCond(s) and their corresponding - # RewriteRule in 'chunk'. - # We then decide whether we comment out the entire - # chunk based on its RewriteRule. - chunk = [] - if A: - chunk.append(line) - line = next(orig_file) - - # RewriteCond(s) must be followed by one RewriteRule - while not line.lstrip().startswith("RewriteRule"): - chunk.append(line) - line = next(orig_file) - - # Now, current line must start with a RewriteRule - chunk.append(line) - - if self._sift_rewrite_rule(line): - if not sift: - new_file.write(comment) - sift = True - - new_file.write(''.join( - ['# ' + l for l in chunk])) - continue - else: - new_file.write(''.join(chunk)) - continue - - new_file.write("\n") + with open(ssl_fp, "a") as new_file: + new_file.write("\n") + new_file.write("\n".join(ssl_vh_contents)) + # The content does not include the closing tag, so add it + new_file.write("\n") + new_file.write("\n") except IOError: logger.fatal("Error writing/reading to file in make_vhost_ssl") raise errors.PluginError("Unable to write/read in make_vhost_ssl") @@ -967,9 +975,96 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): reporter.add_message( "Some rewrite rules copied from {0} were disabled in the " "vhost for your HTTPS site located at {1} because they have " - "the potential to create redirection loops.".format(avail_fp, - ssl_fp), - reporter.MEDIUM_PRIORITY) + "the potential to create redirection loops.".format( + vhost.filep, ssl_fp), reporter.MEDIUM_PRIORITY) + self.aug.set("/augeas/files%s/mtime" % (self._escape(ssl_fp)), "0") + self.aug.set("/augeas/files%s/mtime" % (self._escape(vhost.filep)), "0") + + def _sift_rewrite_rules(self, contents): + """ Helper function for _copy_create_ssl_vhost_skeleton to prepare the + new HTTPS VirtualHost contents. Currently disabling the rewrites """ + + result = [] + sift = False + contents = iter(contents) + + comment = ("# Some rewrite rules in this file were " + "disabled on your HTTPS site,\n" + "# because they have the potential to create " + "redirection loops.\n") + + for line in contents: + A = line.lower().lstrip().startswith("rewritecond") + B = line.lower().lstrip().startswith("rewriterule") + + if not (A or B): + result.append(line) + continue + + # A RewriteRule that doesn't need filtering + if B and not self._sift_rewrite_rule(line): + result.append(line) + continue + + # A RewriteRule that does need filtering + if B and self._sift_rewrite_rule(line): + if not sift: + result.append(comment) + sift = True + result.append("# " + line) + continue + + # We save RewriteCond(s) and their corresponding + # RewriteRule in 'chunk'. + # We then decide whether we comment out the entire + # chunk based on its RewriteRule. + chunk = [] + if A: + chunk.append(line) + line = next(contents) + + # RewriteCond(s) must be followed by one RewriteRule + while not line.lower().lstrip().startswith("rewriterule"): + chunk.append(line) + line = next(contents) + + # Now, current line must start with a RewriteRule + chunk.append(line) + + if self._sift_rewrite_rule(line): + if not sift: + result.append(comment) + sift = True + + result.append('\n'.join( + ['# ' + l for l in chunk])) + continue + else: + result.append('\n'.join(chunk)) + continue + return result, sift + + def _get_vhost_block(self, vhost): + """ Helper method to get VirtualHost contents from the original file. + This is done with help of augeas span, which returns the span start and + end positions + + :returns: `list` of VirtualHost block content lines without closing tag + """ + + try: + span_val = self.aug.span(vhost.path) + except ValueError: + logger.fatal("Error while reading the VirtualHost %s from " + "file %s", vhost.name, vhost.filep, exc_info=True) + raise errors.PluginError("Unable to read VirtualHost from file") + span_filep = span_val[0] + span_start = span_val[5] + span_end = span_val[6] + with open(span_filep, 'r') as fh: + fh.seek(span_start) + vh_contents = fh.read(span_end-span_start) + return vh_contents.split("\n") def _update_ssl_vhosts_addrs(self, vh_path): ssl_addrs = set() @@ -1016,16 +1111,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser.add_dir(vh_path, "Include", self.mod_ssl_conf) def _add_servername_alias(self, target_name, vhost): - fp = self._escape(vhost.filep) - vh_p = self.aug.match("/files%s//* [label()=~regexp('%s')]" % - (fp, parser.case_i("VirtualHost"))) - if not vh_p: - return - vh_path = vh_p[0] - if (self.parser.find_dir("ServerName", target_name, - start=vh_path, exclude=False) or - self.parser.find_dir("ServerAlias", target_name, - start=vh_path, exclude=False)): + vh_path = vhost.path + sname, saliases = self._get_vhost_names(vh_path) + if target_name == sname or target_name in saliases: return if self._has_matching_wildcard(vh_path, target_name): return @@ -1425,7 +1513,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for re_path in rewrite_engine_path_list: # A RewriteEngine directive may also be included in per # directory .htaccess files. We only care about the VirtualHost. - if 'VirtualHost' in re_path: + if 'virtualhost' in re_path.lower(): return self.parser.get_arg(re_path) return False @@ -1516,6 +1604,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def _get_http_vhost(self, ssl_vhost): """Find appropriate HTTP vhost for ssl_vhost.""" # First candidate vhosts filter + if ssl_vhost.ancestor: + return ssl_vhost.ancestor candidate_http_vhs = [ vhost for vhost in self.vhosts if not vhost.ssl ] @@ -1827,25 +1917,46 @@ def get_file_path(vhost_path): :rtype: str """ - # Strip off /files/ - try: - if vhost_path.startswith("/files/"): - avail_fp = vhost_path[7:].split("/") - else: - return None - except AttributeError: - # If we received a None path + if not vhost_path or not vhost_path.startswith("/files/"): return None - last_good = "" - # Loop through the path parts and validate after every addition - for p in avail_fp: - cur_path = last_good+"/"+p - if os.path.exists(cur_path): - last_good = cur_path - else: - break - return last_good + return _split_aug_path(vhost_path)[0] + + +def get_internal_aug_path(vhost_path): + """Get the Augeas path for a vhost with the file path removed. + + :param str vhost_path: Augeas virtual host path + + :returns: Augeas path to vhost relative to the containing file + :rtype: str + + """ + return _split_aug_path(vhost_path)[1] + + +def _split_aug_path(vhost_path): + """Splits an Augeas path into a file path and an internal path. + + After removing "/files", this function splits vhost_path into the + file path and the remaining Augeas path. + + :param str vhost_path: Augeas virtual host path + + :returns: file path and internal Augeas path + :rtype: `tuple` of `str` + + """ + # Strip off /files + file_path = vhost_path[6:] + internal_path = [] + + # Remove components from the end of file_path until it becomes valid + while not os.path.exists(file_path): + file_path, _, internal_path_part = file_path.rpartition("/") + internal_path.append(internal_path_part) + + return file_path, "/".join(reversed(internal_path)) def install_ssl_options_conf(options_ssl): diff --git a/certbot-apache/certbot_apache/obj.py b/certbot-apache/certbot_apache/obj.py index 30cb24844..1e3579858 100644 --- a/certbot-apache/certbot_apache/obj.py +++ b/certbot-apache/certbot_apache/obj.py @@ -112,6 +112,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods :ivar bool ssl: SSLEngine on in vhost :ivar bool enabled: Virtual host is enabled :ivar bool modmacro: VirtualHost is using mod_macro + :ivar VirtualHost ancestor: A non-SSL VirtualHost this is based on https://httpd.apache.org/docs/2.4/vhosts/details.html @@ -123,7 +124,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods strip_name = re.compile(r"^(?:.+://)?([^ :$]*)") def __init__(self, filep, path, addrs, ssl, enabled, name=None, - aliases=None, modmacro=False): + aliases=None, modmacro=False, ancestor=None): # pylint: disable=too-many-arguments """Initialize a VH.""" @@ -135,6 +136,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods self.ssl = ssl self.enabled = enabled self.modmacro = modmacro + self.ancestor = ancestor def get_names(self): """Return a set of all names.""" diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 3e12bf60a..bffbeee6a 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -155,6 +155,17 @@ class MultipleVhostsTest(util.ApacheTest): self.assertEqual(get_file_path("nonexistent"), None) self.assertEqual(self.config._create_vhost("nonexistent"), None) # pylint: disable=protected-access + def test_get_aug_internal_path(self): + from certbot_apache.configurator import get_internal_aug_path + internal_paths = [ + "VirtualHost", "IfModule/VirtualHost", "VirtualHost", "VirtualHost", + "Macro/VirtualHost", "IfModule/VirtualHost", "VirtualHost", + "IfModule/VirtualHost"] + + for i, internal_path in enumerate(internal_paths): + self.assertEqual( + get_internal_aug_path(self.vh_truth[i].path), internal_path) + def test_bad_servername_alias(self): ssl_vh1 = obj.VirtualHost( "fp1", "ap1", set([obj.Addr(("*", "443"))]), @@ -586,6 +597,7 @@ class MultipleVhostsTest(util.ApacheTest): # already listens to the correct port self.assertEqual(mock_add_dir.call_count, 0) + def test_make_vhost_ssl(self): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) @@ -672,11 +684,6 @@ class MultipleVhostsTest(util.ApacheTest): len(self.config.parser.find_dir( directive, None, self.vh_truth[1].path, False)), 0) - def test_make_vhost_ssl_extra_vhs(self): - self.config.aug.match = mock.Mock(return_value=["p1", "p2"]) - self.assertRaises( - errors.PluginError, self.config.make_vhost_ssl, self.vh_truth[0]) - def test_make_vhost_ssl_bad_write(self): mock_open = mock.mock_open() # This calls open @@ -811,6 +818,15 @@ class MultipleVhostsTest(util.ApacheTest): def test_supported_enhancements(self): self.assertTrue(isinstance(self.config.supported_enhancements(), list)) + def test_find_http_vhost_without_ancestor(self): + # pylint: disable=protected-access + vhost = self.vh_truth[0] + vhost.ssl = True + vhost.ancestor = None + res = self.config._get_http_vhost(vhost) + self.assertEqual(self.vh_truth[0].name, res.name) + self.assertEqual(self.vh_truth[0].aliases, res.aliases) + @mock.patch("certbot_apache.configurator.ApacheConfigurator._get_http_vhost") @mock.patch("certbot_apache.display_ops.select_vhost") @mock.patch("certbot.util.exe_exists") @@ -1010,8 +1026,9 @@ class MultipleVhostsTest(util.ApacheTest): # three args to rw_rule self.assertEqual(len(rw_rule), 3) - self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path)) - self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path)) + # [:-3] to remove the vhost index number + self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path[:-3])) + self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path[:-3])) self.assertTrue("rewrite_module" in self.config.parser.modules) @@ -1059,9 +1076,9 @@ class MultipleVhostsTest(util.ApacheTest): self.assertEqual(len(rw_engine), 1) # three args to rw_rule + 1 arg for the pre existing rewrite self.assertEqual(len(rw_rule), 5) - - self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path)) - self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path)) + # [:-3] to remove the vhost index number + self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path[:-3])) + self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path[:-3])) self.assertTrue("rewrite_module" in self.config.parser.modules) @@ -1169,89 +1186,6 @@ class MultipleVhostsTest(util.ApacheTest): not_rewriterule = "NotRewriteRule ^ ..." self.assertFalse(self.config._sift_rewrite_rule(not_rewriterule)) - @certbot_util.patch_get_utility() - def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_get_utility): - self.config.parser.modules.add("rewrite_module") - - http_vhost = self.vh_truth[0] - - self.config.parser.add_dir( - http_vhost.path, "RewriteEngine", "on") - - self.config.parser.add_dir( - http_vhost.path, "RewriteRule", - ["^", - "https://%{SERVER_NAME}%{REQUEST_URI}", - "[L,NE,R=permanent]"]) - self.config.save() - - ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) - - self.assertTrue(self.config.parser.find_dir( - "RewriteEngine", "on", ssl_vhost.path, False)) - - conf_text = open(ssl_vhost.filep).read() - commented_rewrite_rule = ("# RewriteRule ^ " - "https://%{SERVER_NAME}%{REQUEST_URI} " - "[L,NE,R=permanent]") - self.assertTrue(commented_rewrite_rule in conf_text) - mock_get_utility().add_message.assert_called_once_with(mock.ANY, - - mock.ANY) - @certbot_util.patch_get_utility() - def test_make_vhost_ssl_with_existing_rewrite_conds(self, mock_get_utility): - self.config.parser.modules.add("rewrite_module") - - http_vhost = self.vh_truth[0] - - self.config.parser.add_dir( - http_vhost.path, "RewriteEngine", "on") - - # Add a chunk that should not be commented out. - self.config.parser.add_dir(http_vhost.path, - "RewriteCond", ["%{DOCUMENT_ROOT}/%{REQUEST_FILENAME}", "!-f"]) - self.config.parser.add_dir( - http_vhost.path, "RewriteRule", - ["^(.*)$", "b://u%{REQUEST_URI}", "[P,NE,L]"]) - - # Add a chunk that should be commented out. - self.config.parser.add_dir(http_vhost.path, - "RewriteCond", ["%{HTTPS}", "!=on"]) - self.config.parser.add_dir(http_vhost.path, - "RewriteCond", ["%{HTTPS}", "!^$"]) - self.config.parser.add_dir( - http_vhost.path, "RewriteRule", - ["^", - "https://%{SERVER_NAME}%{REQUEST_URI}", - "[L,NE,R=permanent]"]) - - self.config.save() - - ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) - - conf_line_set = set(open(ssl_vhost.filep).read().splitlines()) - - not_commented_cond1 = ("RewriteCond " - "%{DOCUMENT_ROOT}/%{REQUEST_FILENAME} !-f") - not_commented_rewrite_rule = ("RewriteRule " - "^(.*)$ b://u%{REQUEST_URI} [P,NE,L]") - - commented_cond1 = "# RewriteCond %{HTTPS} !=on" - commented_cond2 = "# RewriteCond %{HTTPS} !^$" - commented_rewrite_rule = ("# RewriteRule ^ " - "https://%{SERVER_NAME}%{REQUEST_URI} " - "[L,NE,R=permanent]") - - self.assertTrue(not_commented_cond1 in conf_line_set) - self.assertTrue(not_commented_rewrite_rule in conf_line_set) - - self.assertTrue(commented_cond1 in conf_line_set) - self.assertTrue(commented_cond2 in conf_line_set) - self.assertTrue(commented_rewrite_rule in conf_line_set) - mock_get_utility().add_message.assert_called_once_with(mock.ANY, - mock.ANY) - - def get_achalls(self): """Return testing achallenges.""" account_key = self.rsa512jwk @@ -1362,6 +1296,126 @@ class AugeasVhostsTest(util.ApacheTest): self.config.choose_vhost(name) self.assertEqual(mock_select.call_count, 0) + def test_augeas_span_error(self): + broken_vhost = self.config.vhosts[0] + broken_vhost.path = broken_vhost.path + "/nonexistent" + self.assertRaises(errors.PluginError, self.config.make_vhost_ssl, + broken_vhost) + +class MultiVhostsTest(util.ApacheTest): + """Test vhosts with illegal names dependant on augeas version.""" + # pylint: disable=protected-access + + def setUp(self): # pylint: disable=arguments-differ + td = "debian_apache_2_4/multi_vhosts" + cr = "debian_apache_2_4/multi_vhosts/apache2" + vr = "debian_apache_2_4/multi_vhosts/apache2/sites-available" + super(MultiVhostsTest, self).setUp(test_dir=td, + config_root=cr, + vhost_root=vr) + + self.config = util.get_apache_configurator( + self.config_path, self.vhost_path, self.config_dir, self.work_dir) + self.vh_truth = util.get_vh_truth( + self.temp_dir, "debian_apache_2_4/multi_vhosts") + + def tearDown(self): + shutil.rmtree(self.temp_dir) + shutil.rmtree(self.config_dir) + shutil.rmtree(self.work_dir) + + def test_make_vhost_ssl(self): + ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[1]) + + self.assertEqual( + ssl_vhost.filep, + os.path.join(self.config_path, "sites-available", + "default-le-ssl.conf")) + + self.assertEqual(ssl_vhost.path, + "/files" + ssl_vhost.filep + "/IfModule/VirtualHost") + self.assertEqual(len(ssl_vhost.addrs), 1) + self.assertEqual(set([obj.Addr.fromstring("*:443")]), ssl_vhost.addrs) + self.assertEqual(ssl_vhost.name, "banana.vomit.com") + self.assertTrue(ssl_vhost.ssl) + self.assertFalse(ssl_vhost.enabled) + + self.assertTrue(self.config.parser.find_dir( + "SSLCertificateFile", None, ssl_vhost.path, False)) + self.assertTrue(self.config.parser.find_dir( + "SSLCertificateKeyFile", None, ssl_vhost.path, False)) + + self.assertEqual(self.config.is_name_vhost(self.vh_truth[1]), + self.config.is_name_vhost(ssl_vhost)) + + mock_path = "certbot_apache.configurator.ApacheConfigurator._get_new_vh_path" + with mock.patch(mock_path) as mock_getpath: + mock_getpath.return_value = None + self.assertRaises(errors.PluginError, self.config.make_vhost_ssl, + self.vh_truth[1]) + + def test_get_new_path(self): + with_index_1 = ["/path[1]/section[1]"] + without_index = ["/path/section"] + with_index_2 = ["/path[2]/section[2]"] + self.assertEqual(self.config._get_new_vh_path(without_index, + with_index_1), + None) + self.assertEqual(self.config._get_new_vh_path(without_index, + with_index_2), + with_index_2[0]) + + both = with_index_1 + with_index_2 + self.assertEqual(self.config._get_new_vh_path(without_index, both), + with_index_2[0]) + + @certbot_util.patch_get_utility() + def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_get_utility): + self.config.parser.modules.add("rewrite_module") + + ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[4]) + + self.assertTrue(self.config.parser.find_dir( + "RewriteEngine", "on", ssl_vhost.path, False)) + + conf_text = open(ssl_vhost.filep).read() + commented_rewrite_rule = ("# RewriteRule \"^/secrets/(.+)\" " + "\"https://new.example.com/docs/$1\" [R,L]") + uncommented_rewrite_rule = ("RewriteRule \"^/docs/(.+)\" " + "\"http://new.example.com/docs/$1\" [R,L]") + self.assertTrue(commented_rewrite_rule in conf_text) + self.assertTrue(uncommented_rewrite_rule in conf_text) + mock_get_utility().add_message.assert_called_once_with(mock.ANY, + mock.ANY) + + @certbot_util.patch_get_utility() + def test_make_vhost_ssl_with_existing_rewrite_conds(self, mock_get_utility): + self.config.parser.modules.add("rewrite_module") + + ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[3]) + + conf_lines = open(ssl_vhost.filep).readlines() + conf_line_set = [l.strip() for l in conf_lines] + not_commented_cond1 = ("RewriteCond " + "%{DOCUMENT_ROOT}/%{REQUEST_FILENAME} !-f") + not_commented_rewrite_rule = ("RewriteRule " + "^(.*)$ b://u%{REQUEST_URI} [P,NE,L]") + + commented_cond1 = "# RewriteCond %{HTTPS} !=on" + commented_cond2 = "# RewriteCond %{HTTPS} !^$" + commented_rewrite_rule = ("# RewriteRule ^ " + "https://%{SERVER_NAME}%{REQUEST_URI} " + "[L,NE,R=permanent]") + + self.assertTrue(not_commented_cond1 in conf_line_set) + self.assertTrue(not_commented_rewrite_rule in conf_line_set) + + self.assertTrue(commented_cond1 in conf_line_set) + self.assertTrue(commented_cond2 in conf_line_set) + self.assertTrue(commented_rewrite_rule in conf_line_set) + mock_get_utility().add_message.assert_called_once_with(mock.ANY, + mock.ANY) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/apache2.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/apache2.conf new file mode 100644 index 000000000..2a5bb7be2 --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/apache2.conf @@ -0,0 +1,196 @@ +# This is the main Apache server configuration file. It contains the +# configuration directives that give the server its instructions. +# See http://httpd.apache.org/docs/2.4/ for detailed information about +# the directives and /usr/share/doc/apache2/README.Debian about Debian specific +# hints. +# +# +# Summary of how the Apache 2 configuration works in Debian: +# The Apache 2 web server configuration in Debian is quite different to +# upstream's suggested way to configure the web server. This is because Debian's +# default Apache2 installation attempts to make adding and removing modules, +# virtual hosts, and extra configuration directives as flexible as possible, in +# order to make automating the changes and administering the server as easy as +# possible. + +# It is split into several files forming the configuration hierarchy outlined +# below, all located in the /etc/apache2/ directory: +# +# /etc/apache2/ +# |-- apache2.conf +# | `-- ports.conf +# |-- mods-enabled +# | |-- *.load +# | `-- *.conf +# |-- conf-enabled +# | `-- *.conf +# `-- sites-enabled +# `-- *.conf +# +# +# * apache2.conf is the main configuration file (this file). It puts the pieces +# together by including all remaining configuration files when starting up the +# web server. +# +# * ports.conf is always included from the main configuration file. It is +# supposed to determine listening ports for incoming connections which can be +# customized anytime. +# +# * Configuration files in the mods-enabled/, conf-enabled/ and sites-enabled/ +# directories contain particular configuration snippets which manage modules, +# global configuration fragments, or virtual host configurations, +# respectively. +# +# They are activated by symlinking available configuration files from their +# respective *-available/ counterparts. These should be managed by using our +# helpers a2enmod/a2dismod, a2ensite/a2dissite and a2enconf/a2disconf. See +# their respective man pages for detailed information. +# +# * The binary is called apache2. Due to the use of environment variables, in +# the default configuration, apache2 needs to be started/stopped with +# /etc/init.d/apache2 or apache2ctl. Calling /usr/bin/apache2 directly will not +# work with the default configuration. + + +# Global configuration + +# +# The accept serialization lock file MUST BE STORED ON A LOCAL DISK. +# +Mutex file:${APACHE_LOCK_DIR} default + +# +# PidFile: The file in which the server should record its process +# identification number when it starts. +# This needs to be set in /etc/apache2/envvars +# +PidFile ${APACHE_PID_FILE} + +# +# Timeout: The number of seconds before receives and sends time out. +# +Timeout 300 + +# +# KeepAlive: Whether or not to allow persistent connections (more than +# one request per connection). Set to "Off" to deactivate. +# +KeepAlive On + +# +# MaxKeepAliveRequests: The maximum number of requests to allow +# during a persistent connection. Set to 0 to allow an unlimited amount. +# We recommend you leave this number high, for maximum performance. +# +MaxKeepAliveRequests 100 + +# +# KeepAliveTimeout: Number of seconds to wait for the next request from the +# same client on the same connection. +# +KeepAliveTimeout 5 + + +# These need to be set in /etc/apache2/envvars +User ${APACHE_RUN_USER} +Group ${APACHE_RUN_GROUP} + +# +# HostnameLookups: Log the names of clients or just their IP addresses +# e.g., www.apache.org (on) or 204.62.129.132 (off). +# The default is off because it'd be overall better for the net if people +# had to knowingly turn this feature on, since enabling it means that +# each client request will result in AT LEAST one lookup request to the +# nameserver. +# +HostnameLookups Off + +# ErrorLog: The location of the error log file. +# If you do not specify an ErrorLog directive within a +# container, error messages relating to that virtual host will be +# logged here. If you *do* define an error logfile for a +# container, that host's errors will be logged there and not here. +# +ErrorLog ${APACHE_LOG_DIR}/error.log + +# +# LogLevel: Control the severity of messages logged to the error_log. +# Available values: trace8, ..., trace1, debug, info, notice, warn, +# error, crit, alert, emerg. +# It is also possible to configure the log level for particular modules, e.g. +# "LogLevel info ssl:warn" +# +LogLevel warn + +# Include module configuration: +IncludeOptional mods-enabled/*.load +IncludeOptional mods-enabled/*.conf + +# Include list of ports to listen on +Include ports.conf + + +# Sets the default security model of the Apache2 HTTPD server. It does +# not allow access to the root filesystem outside of /usr/share and /var/www. +# The former is used by web applications packaged in Debian, +# the latter may be used for local directories served by the web server. If +# your system is serving content from a sub-directory in /srv you must allow +# access here, or in any related virtual host. + + Options FollowSymLinks + AllowOverride None + Require all denied + + + + AllowOverride None + Require all granted + + + + Options Indexes FollowSymLinks + AllowOverride None + Require all granted + + +# AccessFileName: The name of the file to look for in each directory +# for additional configuration directives. See also the AllowOverride +# directive. +# +AccessFileName .htaccess + +# +# The following lines prevent .htaccess and .htpasswd files from being +# viewed by Web clients. +# + + Require all denied + + +# The following directives define some format nicknames for use with +# a CustomLog directive. +# +# These deviate from the Common Log Format definitions in that they use %O +# (the actual bytes sent including headers) instead of %b (the size of the +# requested file), because the latter makes it impossible to detect partial +# requests. +# +# Note that the use of %{X-Forwarded-For}i instead of %h is not recommended. +# Use mod_remoteip instead. +# +LogFormat "%v:%p %h %l %u %t \"%r\" %>s %O \"%{Referer}i\" \"%{User-Agent}i\"" vhost_combined +LogFormat "%h %l %u %t \"%r\" %>s %O \"%{Referer}i\" \"%{User-Agent}i\"" combined +LogFormat "%h %l %u %t \"%r\" %>s %O" common +LogFormat "%{Referer}i -> %U" referer +LogFormat "%{User-agent}i" agent + +# Include of directories ignores editors' and dpkg's backup files, +# see README.Debian for details. + +# Include generic snippets of statements +IncludeOptional conf-enabled/*.conf + +# Include the virtual host configurations: +IncludeOptional sites-enabled/*.conf + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/envvars b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/envvars new file mode 100644 index 000000000..a13d9a89e --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/envvars @@ -0,0 +1,29 @@ +# envvars - default environment variables for apache2ctl + +# this won't be correct after changing uid +unset HOME + +# for supporting multiple apache2 instances +if [ "${APACHE_CONFDIR##/etc/apache2-}" != "${APACHE_CONFDIR}" ] ; then + SUFFIX="-${APACHE_CONFDIR##/etc/apache2-}" +else + SUFFIX= +fi + +# Since there is no sane way to get the parsed apache2 config in scripts, some +# settings are defined via environment variables and then used in apache2ctl, +# /etc/init.d/apache2, /etc/logrotate.d/apache2, etc. +export APACHE_RUN_USER=www-data +export APACHE_RUN_GROUP=www-data +# temporary state file location. This might be changed to /run in Wheezy+1 +export APACHE_PID_FILE=/var/run/apache2/apache2$SUFFIX.pid +export APACHE_RUN_DIR=/var/run/apache2$SUFFIX +export APACHE_LOCK_DIR=/var/lock/apache2$SUFFIX +# Only /var/log/apache2 is handled by /etc/logrotate.d/apache2. +export APACHE_LOG_DIR=/var/log/apache2$SUFFIX + +## The locale used by some modules like mod_dav +export LANG=C + +export LANG + diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/ports.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/ports.conf new file mode 100644 index 000000000..5daec58c1 --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/ports.conf @@ -0,0 +1,15 @@ +# If you just change the port or add more ports here, you will likely also +# have to change the VirtualHost statement in +# /etc/apache2/sites-enabled/000-default.conf + +Listen 80 + + + Listen 443 + + + + Listen 443 + + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-available/default.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-available/default.conf new file mode 100644 index 000000000..6ab206b2d --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-available/default.conf @@ -0,0 +1,22 @@ + + + ServerName banana.vomit.net + ServerAdmin webmaster@localhost + DocumentRoot /var/www/html + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + + + + + ServerName banana.vomit.com + ServerAdmin webmaster@localhost + DocumentRoot /var/www/html + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-available/multi-vhost.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-available/multi-vhost.conf new file mode 100644 index 000000000..5f2b727bf --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-available/multi-vhost.conf @@ -0,0 +1,38 @@ + + ServerName 1.multi.vhost.tld + ServerAlias first.multi.vhost.tld + ServerAdmin webmaster@localhost + DocumentRoot /var/www/html + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + + + + ServerName 2.multi.vhost.tld + ServerAlias second.multi.vhost.tld + ServerAdmin webmaster@localhost + DocumentRoot /var/www/html + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined +RewriteEngine on +RewriteCond %{DOCUMENT_ROOT}/%{REQUEST_FILENAME} !-f +RewriteRule ^(.*)$ b://u%{REQUEST_URI} [P,NE,L] +RewriteCond %{HTTPS} !=on +RewriteCond %{HTTPS} !^$ +RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,NE,R=permanent] + + + + + ServerName 3.multi.vhost.tld + ServerAlias third.multi.vhost.tld + ServerAdmin webmaster@localhost + DocumentRoot /var/www/html + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined +RewriteEngine on +RewriteRule "^/secrets/(.+)" "https://new.example.com/docs/$1" [R,L] +RewriteRule "^/docs/(.+)" "http://new.example.com/docs/$1" [R,L] + + diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-enabled/default.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-enabled/default.conf new file mode 120000 index 000000000..032e6bcf0 --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-enabled/default.conf @@ -0,0 +1 @@ +../sites-available/default.conf \ No newline at end of file diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-enabled/multi-vhost.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-enabled/multi-vhost.conf new file mode 120000 index 000000000..7f0910ff4 --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multi_vhosts/apache2/sites-enabled/multi-vhost.conf @@ -0,0 +1 @@ +../sites-available/multi-vhost.conf \ No newline at end of file diff --git a/certbot-apache/certbot_apache/tests/util.py b/certbot-apache/certbot_apache/tests/util.py index 3c33a0e19..d40152ad5 100644 --- a/certbot-apache/certbot_apache/tests/util.py +++ b/certbot-apache/certbot_apache/tests/util.py @@ -164,5 +164,35 @@ def get_vh_truth(temp_dir, config_name): set([obj.Addr.fromstring("10.2.3.4:443")]), True, True, "ocspvhost.com")] return vh_truth - + if config_name == "debian_apache_2_4/multi_vhosts": + prefix = os.path.join( + temp_dir, config_name, "apache2/sites-available") + aug_pre = "/files" + prefix + vh_truth = [ + obj.VirtualHost( + os.path.join(prefix, "default.conf"), + os.path.join(aug_pre, "default.conf/VirtualHost[1]"), + set([obj.Addr.fromstring("*:80")]), + False, True, "ip-172-30-0-17"), + obj.VirtualHost( + os.path.join(prefix, "default.conf"), + os.path.join(aug_pre, "default.conf/VirtualHost[2]"), + set([obj.Addr.fromstring("*:80")]), + False, True, "banana.vomit.com"), + obj.VirtualHost( + os.path.join(prefix, "multi-vhost.conf"), + os.path.join(aug_pre, "multi-vhost.conf/VirtualHost[1]"), + set([obj.Addr.fromstring("*:80")]), + False, True, "1.multi.vhost.tld"), + obj.VirtualHost( + os.path.join(prefix, "multi-vhost.conf"), + os.path.join(aug_pre, "multi-vhost.conf/IfModule/VirtualHost"), + set([obj.Addr.fromstring("*:80")]), + False, True, "2.multi.vhost.tld"), + obj.VirtualHost( + os.path.join(prefix, "multi-vhost.conf"), + os.path.join(aug_pre, "multi-vhost.conf/VirtualHost[2]"), + set([obj.Addr.fromstring("*:80")]), + False, True, "3.multi.vhost.tld")] + return vh_truth return None # pragma: no cover diff --git a/certbot/reverter.py b/certbot/reverter.py index 32355782e..34feafc7e 100644 --- a/certbot/reverter.py +++ b/certbot/reverter.py @@ -491,7 +491,7 @@ class Reverter(object): else: logger.warning( "File: %s - Could not be found to be deleted %s - " - "LE probably shut down unexpectedly", + "Certbot probably shut down unexpectedly", os.linesep, path) except (IOError, OSError): logger.fatal(