From 0473c67c4808a8f2a6ed3aa083b94040b67b75a8 Mon Sep 17 00:00:00 2001 From: sagi Date: Fri, 6 Nov 2015 22:31:30 +0000 Subject: [PATCH 01/26] Add HSTS header enhancement to Apache --- .../letsencrypt_apache/configurator.py | 70 ++++++++++++++++++- .../letsencrypt_apache/constants.py | 6 ++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d376fe4b6..ea6f80fae 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -122,7 +122,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser = None self.version = version self.vhosts = None - self._enhance_func = {"redirect": self._enable_redirect} + self._enhance_func = {"redirect": self._enable_redirect, + "hsts": self._enable_hsts} @property def mod_ssl_conf(self): @@ -681,7 +682,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ############################################################################ def supported_enhancements(self): # pylint: disable=no-self-use """Returns currently supported enhancements.""" - return ["redirect"] + return ["redirect", "hsts"] def enhance(self, domain, enhancement, options=None): """Enhance configuration. @@ -708,6 +709,71 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.warn("Failed %s for %s", enhancement, domain) raise + def _enable_hsts(self, ssl_vhost, unused_options): + """Enables the HSTS header on all HTTP responses. + + .. 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). + + .. 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 + + :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. + + """ + if "headers_module" not in self.parser.modules: + self.enable_mod("headers") + + # Check if HSTS header is already set + self._verify_no_hsts_header(ssl_vhost) + + # Add directives to server + self.parser.add_dir(ssl_vhost.path, "Header", constants.HSTS_ARGS) + self.save_notes += ("Adding HSTS header to every response from ssl " + "vhost in %s\n" % (ssl_vhost.filep)) + self.save() + logger.info("Adding HSTS header to every response from ssl vhost in %s", + ssl_vhost.filep) + + def _verify_no_hsts_header(self, ssl_vhost): + """Checks to see if existing HSTS settings is in place. + + Checks to see if virtualhost already contains a HSTS header + + :param vhost: vhost to check + :type vhost: :class:`~letsencrypt_apache.obj.VirtualHost` + + :returns: boolean + :rtype: (bool) + + :raises errors.PluginError: When an HSTS header 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 match == "Strict-Transport-Security": + raise errors.PluginError("Existing HSTS header") + + for match, arg in itertools.izip(header_path, constants.HSTS_ARGS): + if self.aug.get(match) != arg: + raise errors.PluginError("Unknown Existing HSTS header") + raise errors.PluginError("Let's Encrypt has already enabled HSTS") + + 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 1c17eacc3..05e1bb0e7 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -27,3 +27,9 @@ 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""" + +HSTS_ARGS = [ + "always", "set", "Strict-Transport-Security", + "\"max-age=31536000; includeSubDomains\""] +"""Apache header arguments for HSTS""" + From 93e2023f871927636077e59e026c8abaaf8b0369 Mon Sep 17 00:00:00 2001 From: sagi Date: Fri, 6 Nov 2015 22:32:02 +0000 Subject: [PATCH 02/26] Add HSTS enhancement basic tests --- .../tests/configurator_test.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 7c2137c45..adb96c2cd 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -507,6 +507,29 @@ class TwoVhost80Test(util.ApacheTest): errors.PluginError, self.config.enhance, "letsencrypt.demo", "unknown_enhancement") + @mock.patch("letsencrypt.le_util.run_script") + @mock.patch("letsencrypt.le_util.exe_exists") + def test_hsts(self, mock_exe, _): + self.config.parser.update_runtime_variables = mock.Mock() + self.config.parser.modules.add("mod_ssl.c") + mock_exe.return_value = True + + # This will create an ssl vhost for letsencrypt.demo + self.config.enhance("letsencrypt.demo", "hsts") + + self.assertTrue("headers_module" in self.config.parser.modules) + + # Get the ssl vhost for letsencrypt.demo + ssl_vhost = self.config.assoc["letsencrypt.demo"] + + # 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) + + @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") def test_redirect_well_formed_http(self, mock_exe, _): From 2988a09087b2e350185e1384558ede32da4b178b Mon Sep 17 00:00:00 2001 From: sagi Date: Sat, 7 Nov 2015 05:24:55 +0000 Subject: [PATCH 03/26] Make lint happy, delete trailing whitespaces --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 ++++---- letsencrypt-apache/letsencrypt_apache/constants.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ea6f80fae..d8157c33a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -710,7 +710,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise def _enable_hsts(self, ssl_vhost, unused_options): - """Enables the HSTS header on all HTTP responses. + """Enables the HSTS header on all HTTP responses. .. note:: HSTS defends against SSL stripping attacks. @@ -735,10 +735,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ if "headers_module" not in self.parser.modules: self.enable_mod("headers") - + # Check if HSTS header is already set self._verify_no_hsts_header(ssl_vhost) - + # Add directives to server self.parser.add_dir(ssl_vhost.path, "Header", constants.HSTS_ARGS) self.save_notes += ("Adding HSTS header to every response from ssl " @@ -750,7 +750,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def _verify_no_hsts_header(self, ssl_vhost): """Checks to see if existing HSTS settings is in place. - Checks to see if virtualhost already contains a HSTS header + Checks to see if virtualhost already contains a HSTS header :param vhost: vhost to check :type vhost: :class:`~letsencrypt_apache.obj.VirtualHost` diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 05e1bb0e7..dac796c52 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -28,8 +28,8 @@ REWRITE_HTTPS_ARGS = [ "^", "https://%{SERVER_NAME}%{REQUEST_URI}", "[L,QSA,R=permanent]"] """Apache rewrite rule arguments used for redirections to https vhost""" -HSTS_ARGS = [ - "always", "set", "Strict-Transport-Security", +HSTS_ARGS = [ + "always", "set", "Strict-Transport-Security", "\"max-age=31536000; includeSubDomains\""] """Apache header arguments for HSTS""" From 04136cfbf29223c6c1b8ef5ed9d5dcef7eef832d Mon Sep 17 00:00:00 2001 From: sagi Date: Sun, 8 Nov 2015 04:37:57 +0000 Subject: [PATCH 04/26] Generalized http-header enhancement --- .../letsencrypt_apache/configurator.py | 38 +++++++++---------- .../letsencrypt_apache/constants.py | 10 ++++- .../tests/configurator_test.py | 31 ++++++++++++++- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d8157c33a..9319d8022 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -123,7 +123,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.version = version self.vhosts = None self._enhance_func = {"redirect": self._enable_redirect, - "hsts": self._enable_hsts} + "http-header": self._set_http_header} @property def mod_ssl_conf(self): @@ -682,7 +682,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ############################################################################ def supported_enhancements(self): # pylint: disable=no-self-use """Returns currently supported enhancements.""" - return ["redirect", "hsts"] + return ["redirect", "http-header"] def enhance(self, domain, enhancement, options=None): """Enhance configuration. @@ -709,7 +709,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.warn("Failed %s for %s", enhancement, domain) raise - def _enable_hsts(self, ssl_vhost, unused_options): + def _set_http_header(self, ssl_vhost, header_name): + # TODO REWRITE COMMENT """Enables the HSTS header on all HTTP responses. .. note:: HSTS defends against SSL stripping attacks. @@ -736,18 +737,22 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if "headers_module" not in self.parser.modules: self.enable_mod("headers") - # Check if HSTS header is already set - self._verify_no_hsts_header(ssl_vhost) + # Check if selected header is already set + self._verify_no_http_header(ssl_vhost, header_name) # Add directives to server - self.parser.add_dir(ssl_vhost.path, "Header", constants.HSTS_ARGS) - self.save_notes += ("Adding HSTS header to every response from ssl " - "vhost in %s\n" % (ssl_vhost.filep)) + 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" % + (header_name, ssl_vhost.filep)) + self.save() - logger.info("Adding HSTS header to every response from ssl vhost in %s", + logger.info("Adding %s header to ssl vhost in %s", header_name, ssl_vhost.filep) - def _verify_no_hsts_header(self, ssl_vhost): + 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 virtualhost already contains a HSTS header @@ -765,15 +770,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if header_path: # "Existing Header directive for virtualhost" for match in header_path: - if match == "Strict-Transport-Security": - raise errors.PluginError("Existing HSTS header") - - for match, arg in itertools.izip(header_path, constants.HSTS_ARGS): - if self.aug.get(match) != arg: - raise errors.PluginError("Unknown Existing HSTS header") - raise errors.PluginError("Let's Encrypt has already enabled HSTS") - - + if self.aug.get(match) == 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 dac796c52..63f67fc91 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -28,8 +28,14 @@ REWRITE_HTTPS_ARGS = [ "^", "https://%{SERVER_NAME}%{REQUEST_URI}", "[L,QSA,R=permanent]"] """Apache rewrite rule arguments used for redirections to https vhost""" -HSTS_ARGS = [ - "always", "set", "Strict-Transport-Security", + +HSTS_ARGS = ["always", "set", "Strict-Transport-Security", "\"max-age=31536000; includeSubDomains\""] """Apache header arguments for HSTS""" +UIR_ARGS = ["always", "set", "Content-Security-Policy", + "upgrade-insecure-requests"] + +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 adb96c2cd..3832cf13e 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -15,6 +15,7 @@ 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 @@ -509,13 +510,14 @@ class TwoVhost80Test(util.ApacheTest): @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") - def test_hsts(self, mock_exe, _): + def test_http_header_hsts(self, mock_exe, _): self.config.parser.update_runtime_variables = mock.Mock() self.config.parser.modules.add("mod_ssl.c") mock_exe.return_value = True # This will create an ssl vhost for letsencrypt.demo - self.config.enhance("letsencrypt.demo", "hsts") + self.config.enhance("letsencrypt.demo", "http-header", + "Strict-Transport-Security") self.assertTrue("headers_module" in self.config.parser.modules) @@ -526,6 +528,31 @@ class TwoVhost80Test(util.ApacheTest): # 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) + + @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() + 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"]) + + # This will create an ssl vhost for letsencrypt.demo + self.config.enhance(self.vh_truth[3].name, "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) From ffe32c6ca40c493b70b024ee904523b25daabb37 Mon Sep 17 00:00:00 2001 From: sagi Date: Sun, 8 Nov 2015 15:21:36 +0000 Subject: [PATCH 05/26] 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") From de338c7309bb31244274b1e4f63b59f1f9b72c09 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 9 Nov 2015 22:36:00 +0000 Subject: [PATCH 06/26] Add tests for Upgrade-Insecure-Requests --- .../tests/configurator_test.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index aa224d1b6..4dd1350ac 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -545,6 +545,45 @@ class TwoVhost80Test(util.ApacheTest): self.config.enhance, "encryption-example.demo", "http-header", "Strict-Transport-Security") + @mock.patch("letsencrypt.le_util.run_script") + @mock.patch("letsencrypt.le_util.exe_exists") + def test_http_header_uir(self, mock_exe, _): + self.config.parser.update_runtime_variables = mock.Mock() + self.config.parser.modules.add("mod_ssl.c") + mock_exe.return_value = True + + # This will create an ssl vhost for letsencrypt.demo + self.config.enhance("letsencrypt.demo", "http-header", + "Upgrade-Insecure-Requests") + + self.assertTrue("headers_module" in self.config.parser.modules) + + # Get the ssl vhost for letsencrypt.demo + ssl_vhost = self.config.assoc["letsencrypt.demo"] + + # These are not immediately available in find_dir even with save() and + # load(). They must be found in sites-available + uir_header = self.config.parser.find_dir( + "Header", None, ssl_vhost.path) + + # four args to HSTS header + self.assertEqual(len(uir_header), 4) + + def test_http_header_uir_twice(self): + self.config.parser.modules.add("mod_ssl.c") + # skip the enable mod + self.config.parser.modules.add("headers_module") + + # This will create an ssl vhost for letsencrypt.demo + self.config.enhance("encryption-example.demo", "http-header", + "Upgrade-Insecure-Requests") + + self.assertRaises( + errors.PluginError, + self.config.enhance, "encryption-example.demo", "http-header", + "Upgrade-Insecure-Requests") + + @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") From 188068906554b213853914cf27fb2875b7220e96 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 10 Nov 2015 06:41:59 +0000 Subject: [PATCH 07/26] Add --hsts and --uir CLI flags --- letsencrypt/cli.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5757783cd..1c08d27f6 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -868,6 +868,16 @@ def prepare_and_parse_args(plugins, args): "security", "--no-redirect", action="store_false", help="Do not automatically redirect all HTTP traffic to HTTPS for the newly " "authenticated vhost.", dest="redirect", default=None) + helpful.add( + "security", "--hsts", action="store_true", + help="Add the Strict-Transport-Security header to every HTTP response." + " Forcing browser to use always use SSL for the domain." + " Defends against SSL Stripping.", dest="hsts") + helpful.add( + "security", "--uir", action="store_true", + help="Add the \"Content-Security-Policy: upgrade-insecure-requests\"" + " header to every HTTP response. Forcing the browser to use" + " https:// for every http:// resource.", dest="uir") helpful.add( "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " From 9ad38e9b37b57a542b7070396bef4c3d7985167b Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 11 Nov 2015 19:04:07 +0000 Subject: [PATCH 08/26] Pass args to enhance_config instead of just a redirect flag --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1c08d27f6..e33c0770e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -453,7 +453,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo domains, lineage.privkey, lineage.cert, lineage.chain, lineage.fullchain) - le_client.enhance_config(domains, args.redirect) + le_client.enhance_config(domains, args) if len(lineage.available_versions("cert")) == 1: display_ops.success_installation(domains) @@ -507,7 +507,7 @@ def install(args, config, plugins): le_client.deploy_certificate( domains, args.key_path, args.cert_path, args.chain_path, args.fullchain_path) - le_client.enhance_config(domains, args.redirect) + le_client.enhance_config(domains, args) def revoke(args, config, unused_plugins): # TODO: coop with renewal config From 17ef874c04be603bbf3db88bf90d8e4ad0929db1 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 12 Nov 2015 02:15:42 +0000 Subject: [PATCH 09/26] change args to config in enhance_config --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e33c0770e..36780d2bb 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -453,7 +453,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo domains, lineage.privkey, lineage.cert, lineage.chain, lineage.fullchain) - le_client.enhance_config(domains, args) + le_client.enhance_config(domains, config) if len(lineage.available_versions("cert")) == 1: display_ops.success_installation(domains) @@ -507,7 +507,7 @@ def install(args, config, plugins): le_client.deploy_certificate( domains, args.key_path, args.cert_path, args.chain_path, args.fullchain_path) - le_client.enhance_config(domains, args) + le_client.enhance_config(domains, config) def revoke(args, config, unused_plugins): # TODO: coop with renewal config From e787147eea3c533a43cf0200ccd00a65e2b87846 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 12 Nov 2015 02:24:57 +0000 Subject: [PATCH 10/26] dissect namespace config in enhance_config --- letsencrypt/client.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 8a0ad6af4..bf99a55dd 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -350,7 +350,7 @@ class Client(object): logger.critical("Rollback successful; your server has " "been restarted with your old configuration") - def enhance_config(self, domains, redirect=None): + def enhance_config(self, domains, config=None): """Enhance the configuration. .. todo:: This needs to handle the specific enhancements offered by the @@ -359,6 +359,11 @@ class Client(object): :param list domains: list of domains to configure + :ivar namespace: Namespace typically produced by + :meth:`argparse.ArgumentParser.parse_args`. + :type namespace: :class:`argparse.Namespace` + + :param redirect: If traffic should be forwarded from HTTP to HTTPS. :type redirect: bool or None @@ -371,7 +376,7 @@ class Client(object): "configuration to enhance.") raise errors.Error("No installer available") - if redirect is None: + if config.redirect is None: redirect = enhancements.ask("redirect") # When support for more enhancements are added, the call to the From 68d956f6594124591e890f7f28890900de6c7792 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 12 Nov 2015 03:04:23 +0000 Subject: [PATCH 11/26] make redirect work again --- letsencrypt/client.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index bf99a55dd..2b19176c2 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -371,12 +371,14 @@ class Client(object): client. """ + redirect = config.redirect + if self.installer is None: logger.warning("No installer is specified, there isn't any " "configuration to enhance.") raise errors.Error("No installer available") - if config.redirect is None: + if redirect is None: redirect = enhancements.ask("redirect") # When support for more enhancements are added, the call to the From b1e3c89048c458287df29fcf2fd596bb53d402e4 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 12 Nov 2015 04:49:31 +0000 Subject: [PATCH 12/26] add a general apply_enhancement to replace redirect_to_ssl --- letsencrypt/client.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 2b19176c2..aa1718def 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -353,25 +353,19 @@ class Client(object): def enhance_config(self, domains, config=None): """Enhance the configuration. - .. todo:: This needs to handle the specific enhancements offered by the - installer. We will also have to find a method to pass in the chosen - values efficiently. - :param list domains: list of domains to configure :ivar namespace: Namespace typically produced by :meth:`argparse.ArgumentParser.parse_args`. :type namespace: :class:`argparse.Namespace` - - :param redirect: If traffic should be forwarded from HTTP to HTTPS. - :type redirect: bool or None - :raises .errors.Error: if no installer is specified in the client. """ redirect = config.redirect + hsts = config.hsts + uir = config.uir # Upgrade Insecure Requests if self.installer is None: logger.warning("No installer is specified, there isn't any " @@ -381,27 +375,27 @@ class Client(object): if redirect is None: redirect = enhancements.ask("redirect") - # When support for more enhancements are added, the call to the - # plugin's `enhance` function should be wrapped by an ErrorHandler if redirect: - self.redirect_to_ssl(domains) + self.apply_enhancement(domains, "redirect") - def redirect_to_ssl(self, domains): + def apply_enhancement(self, domains, enhancement, options=None): + # TODO change comment """Redirect all traffic from HTTP to HTTPS - :param vhost: list of ssl_vhosts - :type vhost: :class:`letsencrypt.interfaces.IInstaller` + :param domains: list of ssl_vhosts + :type str """ with error_handler.ErrorHandler(self.installer.recovery_routine): for dom in domains: try: - self.installer.enhance(dom, "redirect") + self.installer.enhance(dom, enhancement) except errors.PluginError: - logger.warn("Unable to perform redirect for %s", dom) + logger.warn("Unable to set enhancement %s for %s", + enhancement, dom) raise - self.installer.save("Add Redirects") + self.installer.save("Add enhancement %s" % (enhancement)) self.installer.restart() From 8185ea931c869ee5f916a6f7f7a45c3eb6bf6f12 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 12 Nov 2015 05:08:30 +0000 Subject: [PATCH 13/26] make hsts and uri cli args actually work --- letsencrypt/client.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index aa1718def..81de32bbe 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -367,6 +367,7 @@ class Client(object): hsts = config.hsts uir = config.uir # Upgrade Insecure Requests + if self.installer is None: logger.warning("No installer is specified, there isn't any " "configuration to enhance.") @@ -378,6 +379,16 @@ class Client(object): if redirect: self.apply_enhancement(domains, "redirect") + if hsts: + self.apply_enhancement(domains, "http-header", + "Strict-Transport-Security") + if uir: + self.apply_enhancement(domains, "http-header", + "Upgrade-Insecure-Requests") + + if (redirect or hsts or uir): + self.installer.restart() + def apply_enhancement(self, domains, enhancement, options=None): # TODO change comment """Redirect all traffic from HTTP to HTTPS @@ -389,14 +400,13 @@ class Client(object): with error_handler.ErrorHandler(self.installer.recovery_routine): for dom in domains: try: - self.installer.enhance(dom, enhancement) + self.installer.enhance(dom, enhancement, options) except errors.PluginError: logger.warn("Unable to set enhancement %s for %s", enhancement, dom) raise self.installer.save("Add enhancement %s" % (enhancement)) - self.installer.restart() def validate_key_csr(privkey, csr=None): From 796eef802d7ccdd70c03192220c0c58979701cb7 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 12 Nov 2015 05:20:10 +0000 Subject: [PATCH 14/26] add apply_enhancement comment --- letsencrypt/client.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 81de32bbe..65098bc18 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -390,11 +390,17 @@ class Client(object): self.installer.restart() def apply_enhancement(self, domains, enhancement, options=None): - # TODO change comment - """Redirect all traffic from HTTP to HTTPS + """Applies an enhacement on all domains. :param domains: list of ssl_vhosts - :type str + :type list of str + + :param enhancement: name of enhancement, e.g. http-header + :type str + + .. note:: when more options are need make options a list. + :param options: options to enhancement, e.g. Strict-Transport-Security + :type str """ with error_handler.ErrorHandler(self.installer.recovery_routine): From b76ef3a293d33e0481736f33b141c7715c8476b8 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 12 Nov 2015 05:25:44 +0000 Subject: [PATCH 15/26] make lint happy --- letsencrypt/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 65098bc18..53874b7dd 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -386,14 +386,14 @@ class Client(object): self.apply_enhancement(domains, "http-header", "Upgrade-Insecure-Requests") - if (redirect or hsts or uir): + if redirect or hsts or uir: self.installer.restart() def apply_enhancement(self, domains, enhancement, options=None): - """Applies an enhacement on all domains. + """Applies an enhacement on all domains. :param domains: list of ssl_vhosts - :type list of str + :type list of str :param enhancement: name of enhancement, e.g. http-header :type str From ddf5b28f7db5f2fd312f2a9e6a901f3bd9a8e6f3 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 16 Nov 2015 20:06:16 +0000 Subject: [PATCH 16/26] fix tests and make linter happy --- letsencrypt/client.py | 36 ++++++++++++++++++-------------- letsencrypt/tests/client_test.py | 28 ++++++++++++++++++++----- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e009400d2..cc1f2aadb 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -359,7 +359,7 @@ class Client(object): :param list domains: list of domains to configure - :ivar namespace: Namespace typically produced by + :ivar config: Namespace typically produced by :meth:`argparse.ArgumentParser.parse_args`. :type namespace: :class:`argparse.Namespace` @@ -367,29 +367,32 @@ class Client(object): client. """ - redirect = config.redirect - hsts = config.hsts - uir = config.uir # Upgrade Insecure Requests - if self.installer is None: logger.warning("No installer is specified, there isn't any " "configuration to enhance.") raise errors.Error("No installer available") - if redirect is None: + if config is None: redirect = enhancements.ask("redirect") + if redirect: + self.apply_enhancement(domains, "redirect") + else: + redirect = config.redirect + hsts = config.hsts + uir = config.uir # Upgrade Insecure Requests - if redirect: - self.apply_enhancement(domains, "redirect") + if redirect: + self.apply_enhancement(domains, "redirect") - if hsts: - self.apply_enhancement(domains, "http-header", - "Strict-Transport-Security") - if uir: - self.apply_enhancement(domains, "http-header", - "Upgrade-Insecure-Requests") + if hsts: + self.apply_enhancement(domains, "http-header", + "Strict-Transport-Security") + if uir: + self.apply_enhancement(domains, "http-header", + "Upgrade-Insecure-Requests") + msg = ("We were unable to restart web server") if redirect or hsts or uir: with error_handler.ErrorHandler(self._rollback_and_restart, msg): self.installer.restart() @@ -408,8 +411,9 @@ class Client(object): :type str """ - msg = ("We were unable to set up a redirect for your server, " - "however, we successfully installed your certificate.") + msg = ("We were unable to set up enhancement %s for your server, " + "however, we successfully installed your certificate." + % (enhancement)) with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg): for dom in domains: try: diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index d396e25bc..8b84fd3e2 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -20,6 +20,15 @@ KEY = test_util.load_vector("rsa512_key.pem") CSR_SAN = test_util.load_vector("csr-san.der") +class ConfigHelper(object): + """Creates a dummy object to imitate a namespace object + + Example: cfg = ConfigHelper(redirect=True, hsts=False, uir=False) + will result in: cfg.redirect=True, cfg.hsts=False, etc. + """ + def __init__(self, **kwds): + self.__dict__.update(kwds) + class RegisterTest(unittest.TestCase): """Tests for letsencrypt.client.register.""" @@ -219,7 +228,7 @@ class ClientTest(unittest.TestCase): self.client.installer = installer self.client.enhance_config(["foo.bar"]) - installer.enhance.assert_called_once_with("foo.bar", "redirect") + installer.enhance.assert_called_once_with("foo.bar", "redirect", None) self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() @@ -236,8 +245,10 @@ class ClientTest(unittest.TestCase): self.client.installer = installer installer.enhance.side_effect = errors.PluginError + config = ConfigHelper(redirect=True, hsts=False, uir=False) + self.assertRaises(errors.PluginError, - self.client.enhance_config, ["foo.bar"], True) + self.client.enhance_config, ["foo.bar"], config) installer.recovery_routine.assert_called_once_with() self.assertEqual(mock_get_utility().add_message.call_count, 1) @@ -250,8 +261,10 @@ class ClientTest(unittest.TestCase): self.client.installer = installer installer.save.side_effect = errors.PluginError + config = ConfigHelper(redirect=True, hsts=False, uir=False) + self.assertRaises(errors.PluginError, - self.client.enhance_config, ["foo.bar"], True) + self.client.enhance_config, ["foo.bar"], config) installer.recovery_routine.assert_called_once_with() self.assertEqual(mock_get_utility().add_message.call_count, 1) @@ -264,8 +277,11 @@ class ClientTest(unittest.TestCase): self.client.installer = installer installer.restart.side_effect = [errors.PluginError, None] + config = ConfigHelper(redirect=True, hsts=False, uir=False) + self.assertRaises(errors.PluginError, - self.client.enhance_config, ["foo.bar"], True) + self.client.enhance_config, ["foo.bar"], config) + self.assertEqual(mock_get_utility().add_message.call_count, 1) installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 2) @@ -280,8 +296,10 @@ class ClientTest(unittest.TestCase): installer.restart.side_effect = errors.PluginError installer.rollback_checkpoints.side_effect = errors.ReverterError + config = ConfigHelper(redirect=True, hsts=False, uir=False) + self.assertRaises(errors.PluginError, - self.client.enhance_config, ["foo.bar"], True) + self.client.enhance_config, ["foo.bar"], config) self.assertEqual(mock_get_utility().add_message.call_count, 1) installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 1) From 1098126b7b28b54b222b458737b0a6bcc6ac04df Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 16 Nov 2015 20:31:49 +0000 Subject: [PATCH 17/26] tests hsts, redirect and uir --- letsencrypt/tests/client_test.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 8b84fd3e2..c66ad1e08 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -232,6 +232,32 @@ class ClientTest(unittest.TestCase): self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() + @mock.patch("letsencrypt.client.enhancements") + def test_enhance_config_no_ask(self, mock_enhancements): + self.assertRaises(errors.Error, + self.client.enhance_config, ["foo.bar"]) + + mock_enhancements.ask.return_value = True + installer = mock.MagicMock() + self.client.installer = installer + + config = ConfigHelper(redirect=True, hsts=False, uir=False) + self.client.enhance_config(["foo.bar"], config) + installer.enhance.assert_called_with("foo.bar", "redirect", None) + + config = ConfigHelper(redirect=False, hsts=True, uir=False) + self.client.enhance_config(["foo.bar"], config) + installer.enhance.assert_called_with("foo.bar", "http-header", + "Strict-Transport-Security") + + config = ConfigHelper(redirect=False, hsts=False, uir=True) + self.client.enhance_config(["foo.bar"], config) + installer.enhance.assert_called_with("foo.bar", "http-header", + "Upgrade-Insecure-Requests") + + self.assertEqual(installer.save.call_count, 3) + self.assertEqual(installer.restart.call_count, 3) + def test_enhance_config_no_installer(self): self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"]) From 17ea7bb316eef6e0cef8785e6e9c79fc06eb987f Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 16 Nov 2015 20:41:39 +0000 Subject: [PATCH 18/26] comment and simplify things --- letsencrypt/cli.py | 4 ++-- letsencrypt/client.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3e8d4d833..8393d6dd0 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -913,12 +913,12 @@ def prepare_and_parse_args(plugins, args): "security", "--hsts", action="store_true", help="Add the Strict-Transport-Security header to every HTTP response." " Forcing browser to use always use SSL for the domain." - " Defends against SSL Stripping.", dest="hsts") + " Defends against SSL Stripping.", dest="hsts", default=False) helpful.add( "security", "--uir", action="store_true", help="Add the \"Content-Security-Policy: upgrade-insecure-requests\"" " header to every HTTP response. Forcing the browser to use" - " https:// for every http:// resource.", dest="uir") + " https:// for every http:// resource.", dest="uir", default=False) helpful.add( "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " diff --git a/letsencrypt/client.py b/letsencrypt/client.py index cc1f2aadb..e3e365bb8 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -410,6 +410,9 @@ class Client(object): :param options: options to enhancement, e.g. Strict-Transport-Security :type str + :raises .errors.Error: if no installer is specified in the + client. + """ msg = ("We were unable to set up enhancement %s for your server, " "however, we successfully installed your certificate." From 58110a69f4820fcf70604c8c260caf03ad498fe8 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 17 Nov 2015 07:23:19 +0000 Subject: [PATCH 19/26] more elegant enhance_config, add --no- flags to hsts and uir --- letsencrypt/cli.py | 11 ++++++++++- letsencrypt/client.py | 33 +++++++++++++++++--------------- letsencrypt/tests/client_test.py | 11 +++++++---- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 8393d6dd0..ce419b393 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -914,11 +914,20 @@ def prepare_and_parse_args(plugins, args): help="Add the Strict-Transport-Security header to every HTTP response." " Forcing browser to use always use SSL for the domain." " Defends against SSL Stripping.", dest="hsts", default=False) + helpful.add( + "security", "--no-hsts", action="store_false", + help="Do not automaticcally add the Strict-Transport-Security header" + " to every HTTP response.", dest="hsts", default=False) helpful.add( "security", "--uir", action="store_true", help="Add the \"Content-Security-Policy: upgrade-insecure-requests\"" " header to every HTTP response. Forcing the browser to use" - " https:// for every http:// resource.", dest="uir", default=False) + " https:// for every http:// resource.", dest="uir", default=None) + helpful.add( + "security", "--no-uir", action="store_false", + help=" Do not automatically set the \"Content-Security-Policy:" + " upgrade-insecure-requests\" header to every HTTP response.", + dest="uir", default=None) helpful.add( "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e3e365bb8..be6fc7a22 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -354,13 +354,14 @@ class Client(object): with error_handler.ErrorHandler(self._rollback_and_restart, msg): # sites may have been enabled / final cleanup self.installer.restart() - def enhance_config(self, domains, config=None): + def enhance_config(self, domains, config): """Enhance the configuration. :param list domains: list of domains to configure :ivar config: Namespace typically produced by :meth:`argparse.ArgumentParser.parse_args`. + it must have the redirect, hsts and uir attributes. :type namespace: :class:`argparse.Namespace` :raises .errors.Error: if no installer is specified in the @@ -374,23 +375,25 @@ class Client(object): raise errors.Error("No installer available") if config is None: + logger.warning("No config is specified.") + raise errors.Error("No config available") + + redirect = config.redirect + hsts = config.hsts + uir = config.uir # Upgrade Insecure Requests + + if redirect is None: redirect = enhancements.ask("redirect") - if redirect: - self.apply_enhancement(domains, "redirect") - else: - redirect = config.redirect - hsts = config.hsts - uir = config.uir # Upgrade Insecure Requests - if redirect: - self.apply_enhancement(domains, "redirect") + if redirect: + self.apply_enhancement(domains, "redirect") - if hsts: - self.apply_enhancement(domains, "http-header", - "Strict-Transport-Security") - if uir: - self.apply_enhancement(domains, "http-header", - "Upgrade-Insecure-Requests") + if hsts: + self.apply_enhancement(domains, "http-header", + "Strict-Transport-Security") + if uir: + self.apply_enhancement(domains, "http-header", + "Upgrade-Insecure-Requests") msg = ("We were unable to restart web server") if redirect or hsts or uir: diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index c66ad1e08..4208027aa 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -220,22 +220,24 @@ class ClientTest(unittest.TestCase): @mock.patch("letsencrypt.client.enhancements") def test_enhance_config(self, mock_enhancements): + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.Error, - self.client.enhance_config, ["foo.bar"]) + self.client.enhance_config, ["foo.bar"], config) mock_enhancements.ask.return_value = True installer = mock.MagicMock() self.client.installer = installer - self.client.enhance_config(["foo.bar"]) + self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_called_once_with("foo.bar", "redirect", None) self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() @mock.patch("letsencrypt.client.enhancements") def test_enhance_config_no_ask(self, mock_enhancements): + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.Error, - self.client.enhance_config, ["foo.bar"]) + self.client.enhance_config, ["foo.bar"], config) mock_enhancements.ask.return_value = True installer = mock.MagicMock() @@ -259,8 +261,9 @@ class ClientTest(unittest.TestCase): self.assertEqual(installer.restart.call_count, 3) def test_enhance_config_no_installer(self): + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.Error, - self.client.enhance_config, ["foo.bar"]) + self.client.enhance_config, ["foo.bar"], config) @mock.patch("letsencrypt.client.zope.component.getUtility") @mock.patch("letsencrypt.client.enhancements") From eb5e345c3ec25d82d7bb519e2d6fc82448dc6433 Mon Sep 17 00:00:00 2001 From: sagi Date: Sun, 22 Nov 2015 18:40:19 +0000 Subject: [PATCH 20/26] change vhost to ssl_vhost, add header_name explanation in comments. --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 189af25e0..b3b5df392 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -772,9 +772,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): Checks to see if virtualhost already contains a header_name header - :param vhost: vhost to check + :param ssl_vhost: vhost to check :type vhost: :class:`~letsencrypt_apache.obj.VirtualHost` + :param header_name: a header name, e.g: Strict-Transport-Security + :type str + :returns: boolean :rtype: (bool) From f8a32160820a4fb0886c5091a4c825f9905a5bad Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 23 Nov 2015 20:11:47 +0000 Subject: [PATCH 21/26] change header_name to header_substring --- .../letsencrypt_apache/configurator.py | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index b3b5df392..65e759061 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -728,69 +728,69 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.warn("Failed %s for %s", enhancement, domain) raise - def _set_http_header(self, ssl_vhost, header_name): - """Enables header header_name on ssl_vhost. + def _set_http_header(self, ssl_vhost, header_substring): + """Enables header that is identified by header_substring on ssl_vhost. - 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] + If the header identified by header_substring is not already set, + a new Header directive is placed in ssl_vhost's configuration with + arguments from: constants.HTTP_HEADER[header_substring] .. 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 header_name: a header name, e.g: Strict-Transport-Security + :param header_substring: string that uniquely identifies a header. + e.g: Strict-Transport-Security, Upgrade-Insecure-Requests. :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 - set with header header_name. + set with header header_substring. """ if "headers_module" not in self.parser.modules: self.enable_mod("headers") # Check if selected header is already set - self._verify_no_http_header(ssl_vhost, header_name) + self._verify_no_http_header(ssl_vhost, header_substring) # Add directives to server self.parser.add_dir(ssl_vhost.path, "Header", - constants.HEADER_ARGS[header_name]) + constants.HEADER_ARGS[header_substring]) self.save_notes += ("Adding %s header to ssl vhost in %s\n" % - (header_name, ssl_vhost.filep)) + (header_substring, ssl_vhost.filep)) self.save() - logger.info("Adding %s header to ssl vhost in %s", header_name, + logger.info("Adding %s header to ssl vhost in %s", header_substring, ssl_vhost.filep) - def _verify_no_http_header(self, ssl_vhost, header_name): - """Checks to see if existing header_name header is in place. - - Checks to see if virtualhost already contains a header_name header + def _verify_no_http_header(self, ssl_vhost, header_substring): + """Checks to see if an there is an existing Header directive that + contains the string header_substring. :param ssl_vhost: vhost to check :type vhost: :class:`~letsencrypt_apache.obj.VirtualHost` - :param header_name: a header name, e.g: Strict-Transport-Security + :param header_substring: a header name, e.g: Strict-Transport-Security :type str :returns: boolean :rtype: (bool) - :raises errors.PluginError: When header header_name exists + :raises errors.PluginError: When header header_substring 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).lower() == header_name.lower(): + if self.aug.get(match).lower() == header_substring.lower(): raise errors.PluginError("Existing %s header" % - (header_name)) + (header_substring)) def _enable_redirect(self, ssl_vhost, unused_options): """Redirect all equivalent HTTP traffic to ssl_vhost. From b75354add00bca1ff5bb074922e1d4893a2c10dd Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 23 Nov 2015 20:13:08 +0000 Subject: [PATCH 22/26] change verify_no_http_header to verify_no_matching_http_header --- letsencrypt-apache/letsencrypt_apache/configurator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 65e759061..e5e4edc80 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -755,7 +755,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.enable_mod("headers") # Check if selected header is already set - self._verify_no_http_header(ssl_vhost, header_substring) + self._verify_no_matching_http_header(ssl_vhost, header_substring) # Add directives to server self.parser.add_dir(ssl_vhost.path, "Header", @@ -768,7 +768,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.info("Adding %s header to ssl vhost in %s", header_substring, ssl_vhost.filep) - def _verify_no_http_header(self, ssl_vhost, header_substring): + def _verify_no_matching_http_header(self, ssl_vhost, header_substring): """Checks to see if an there is an existing Header directive that contains the string header_substring. From 7df7228a531daa6963909e4b2ade58f2f1a019c1 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 23 Nov 2015 22:41:02 +0000 Subject: [PATCH 23/26] add regex to detect header_substring in header directive definition --- letsencrypt-apache/letsencrypt_apache/configurator.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index e5e4edc80..6ef1fbee2 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -775,7 +775,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param ssl_vhost: vhost to check :type vhost: :class:`~letsencrypt_apache.obj.VirtualHost` - :param header_substring: a header name, e.g: Strict-Transport-Security + :param header_substring: string that uniquely identifies a header. + e.g: Strict-Transport-Security, Upgrade-Insecure-Requests. :type str :returns: boolean @@ -787,8 +788,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): header_path = self.parser.find_dir("Header", None, start=ssl_vhost.path) if header_path: # "Existing Header directive for virtualhost" + pat = '(?:[ "]|^)(%s)(?:[ "]|$)' % (header_substring.lower()) for match in header_path: - if self.aug.get(match).lower() == header_substring.lower(): + if re.search(pat, self.aug.get(match).lower()): raise errors.PluginError("Existing %s header" % (header_substring)) From 72fcee42649f643fa9486a41524d92f339359e82 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 23 Nov 2015 23:58:58 +0000 Subject: [PATCH 24/26] change Error to PluginError in comment --- letsencrypt/client.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e229d3c8d..e1b0c4b84 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -442,8 +442,9 @@ class Client(object): :param options: options to enhancement, e.g. Strict-Transport-Security :type str - :raises .errors.Error: if no installer is specified in the - client. + :raises .errors.PluginError: If Enhancement is not supported, or if + there is any other problem with the enhancement. + """ msg = ("We were unable to set up enhancement %s for your server, " From 7467496984b444047866831240d8ba25b67102e7 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 24 Nov 2015 23:33:21 +0000 Subject: [PATCH 25/26] change enhancement http-header to ensure-http-header --- .../letsencrypt_apache/configurator.py | 4 ++-- .../letsencrypt_apache/tests/configurator_test.py | 12 ++++++------ letsencrypt/client.py | 6 +++--- letsencrypt/tests/client_test.py | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 6ef1fbee2..4b66c5c6f 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -122,7 +122,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.version = version self.vhosts = None self._enhance_func = {"redirect": self._enable_redirect, - "http-header": self._set_http_header} + "ensure-http-header": self._set_http_header} @property def mod_ssl_conf(self): @@ -701,7 +701,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ############################################################################ def supported_enhancements(self): # pylint: disable=no-self-use """Returns currently supported enhancements.""" - return ["redirect", "http-header"] + return ["redirect", "ensure-http-header"] def enhance(self, domain, enhancement, options=None): """Enhance configuration. diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 36a3f13fa..8eb1e16e2 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -521,7 +521,7 @@ class TwoVhost80Test(util.ApacheTest): mock_exe.return_value = True # This will create an ssl vhost for letsencrypt.demo - self.config.enhance("letsencrypt.demo", "http-header", + self.config.enhance("letsencrypt.demo", "ensure-http-header", "Strict-Transport-Security") self.assertTrue("headers_module" in self.config.parser.modules) @@ -543,12 +543,12 @@ class TwoVhost80Test(util.ApacheTest): self.config.parser.modules.add("headers_module") # This will create an ssl vhost for letsencrypt.demo - self.config.enhance("encryption-example.demo", "http-header", + self.config.enhance("encryption-example.demo", "ensure-http-header", "Strict-Transport-Security") self.assertRaises( errors.PluginError, - self.config.enhance, "encryption-example.demo", "http-header", + self.config.enhance, "encryption-example.demo", "ensure-http-header", "Strict-Transport-Security") @mock.patch("letsencrypt.le_util.run_script") @@ -559,7 +559,7 @@ class TwoVhost80Test(util.ApacheTest): mock_exe.return_value = True # This will create an ssl vhost for letsencrypt.demo - self.config.enhance("letsencrypt.demo", "http-header", + self.config.enhance("letsencrypt.demo", "ensure-http-header", "Upgrade-Insecure-Requests") self.assertTrue("headers_module" in self.config.parser.modules) @@ -581,12 +581,12 @@ class TwoVhost80Test(util.ApacheTest): self.config.parser.modules.add("headers_module") # This will create an ssl vhost for letsencrypt.demo - self.config.enhance("encryption-example.demo", "http-header", + self.config.enhance("encryption-example.demo", "ensure-http-header", "Upgrade-Insecure-Requests") self.assertRaises( errors.PluginError, - self.config.enhance, "encryption-example.demo", "http-header", + self.config.enhance, "encryption-example.demo", "ensure-http-header", "Upgrade-Insecure-Requests") diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e1b0c4b84..3eaf9eaef 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -418,10 +418,10 @@ class Client(object): self.apply_enhancement(domains, "redirect") if hsts: - self.apply_enhancement(domains, "http-header", + self.apply_enhancement(domains, "ensure-http-header", "Strict-Transport-Security") if uir: - self.apply_enhancement(domains, "http-header", + self.apply_enhancement(domains, "ensure-http-header", "Upgrade-Insecure-Requests") msg = ("We were unable to restart web server") @@ -435,7 +435,7 @@ class Client(object): :param domains: list of ssl_vhosts :type list of str - :param enhancement: name of enhancement, e.g. http-header + :param enhancement: name of enhancement, e.g. ensure-http-header :type str .. note:: when more options are need make options a list. diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 340e88abe..578cd77ab 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -262,12 +262,12 @@ class ClientTest(unittest.TestCase): config = ConfigHelper(redirect=False, hsts=True, uir=False) self.client.enhance_config(["foo.bar"], config) - installer.enhance.assert_called_with("foo.bar", "http-header", + installer.enhance.assert_called_with("foo.bar", "ensure-http-header", "Strict-Transport-Security") config = ConfigHelper(redirect=False, hsts=False, uir=True) self.client.enhance_config(["foo.bar"], config) - installer.enhance.assert_called_with("foo.bar", "http-header", + installer.enhance.assert_called_with("foo.bar", "ensure-http-header", "Upgrade-Insecure-Requests") self.assertEqual(installer.save.call_count, 3) From 090a9a0e465611b0c4448eaedc97bf9c5b93791b Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 Nov 2015 01:56:49 +0000 Subject: [PATCH 26/26] add PluginEnhancementAlreadyPresent and use it --- .../letsencrypt_apache/configurator.py | 16 +++++++++++----- .../tests/configurator_test.py | 6 +++--- letsencrypt/cli.py | 2 +- letsencrypt/client.py | 3 +++ letsencrypt/errors.py | 4 ++++ 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 4b66c5c6f..a5a56f6c4 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -782,7 +782,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :returns: boolean :rtype: (bool) - :raises errors.PluginError: When header header_substring exists + :raises errors.PluginEnhancementAlreadyPresent When header + header_substring exists """ header_path = self.parser.find_dir("Header", None, start=ssl_vhost.path) @@ -791,8 +792,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): pat = '(?:[ "]|^)(%s)(?:[ "]|$)' % (header_substring.lower()) for match in header_path: if re.search(pat, self.aug.get(match).lower()): - raise errors.PluginError("Existing %s header" % - (header_substring)) + raise errors.PluginEnhancementAlreadyPresent( + "Existing %s header" % (header_substring)) def _enable_redirect(self, ssl_vhost, unused_options): """Redirect all equivalent HTTP traffic to ssl_vhost. @@ -863,8 +864,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param vhost: vhost to check :type vhost: :class:`~letsencrypt_apache.obj.VirtualHost` - :raises errors.PluginError: When another redirection exists + :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) @@ -881,7 +886,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): rewrite_path, constants.REWRITE_HTTPS_ARGS): if self.aug.get(match) != arg: raise errors.PluginError("Unknown Existing RewriteRule") - raise errors.PluginError( + + raise errors.PluginEnhancementAlreadyPresent( "Let's Encrypt has already enabled redirection") def _create_redirect_vhost(self, ssl_vhost): diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 8eb1e16e2..a7714615e 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -547,7 +547,7 @@ class TwoVhost80Test(util.ApacheTest): "Strict-Transport-Security") self.assertRaises( - errors.PluginError, + errors.PluginEnhancementAlreadyPresent, self.config.enhance, "encryption-example.demo", "ensure-http-header", "Strict-Transport-Security") @@ -585,7 +585,7 @@ class TwoVhost80Test(util.ApacheTest): "Upgrade-Insecure-Requests") self.assertRaises( - errors.PluginError, + errors.PluginEnhancementAlreadyPresent, self.config.enhance, "encryption-example.demo", "ensure-http-header", "Upgrade-Insecure-Requests") @@ -631,7 +631,7 @@ class TwoVhost80Test(util.ApacheTest): self.config.parser.modules.add("rewrite_module") self.config.enhance("encryption-example.demo", "redirect") self.assertRaises( - errors.PluginError, + errors.PluginEnhancementAlreadyPresent, self.config.enhance, "encryption-example.demo", "redirect") def test_unknown_rewrite(self): diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 34551c97f..a30cb223d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -930,7 +930,7 @@ def prepare_and_parse_args(plugins, args): " Defends against SSL Stripping.", dest="hsts", default=False) helpful.add( "security", "--no-hsts", action="store_false", - help="Do not automaticcally add the Strict-Transport-Security header" + help="Do not automatically add the Strict-Transport-Security header" " to every HTTP response.", dest="hsts", default=False) helpful.add( "security", "--uir", action="store_true", diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 3eaf9eaef..f7010e09d 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -454,6 +454,9 @@ class Client(object): for dom in domains: try: self.installer.enhance(dom, enhancement, options) + except errors.PluginEnhancementAlreadyPresent: + logger.warn("Enhancement %s was already set.", + enhancement) except errors.PluginError: logger.warn("Unable to set enhancement %s for %s", enhancement, dom) diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py index 0df544b0d..1358d1048 100644 --- a/letsencrypt/errors.py +++ b/letsencrypt/errors.py @@ -66,6 +66,10 @@ class PluginError(Error): """Let's Encrypt Plugin error.""" +class PluginEnhancementAlreadyPresent(Error): + """ Enhancement was already set """ + + class PluginSelectionError(Error): """A problem with plugin/configurator selection or setup"""