mirror of
https://github.com/certbot/certbot.git
synced 2026-01-21 19:01:07 +03:00
Merge pull request #1707 from sagi/bugfix_redirect
Fix Redirect Verification
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
"""Apache Configuration based off of Augeas Configurator."""
|
||||
# pylint: disable=too-many-lines
|
||||
import filecmp
|
||||
import itertools
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
@@ -26,6 +25,7 @@ from letsencrypt_apache import tls_sni_01
|
||||
from letsencrypt_apache import obj
|
||||
from letsencrypt_apache import parser
|
||||
|
||||
from collections import defaultdict
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -904,15 +904,33 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
|
||||
"redirection")
|
||||
self._create_redirect_vhost(ssl_vhost)
|
||||
else:
|
||||
# Check if redirection already exists
|
||||
self._verify_no_redirects(general_vh)
|
||||
# Check if LetsEncrypt redirection already exists
|
||||
self._verify_no_letsencrypt_redirect(general_vh)
|
||||
|
||||
|
||||
# Note: if code flow gets here it means we didn't find the exact
|
||||
# letsencrypt RewriteRule config for redirection. Finding
|
||||
# another RewriteRule is likely to be fine in most or all cases,
|
||||
# but redirect loops are possible in very obscure cases; see #1620
|
||||
# for reasoning.
|
||||
if self._is_rewrite_exists(general_vh):
|
||||
logger.warn("Added an HTTP->HTTPS rewrite in addition to "
|
||||
"other RewriteRules; you may wish to check for "
|
||||
"overall consistency.")
|
||||
|
||||
# Add directives to server
|
||||
# Note: These are not immediately searchable in sites-enabled
|
||||
# even with save() and load()
|
||||
self.parser.add_dir(general_vh.path, "RewriteEngine", "on")
|
||||
self.parser.add_dir(general_vh.path, "RewriteRule",
|
||||
if not self._is_rewrite_engine_on(general_vh):
|
||||
self.parser.add_dir(general_vh.path, "RewriteEngine", "on")
|
||||
|
||||
if self.get_version() >= (2, 3, 9):
|
||||
self.parser.add_dir(general_vh.path, "RewriteRule",
|
||||
constants.REWRITE_HTTPS_ARGS_WITH_END)
|
||||
else:
|
||||
self.parser.add_dir(general_vh.path, "RewriteRule",
|
||||
constants.REWRITE_HTTPS_ARGS)
|
||||
|
||||
self.save_notes += ("Redirecting host in %s to ssl vhost in %s\n" %
|
||||
(general_vh.filep, ssl_vhost.filep))
|
||||
self.save()
|
||||
@@ -920,40 +938,67 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
|
||||
logger.info("Redirecting vhost in %s to ssl vhost in %s",
|
||||
general_vh.filep, ssl_vhost.filep)
|
||||
|
||||
def _verify_no_redirects(self, vhost):
|
||||
"""Checks to see if existing redirect is in place.
|
||||
def _verify_no_letsencrypt_redirect(self, vhost):
|
||||
"""Checks to see if a redirect was already installed by letsencrypt.
|
||||
|
||||
Checks to see if virtualhost already contains a rewrite or redirect
|
||||
returns boolean, integer
|
||||
Checks to see if virtualhost already contains a rewrite rule that is
|
||||
identical to Letsencrypt's redirection rewrite rule.
|
||||
|
||||
:param vhost: vhost to check
|
||||
:type vhost: :class:`~letsencrypt_apache.obj.VirtualHost`
|
||||
|
||||
:raises errors.PluginEnhancementAlreadyPresent: When the exact
|
||||
letsencrypt redirection WriteRule exists in virtual host.
|
||||
|
||||
errors.PluginError: When there exists directives that may hint
|
||||
other redirection. (TODO: We should not throw a PluginError,
|
||||
but that's for an other PR.)
|
||||
"""
|
||||
rewrite_path = self.parser.find_dir(
|
||||
"RewriteRule", None, start=vhost.path)
|
||||
redirect_path = self.parser.find_dir("Redirect", None, start=vhost.path)
|
||||
"RewriteRule", None, start=vhost.path)
|
||||
|
||||
if redirect_path:
|
||||
# "Existing Redirect directive for virtualhost"
|
||||
raise errors.PluginError("Existing Redirect present on HTTP vhost.")
|
||||
if rewrite_path:
|
||||
# "No existing redirection for virtualhost"
|
||||
if len(rewrite_path) != len(constants.REWRITE_HTTPS_ARGS):
|
||||
raise errors.PluginError("Unknown Existing RewriteRule")
|
||||
for match, arg in itertools.izip(
|
||||
rewrite_path, constants.REWRITE_HTTPS_ARGS):
|
||||
if self.aug.get(match) != arg:
|
||||
raise errors.PluginError("Unknown Existing RewriteRule")
|
||||
# There can be other RewriteRule directive lines in vhost config.
|
||||
# rewrite_args_dict keys are directive ids and the corresponding value
|
||||
# for each is a list of arguments to that directive.
|
||||
rewrite_args_dict = defaultdict(list)
|
||||
pat = r'.*(directive\[\d+\]).*'
|
||||
for match in rewrite_path:
|
||||
m = re.match(pat, match)
|
||||
if m:
|
||||
dir_id = m.group(1)
|
||||
rewrite_args_dict[dir_id].append(match)
|
||||
|
||||
raise errors.PluginEnhancementAlreadyPresent(
|
||||
"Let's Encrypt has already enabled redirection")
|
||||
if rewrite_args_dict:
|
||||
redirect_args = [constants.REWRITE_HTTPS_ARGS,
|
||||
constants.REWRITE_HTTPS_ARGS_WITH_END]
|
||||
|
||||
for matches in rewrite_args_dict.values():
|
||||
if [self.aug.get(x) for x in matches] in redirect_args:
|
||||
raise errors.PluginEnhancementAlreadyPresent(
|
||||
"Let's Encrypt has already enabled redirection")
|
||||
|
||||
def _is_rewrite_exists(self, vhost):
|
||||
"""Checks if there exists a RewriteRule directive in vhost
|
||||
|
||||
:param vhost: vhost to check
|
||||
:type vhost: :class:`~letsencrypt_apache.obj.VirtualHost`
|
||||
|
||||
:returns: True if a RewriteRule directive exists.
|
||||
:rtype: bool
|
||||
|
||||
"""
|
||||
rewrite_path = self.parser.find_dir(
|
||||
"RewriteRule", None, start=vhost.path)
|
||||
return bool(rewrite_path)
|
||||
|
||||
def _is_rewrite_engine_on(self, vhost):
|
||||
"""Checks if a RewriteEngine directive is on
|
||||
|
||||
:param vhost: vhost to check
|
||||
:type vhost: :class:`~letsencrypt_apache.obj.VirtualHost`
|
||||
|
||||
"""
|
||||
rewrite_engine_path = self.parser.find_dir("RewriteEngine", "on",
|
||||
start=vhost.path)
|
||||
if rewrite_engine_path:
|
||||
return self.parser.get_arg(rewrite_engine_path[0])
|
||||
return False
|
||||
|
||||
def _create_redirect_vhost(self, ssl_vhost):
|
||||
"""Creates an http_vhost specifically to redirect for the ssl_vhost.
|
||||
@@ -990,6 +1035,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
|
||||
if ssl_vhost.aliases:
|
||||
serveralias = "ServerAlias " + " ".join(ssl_vhost.aliases)
|
||||
|
||||
rewrite_rule_args = []
|
||||
if self.get_version() >= (2, 3, 9):
|
||||
rewrite_rule_args = constants.REWRITE_HTTPS_ARGS_WITH_END
|
||||
else:
|
||||
rewrite_rule_args = constants.REWRITE_HTTPS_ARGS
|
||||
|
||||
|
||||
return ("<VirtualHost %s>\n"
|
||||
"%s \n"
|
||||
"%s \n"
|
||||
@@ -1003,7 +1055,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
|
||||
"</VirtualHost>\n"
|
||||
% (" ".join(str(addr) for addr in self._get_proposed_addrs(ssl_vhost)),
|
||||
servername, serveralias,
|
||||
" ".join(constants.REWRITE_HTTPS_ARGS)))
|
||||
" ".join(rewrite_rule_args)))
|
||||
|
||||
def _write_out_redirect(self, ssl_vhost, text):
|
||||
# This is the default name
|
||||
|
||||
@@ -25,8 +25,12 @@ AUGEAS_LENS_DIR = pkg_resources.resource_filename(
|
||||
|
||||
REWRITE_HTTPS_ARGS = [
|
||||
"^", "https://%{SERVER_NAME}%{REQUEST_URI}", "[L,QSA,R=permanent]"]
|
||||
"""Apache rewrite rule arguments used for redirections to https vhost"""
|
||||
"""Apache version<2.3.9 rewrite rule arguments used for redirections to https vhost"""
|
||||
|
||||
REWRITE_HTTPS_ARGS_WITH_END = [
|
||||
"^", "https://%{SERVER_NAME}%{REQUEST_URI}", "[END,QSA,R=permanent]"]
|
||||
"""Apache version >= 2.3.9 rewrite rule arguments used for redirections to
|
||||
https vhost"""
|
||||
|
||||
HSTS_ARGS = ["always", "set", "Strict-Transport-Security",
|
||||
"\"max-age=31536000; includeSubDomains\""]
|
||||
|
||||
@@ -653,6 +653,19 @@ class TwoVhost80Test(util.ApacheTest):
|
||||
def test_supported_enhancements(self):
|
||||
self.assertTrue(isinstance(self.config.supported_enhancements(), list))
|
||||
|
||||
|
||||
@mock.patch("letsencrypt.le_util.exe_exists")
|
||||
def test_enhance_unknown_vhost(self, mock_exe):
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
mock_exe.return_value = True
|
||||
ssl_vh = obj.VirtualHost(
|
||||
"fp", "ap", set([obj.Addr(("*", "443")), obj.Addr(("satoshi.com",))]),
|
||||
True, False)
|
||||
self.config.vhosts.append(ssl_vh)
|
||||
self.assertRaises(
|
||||
errors.PluginError,
|
||||
self.config.enhance, "satoshi.com", "redirect")
|
||||
|
||||
def test_enhance_unknown_enhancement(self):
|
||||
self.assertRaises(
|
||||
errors.PluginError,
|
||||
@@ -741,6 +754,8 @@ class TwoVhost80Test(util.ApacheTest):
|
||||
def test_redirect_well_formed_http(self, mock_exe, _):
|
||||
self.config.parser.update_runtime_variables = mock.Mock()
|
||||
mock_exe.return_value = True
|
||||
self.config.get_version = mock.Mock(return_value=(2, 2))
|
||||
|
||||
# This will create an ssl vhost for letsencrypt.demo
|
||||
self.config.enhance("letsencrypt.demo", "redirect")
|
||||
|
||||
@@ -760,6 +775,55 @@ class TwoVhost80Test(util.ApacheTest):
|
||||
|
||||
self.assertTrue("rewrite_module" in self.config.parser.modules)
|
||||
|
||||
def test_rewrite_rule_exists(self):
|
||||
# Skip the enable mod
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.get_version = mock.Mock(return_value=(2, 3, 9))
|
||||
self.config.parser.add_dir(
|
||||
self.vh_truth[3].path, "RewriteRule", ["Unknown"])
|
||||
self.assertTrue(self.config._is_rewrite_exists(self.vh_truth[3])) # pylint: disable=protected-access
|
||||
|
||||
def test_rewrite_engine_exists(self):
|
||||
# Skip the enable mod
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.get_version = mock.Mock(return_value=(2, 3, 9))
|
||||
self.config.parser.add_dir(
|
||||
self.vh_truth[3].path, "RewriteEngine", "on")
|
||||
self.assertTrue(self.config._is_rewrite_engine_on(self.vh_truth[3])) # pylint: disable=protected-access
|
||||
|
||||
|
||||
@mock.patch("letsencrypt.le_util.run_script")
|
||||
@mock.patch("letsencrypt.le_util.exe_exists")
|
||||
def test_redirect_with_existing_rewrite(self, mock_exe, _):
|
||||
self.config.parser.update_runtime_variables = mock.Mock()
|
||||
mock_exe.return_value = True
|
||||
self.config.get_version = mock.Mock(return_value=(2, 2))
|
||||
|
||||
# Create a preexisting rewrite rule
|
||||
self.config.parser.add_dir(
|
||||
self.vh_truth[3].path, "RewriteRule", ["Unknown"])
|
||||
self.config.save()
|
||||
|
||||
# This will create an ssl vhost for letsencrypt.demo
|
||||
self.config.enhance("letsencrypt.demo", "redirect")
|
||||
|
||||
# These are not immediately available in find_dir even with save() and
|
||||
# load(). They must be found in sites-available
|
||||
rw_engine = self.config.parser.find_dir(
|
||||
"RewriteEngine", "on", self.vh_truth[3].path)
|
||||
rw_rule = self.config.parser.find_dir(
|
||||
"RewriteRule", None, self.vh_truth[3].path)
|
||||
|
||||
self.assertEqual(len(rw_engine), 1)
|
||||
# three args to rw_rule + 1 arg for the pre existing rewrite
|
||||
self.assertEqual(len(rw_rule), 4)
|
||||
|
||||
self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path))
|
||||
self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path))
|
||||
|
||||
self.assertTrue("rewrite_module" in self.config.parser.modules)
|
||||
|
||||
|
||||
def test_redirect_with_conflict(self):
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
ssl_vh = obj.VirtualHost(
|
||||
@@ -774,43 +838,16 @@ class TwoVhost80Test(util.ApacheTest):
|
||||
def test_redirect_twice(self):
|
||||
# Skip the enable mod
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.get_version = mock.Mock(return_value=(2, 3, 9))
|
||||
|
||||
self.config.enhance("encryption-example.demo", "redirect")
|
||||
self.assertRaises(
|
||||
errors.PluginEnhancementAlreadyPresent,
|
||||
self.config.enhance, "encryption-example.demo", "redirect")
|
||||
|
||||
def test_unknown_rewrite(self):
|
||||
# Skip the enable mod
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.parser.add_dir(
|
||||
self.vh_truth[3].path, "RewriteRule", ["Unknown"])
|
||||
self.config.save()
|
||||
self.assertRaises(
|
||||
errors.PluginError,
|
||||
self.config.enhance, "letsencrypt.demo", "redirect")
|
||||
|
||||
def test_unknown_rewrite2(self):
|
||||
# Skip the enable mod
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.parser.add_dir(
|
||||
self.vh_truth[3].path, "RewriteRule", ["Unknown", "2", "3"])
|
||||
self.config.save()
|
||||
self.assertRaises(
|
||||
errors.PluginError,
|
||||
self.config.enhance, "letsencrypt.demo", "redirect")
|
||||
|
||||
def test_unknown_redirect(self):
|
||||
# Skip the enable mod
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.parser.add_dir(
|
||||
self.vh_truth[3].path, "Redirect", ["Unknown"])
|
||||
self.config.save()
|
||||
self.assertRaises(
|
||||
errors.PluginError,
|
||||
self.config.enhance, "letsencrypt.demo", "redirect")
|
||||
|
||||
def test_create_own_redirect(self):
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.get_version = mock.Mock(return_value=(2, 3, 9))
|
||||
# For full testing... give names...
|
||||
self.vh_truth[1].name = "default.com"
|
||||
self.vh_truth[1].aliases = set(["yes.default.com"])
|
||||
@@ -818,6 +855,17 @@ class TwoVhost80Test(util.ApacheTest):
|
||||
self.config._enable_redirect(self.vh_truth[1], "") # pylint: disable=protected-access
|
||||
self.assertEqual(len(self.config.vhosts), 7)
|
||||
|
||||
def test_create_own_redirect_for_old_apache_version(self):
|
||||
self.config.parser.modules.add("rewrite_module")
|
||||
self.config.get_version = mock.Mock(return_value=(2, 2))
|
||||
# For full testing... give names...
|
||||
self.vh_truth[1].name = "default.com"
|
||||
self.vh_truth[1].aliases = set(["yes.default.com"])
|
||||
|
||||
self.config._enable_redirect(self.vh_truth[1], "") # pylint: disable=protected-access
|
||||
self.assertEqual(len(self.config.vhosts), 7)
|
||||
|
||||
|
||||
def get_achalls(self):
|
||||
"""Return testing achallenges."""
|
||||
account_key = self.rsa512jwk
|
||||
|
||||
Reference in New Issue
Block a user