From ffe32c6ca40c493b70b024ee904523b25daabb37 Mon Sep 17 00:00:00 2001 From: sagi Date: Sun, 8 Nov 2015 15:21:36 +0000 Subject: [PATCH] Add tests and comments --- .../letsencrypt_apache/configurator.py | 30 ++++++++----------- .../letsencrypt_apache/constants.py | 6 ++-- .../tests/configurator_test.py | 27 +++++------------ 3 files changed, 24 insertions(+), 39 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 9319d8022..e3adfa927 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -710,28 +710,25 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise def _set_http_header(self, ssl_vhost, header_name): - # TODO REWRITE COMMENT - """Enables the HSTS header on all HTTP responses. + """Enables header header_name on ssl_vhost. - .. note:: HSTS defends against SSL stripping attacks. - - - Adds the Strict-Transport-Security header with max-age=31536000 (1 year) - and includeSubDomains (all subdomains are also set with HSTS). + If header_name is not already set, a new Header directive is placed in + ssl_vhost's configuration with arguments from: + constants.HTTP_HEADER[header_name] .. note:: This function saves the configuration :param ssl_vhost: Destination of traffic, an ssl enabled vhost :type ssl_vhost: :class:`~letsencrypt_apache.obj.VirtualHost` - :param unused_options: Not currently used - :type unused_options: Not Available + :param header_name: a header name, e.g: Strict-Transport-Security + :type str :returns: Success, general_vhost (HTTP vhost) :rtype: (bool, :class:`~letsencrypt_apache.obj.VirtualHost`) :raises .errors.PluginError: If no viable HTTP host can be created or - used for the HSTS. + set with header header_name. """ if "headers_module" not in self.parser.modules: @@ -744,7 +741,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser.add_dir(ssl_vhost.path, "Header", constants.HEADER_ARGS[header_name]) - self.save_notes += ("Adding %s header to ssl vhost in %s\n" % + self.save_notes += ("Adding %s header to ssl vhost in %s\n" % (header_name, ssl_vhost.filep)) self.save() @@ -752,10 +749,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ssl_vhost.filep) def _verify_no_http_header(self, ssl_vhost, header_name): - # TODO revise comment - """Checks to see if existing HSTS settings is in place. + """Checks to see if existing header_name header is in place. - Checks to see if virtualhost already contains a HSTS header + Checks to see if virtualhost already contains a header_name header :param vhost: vhost to check :type vhost: :class:`~letsencrypt_apache.obj.VirtualHost` @@ -763,17 +759,17 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :returns: boolean :rtype: (bool) - :raises errors.PluginError: When an HSTS header exists + :raises errors.PluginError: When header header_name exists """ header_path = self.parser.find_dir("Header", None, start=ssl_vhost.path) if header_path: # "Existing Header directive for virtualhost" for match in header_path: - if self.aug.get(match) == header_name.lower(): + if self.aug.get(match).lower() == header_name.lower(): raise errors.PluginError("Existing %s header" % (header_name)) - + def _enable_redirect(self, ssl_vhost, unused_options): """Redirect all equivalent HTTP traffic to ssl_vhost. diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 63f67fc91..813eae582 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -34,8 +34,8 @@ HSTS_ARGS = ["always", "set", "Strict-Transport-Security", """Apache header arguments for HSTS""" UIR_ARGS = ["always", "set", "Content-Security-Policy", - "upgrade-insecure-requests"] + "upgrade-insecure-requests"] -HEADER_ARGS = {"Strict-Transport-Security" : HSTS_ARGS, - "Upgrade-Insecure-Requests" : UIR_ARGS} +HEADER_ARGS = {"Strict-Transport-Security": HSTS_ARGS, + "Upgrade-Insecure-Requests": UIR_ARGS} diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 3832cf13e..aa224d1b6 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -15,7 +15,6 @@ from letsencrypt import errors from letsencrypt.tests import acme_util from letsencrypt_apache import configurator -from letsencrypt_apache import constants from letsencrypt_apache import obj from letsencrypt_apache.tests import util @@ -532,29 +531,19 @@ class TwoVhost80Test(util.ApacheTest): # four args to HSTS header self.assertEqual(len(hsts_header), 4) - @mock.patch("letsencrypt.le_util.run_script") - @mock.patch("letsencrypt.le_util.exe_exists") - def test_http_header_hsts_with_conflict(self, mock_exe, _): - mock_exe.return_value = True - self.config.parser.update_runtime_variables = mock.Mock() + def test_http_header_hsts_twice(self): self.config.parser.modules.add("mod_ssl.c") - - ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[3]) - self.config.parser.add_dir( - ssl_vhost.path, "Header", constants.HEADER_ARGS[ - "Strict-Transport-Security"]) + # skip the enable mod + self.config.parser.modules.add("headers_module") # This will create an ssl vhost for letsencrypt.demo - self.config.enhance(self.vh_truth[3].name, "http-header", + self.config.enhance("encryption-example.demo", "http-header", "Strict-Transport-Security") - # These are not immediately available in find_dir even with save() and - # load(). They must be found in sites-available - hsts_header = self.config.parser.find_dir( - "Header", None, ssl_vhost.path) - - # four args to HSTS header - self.assertEqual(len(hsts_header), 4) + self.assertRaises( + errors.PluginError, + self.config.enhance, "encryption-example.demo", "http-header", + "Strict-Transport-Security") @mock.patch("letsencrypt.le_util.run_script")