From 0473c67c4808a8f2a6ed3aa083b94040b67b75a8 Mon Sep 17 00:00:00 2001 From: sagi Date: Fri, 6 Nov 2015 22:31:30 +0000 Subject: [PATCH 01/87] 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/87] 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/87] 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 465efc96012d49f64172a512faaed29ec21b718f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 7 Nov 2015 20:01:29 +0000 Subject: [PATCH 04/87] Custom acme.messages.Error (fixes #946). --- acme/acme/messages.py | 58 +++++++++++++++----------------------- acme/acme/messages_test.py | 47 ++++++++++++------------------ 2 files changed, 40 insertions(+), 65 deletions(-) diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 9d4dcbf30..0b9ea8105 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -2,12 +2,13 @@ import collections from acme import challenges +from acme import errors from acme import fields from acme import jose from acme import util -class Error(jose.JSONObjectWithFields, Exception): +class Error(jose.JSONObjectWithFields, errors.Error): """ACME error. https://tools.ietf.org/html/draft-ietf-appsawg-http-problem-00 @@ -17,55 +18,40 @@ class Error(jose.JSONObjectWithFields, Exception): :ivar unicode detail: """ - ERROR_TYPE_NAMESPACE = 'urn:acme:error:' - ERROR_TYPE_DESCRIPTIONS = { - 'badCSR': 'The CSR is unacceptable (e.g., due to a short key)', - 'badNonce': 'The client sent an unacceptable anti-replay nonce', - 'connection': 'The server could not connect to the client for DV', - 'dnssec': 'The server could not validate a DNSSEC signed domain', - 'malformed': 'The request message was malformed', - 'rateLimited': 'There were too many requests of a given type', - 'serverInternal': 'The server experienced an internal error', - 'tls': 'The server experienced a TLS error during DV', - 'unauthorized': 'The client lacks sufficient authorization', - 'unknownHost': 'The server could not resolve a domain name', - } + ERROR_TYPE_DESCRIPTIONS = dict( + ('urn:acme:error:' + name, description) for name, description in ( + ('badCSR', 'The CSR is unacceptable (e.g., due to a short key)'), + ('badNonce', 'The client sent an unacceptable anti-replay nonce'), + ('connection', 'The server could not connect to the client for DV'), + ('dnssec', 'The server could not validate a DNSSEC signed domain'), + ('malformed', 'The request message was malformed'), + ('rateLimited', 'There were too many requests of a given type'), + ('serverInternal', 'The server experienced an internal error'), + ('tls', 'The server experienced a TLS error during DV'), + ('unauthorized', 'The client lacks sufficient authorization'), + ('unknownHost', 'The server could not resolve a domain name'), + ) + ) typ = jose.Field('type') title = jose.Field('title', omitempty=True) detail = jose.Field('detail') - @typ.encoder - def typ(value): # pylint: disable=missing-docstring,no-self-argument - return Error.ERROR_TYPE_NAMESPACE + value - - @typ.decoder - def typ(value): # pylint: disable=missing-docstring,no-self-argument - # pylint thinks isinstance(value, Error), so startswith is not found - # pylint: disable=no-member - if not value.startswith(Error.ERROR_TYPE_NAMESPACE): - raise jose.DeserializationError('Missing error type prefix') - - without_prefix = value[len(Error.ERROR_TYPE_NAMESPACE):] - if without_prefix not in Error.ERROR_TYPE_DESCRIPTIONS: - raise jose.DeserializationError('Error type not recognized') - - return without_prefix - @property def description(self): """Hardcoded error description based on its type. + :returns: Description if standard ACME error or ``None``. :rtype: unicode """ - return self.ERROR_TYPE_DESCRIPTIONS[self.typ] + return self.ERROR_TYPE_DESCRIPTIONS.get(self.typ) def __str__(self): - if self.typ is not None: - return ' :: '.join([self.typ, self.description, self.detail]) - else: - return str(self.detail) + return ' :: '.join( + part for part in + (self.typ, self.description, self.detail, self.title) + if part is not None) class _Constant(jose.JSONDeSerializable, collections.Hashable): diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py index 6c1c4f596..5a7a71299 100644 --- a/acme/acme/messages_test.py +++ b/acme/acme/messages_test.py @@ -18,41 +18,30 @@ class ErrorTest(unittest.TestCase): def setUp(self): from acme.messages import Error - self.error = Error(detail='foo', typ='malformed', title='title') - self.jobj = {'detail': 'foo', 'title': 'some title'} - - def test_typ_prefix(self): - self.assertEqual('malformed', self.error.typ) - self.assertEqual( - 'urn:acme:error:malformed', self.error.to_partial_json()['type']) - self.assertEqual( - 'malformed', self.error.from_json(self.error.to_partial_json()).typ) - - def test_typ_decoder_missing_prefix(self): - from acme.messages import Error - self.jobj['type'] = 'malformed' - self.assertRaises(jose.DeserializationError, Error.from_json, self.jobj) - self.jobj['type'] = 'not valid bare type' - self.assertRaises(jose.DeserializationError, Error.from_json, self.jobj) - - def test_typ_decoder_not_recognized(self): - from acme.messages import Error - self.jobj['type'] = 'urn:acme:error:baz' - self.assertRaises(jose.DeserializationError, Error.from_json, self.jobj) - - def test_description(self): - self.assertEqual( - 'The request message was malformed', self.error.description) + self.error = Error( + detail='foo', typ='urn:acme:error:malformed', title='title') + self.jobj = { + 'detail': 'foo', + 'title': 'some title', + 'type': 'urn:acme:error:malformed', + } + self.error_custom = Error(typ='custom', detail='bar') + self.jobj_cusom = {'type': 'custom', 'detail': 'bar'} def test_from_json_hashable(self): from acme.messages import Error hash(Error.from_json(self.error.to_json())) + def test_description(self): + self.assertEqual( + 'The request message was malformed', self.error.description) + self.assertTrue(self.error_custom.description is None) + def test_str(self): self.assertEqual( - 'malformed :: The request message was malformed :: foo', - str(self.error)) - self.assertEqual('foo', str(self.error.update(typ=None))) + 'urn:acme:error:malformed :: The request message was ' + 'malformed :: foo :: title', str(self.error)) + self.assertEqual('custom :: bar', str(self.error_custom)) class ConstantTest(unittest.TestCase): @@ -232,7 +221,7 @@ class ChallengeBodyTest(unittest.TestCase): from acme.messages import Error from acme.messages import STATUS_INVALID self.status = STATUS_INVALID - error = Error(typ='serverInternal', + error = Error(typ='urn:acme:error:serverInternal', detail='Unable to communicate with DNS server') self.challb = ChallengeBody( uri='http://challb', chall=self.chall, status=self.status, From 04136cfbf29223c6c1b8ef5ed9d5dcef7eef832d Mon Sep 17 00:00:00 2001 From: sagi Date: Sun, 8 Nov 2015 04:37:57 +0000 Subject: [PATCH 05/87] 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 06/87] 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 07/87] 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 18da7dfce2e9128b2e27c89935a9f3be4385a03e Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 8 Nov 2015 14:19:58 -0600 Subject: [PATCH 08/87] Implement @pde's suggestions for Apache From this IRC log: 2015-11-02 16:31:29 @pdeee for >= 2.4.8: 2015-11-02 16:32:23 @pdeee add new SSLCertificateFile pointing to fullchain.pem 2015-11-02 16:33:10 @pdeee remove all preexisting SSLCertificateFile, SSLCertificateChainFile, SSLCACertificatePath, and possibly other fields subject to careful research :) 2015-11-02 16:33:21 @pdeee for < 2.4.8: 2015-11-02 16:34:03 @pdeee add SSLCertificateFile pointing to cert.pem 2015-11-02 16:34:42 @pdeee and SSLCertificateChainFile pointing to chain.pem 2015-11-02 16:34:50 xamnesiax gotcha 2015-11-02 16:34:55 @pdeee remove all preexisting/conflicting entries 2015-11-02 16:35:19 xamnesiax Am I correct to assume that this can all be done from deploy_certs in the apache configurator? 2015-11-02 16:36:32 xamnesiax deploy_cert * 2015-11-02 16:36:48 @pdeee I think so 2015-11-02 16:36:59 @pdeee again, jdkasten may wish to say more Pull strings out for find_dir A bit of logging Add version logging Logging, temporarily remove one branch of the conditional for testing Fix bad directive stringgrabbing code Fix directive removal logic Grab string from tree to be removed --- .../letsencrypt_apache/configurator.py | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d376fe4b6..173be4104 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -212,14 +212,22 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) # Assign the final directives; order is maintained in find_dir - self.aug.set(path["cert_path"][-1], cert_path) - self.aug.set(path["cert_key"][-1], key_path) - if chain_path is not None: - if not path["chain_path"]: - self.parser.add_dir( - vhost.path, "SSLCertificateChainFile", chain_path) - else: - self.aug.set(path["chain_path"][-1], chain_path) + if self.version >= (2, 4, 8): + logger.debug("Apache version (%s) is >= 2.4.8", + ".".join(map(str,self.version))) + for directive in ["SSLCertificateKeyFile", "SSLCertificateChainFile", + "SSLCACertificatePath"]: + logging.debug("Trying to delete directive '%s'", directive) + directive_tree = self.parser.find_dir(directive, None, vhost.path) + logging.debug(directive_tree) + if directive_tree: + logger.debug("Removing directive %s", directive) + self.aug.remove(re.sub(r"/\w*$", "", directive_tree[-1])) + logging.debug("fullchain path: %s", fullchain_path) + self.aug.set(path["cert_path"][-1], fullchain_path) + elif self.version < (2, 4, 8): + logger.debug("Apache version (%s) is < 2.4.8", + ".".join(map(str,self.version))) # Save notes about the transaction that took place self.save_notes += ("Changed vhost at %s with addresses of %s\n" From 1d2ba931b37cfca5d37d123d124a785d63f53121 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 8 Nov 2015 16:47:09 -0600 Subject: [PATCH 09/87] Improve the implementation of the suggestion Write the code to set directives Fix logging in _remove_existing_ssl_directives Fix logging statement --- .../letsencrypt_apache/configurator.py | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 173be4104..eb8268e33 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -213,21 +213,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Assign the final directives; order is maintained in find_dir if self.version >= (2, 4, 8): - logger.debug("Apache version (%s) is >= 2.4.8", - ".".join(map(str,self.version))) - for directive in ["SSLCertificateKeyFile", "SSLCertificateChainFile", - "SSLCACertificatePath"]: - logging.debug("Trying to delete directive '%s'", directive) - directive_tree = self.parser.find_dir(directive, None, vhost.path) - logging.debug(directive_tree) - if directive_tree: - logger.debug("Removing directive %s", directive) - self.aug.remove(re.sub(r"/\w*$", "", directive_tree[-1])) - logging.debug("fullchain path: %s", fullchain_path) self.aug.set(path["cert_path"][-1], fullchain_path) elif self.version < (2, 4, 8): - logger.debug("Apache version (%s) is < 2.4.8", - ".".join(map(str,self.version))) + self.aug.set(path["cert_path"][-1], cert_path) + self.aug.set(path["chain_path"][-1], chain_path) # Save notes about the transaction that took place self.save_notes += ("Changed vhost at %s with addresses of %s\n" @@ -583,6 +572,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Update Addresses self._update_ssl_vhosts_addrs(vh_p) + # Remove existing SSL directives + logging.info("Removing existing SSL directives") + self._remove_existing_ssl_directives(vh_p) + # Add directives self._add_dummy_ssl_directives(vh_p) @@ -651,6 +644,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return ssl_addrs + def _remove_existing_ssl_directives(self, vh_path): + for directive in ["SSLCertificateKeyFile", "SSLCertificateChainFile", + "SSLCACertificatePath", "SSLCertificateFile"]: + logger.debug("Trying to delete directive '%s'", directive) + directive_tree = self.parser.find_dir(directive, None, vh_path) + logger.debug("Parser found %s", directive_tree) + if directive_tree: + logger.debug("Removing directive %s", directive) + self.aug.remove(re.sub(r"/\w*$", "", directive_tree[-1])) + def _add_dummy_ssl_directives(self, vh_path): self.parser.add_dir(vh_path, "SSLCertificateFile", "insert_cert_file_path") From b26c13893864d15c3fbc7c646df003c50b1e9463 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 8 Nov 2015 17:37:28 -0600 Subject: [PATCH 10/87] Wire in everything, remove cert_key Add debug. Will be removed. Logging --- .../letsencrypt_apache/configurator.py | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index eb8268e33..ecb6fe09a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -192,39 +192,51 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): path["cert_path"] = self.parser.find_dir( "SSLCertificateFile", None, vhost.path) - path["cert_key"] = self.parser.find_dir( - "SSLCertificateKeyFile", None, vhost.path) # Only include if a certificate chain is specified if chain_path is not None: path["chain_path"] = self.parser.find_dir( "SSLCertificateChainFile", None, vhost.path) - if not path["cert_path"] or not path["cert_key"]: + if not path["cert_path"]: # Throw some can't find all of the directives error" logger.warn( - "Cannot find a cert or key directive in %s. " + "Cannot find a cert directive in %s. " "VirtualHost was not modified", vhost.path) # Presumably break here so that the virtualhost is not modified raise errors.PluginError( - "Unable to find cert and/or key directives") + "Unable to find cert directive") logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) # Assign the final directives; order is maintained in find_dir if self.version >= (2, 4, 8): + logger.debug("Apache version (%s) is >= 2.4.8", + ".".join(map(str,self.version))) + set_cert_path = fullchain_path + logger.debug(fullchain_path) + logger.debug(path["cert_path"][-1]) self.aug.set(path["cert_path"][-1], fullchain_path) elif self.version < (2, 4, 8): + logger.debug("Apache version (%s) is < 2.4.8", + ".".join(map(str,self.version))) + set_cert_path = cert_path self.aug.set(path["cert_path"][-1], cert_path) - self.aug.set(path["chain_path"][-1], chain_path) + if not path["chain_path"]: + self.parser.add_dir(vhost.path, + "SSLCertificateChainFile", chain_path) + else: + self.aug.set(path["chain_path"][-1], chain_path) + + with open("%s/sites-available/%s" % (self.parser.root, os.path.basename(vhost.filep))) as f: + logger.debug(f.read()) # Save notes about the transaction that took place self.save_notes += ("Changed vhost at %s with addresses of %s\n" - "\tSSLCertificateFile %s\n" - "\tSSLCertificateKeyFile %s\n" % + "\tSSLCertificateFile %s\n" % (vhost.filep, ", ".join(str(addr) for addr in vhost.addrs), - cert_path, key_path)) + set_cert_path)) if chain_path is not None: self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path @@ -573,7 +585,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self._update_ssl_vhosts_addrs(vh_p) # Remove existing SSL directives - logging.info("Removing existing SSL directives") + logger.info("Removing existing SSL directives") self._remove_existing_ssl_directives(vh_p) # Add directives @@ -657,8 +669,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def _add_dummy_ssl_directives(self, vh_path): self.parser.add_dir(vh_path, "SSLCertificateFile", "insert_cert_file_path") - self.parser.add_dir(vh_path, "SSLCertificateKeyFile", - "insert_key_file_path") self.parser.add_dir(vh_path, "Include", self.mod_ssl_conf) def _add_name_vhost_if_necessary(self, vhost): From e63fa279a489b552ff770bb9aaac4e7de17ba1f6 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Mon, 9 Nov 2015 17:04:21 -0600 Subject: [PATCH 11/87] Reintroduce cert_key, remove bad logging --- .../letsencrypt_apache/configurator.py | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ecb6fe09a..d4a738925 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -192,20 +192,22 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): path["cert_path"] = self.parser.find_dir( "SSLCertificateFile", None, vhost.path) + path["cert_key"] = self.parser.find_dir( + "SSLCertificateKeyFile", None, vhost.path) # Only include if a certificate chain is specified if chain_path is not None: path["chain_path"] = self.parser.find_dir( "SSLCertificateChainFile", None, vhost.path) - if not path["cert_path"]: + if not path["cert_path"] or not path["cert_key"]: # Throw some can't find all of the directives error" logger.warn( - "Cannot find a cert directive in %s. " + "Cannot find a cert or key directive in %s. " "VirtualHost was not modified", vhost.path) # Presumably break here so that the virtualhost is not modified raise errors.PluginError( - "Unable to find cert directive") + "Unable to find cert and/or key directives") logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) @@ -214,29 +216,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.debug("Apache version (%s) is >= 2.4.8", ".".join(map(str,self.version))) set_cert_path = fullchain_path - logger.debug(fullchain_path) - logger.debug(path["cert_path"][-1]) self.aug.set(path["cert_path"][-1], fullchain_path) + self.aug.set(path["cert_key"][-1], key_path) elif self.version < (2, 4, 8): logger.debug("Apache version (%s) is < 2.4.8", ".".join(map(str,self.version))) set_cert_path = cert_path self.aug.set(path["cert_path"][-1], cert_path) + self.aug.set(path["cert_key"][-1], key_path) if not path["chain_path"]: self.parser.add_dir(vhost.path, "SSLCertificateChainFile", chain_path) else: self.aug.set(path["chain_path"][-1], chain_path) - with open("%s/sites-available/%s" % (self.parser.root, os.path.basename(vhost.filep))) as f: - logger.debug(f.read()) - # Save notes about the transaction that took place self.save_notes += ("Changed vhost at %s with addresses of %s\n" - "\tSSLCertificateFile %s\n" % + "\tSSLCertificateFile %s\n" + "\tSSLCertificateKeyFile %s\n" % (vhost.filep, ", ".join(str(addr) for addr in vhost.addrs), - set_cert_path)) + set_cert_path, key_path)) if chain_path is not None: self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path @@ -669,6 +669,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def _add_dummy_ssl_directives(self, vh_path): self.parser.add_dir(vh_path, "SSLCertificateFile", "insert_cert_file_path") + self.parser.add_dir(vh_path, "SSLCertificateKeyFile", + "insert_key_file_path") self.parser.add_dir(vh_path, "Include", self.mod_ssl_conf) def _add_name_vhost_if_necessary(self, vhost): From 30c44ef1e274be6873c5a4adcda5269f25b7690e Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Mon, 9 Nov 2015 17:42:38 -0600 Subject: [PATCH 12/87] Fix lint errors --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d4a738925..5d5907895 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -214,13 +214,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Assign the final directives; order is maintained in find_dir if self.version >= (2, 4, 8): logger.debug("Apache version (%s) is >= 2.4.8", - ".".join(map(str,self.version))) + ".".join(str(i) for i in self.version)) set_cert_path = fullchain_path self.aug.set(path["cert_path"][-1], fullchain_path) self.aug.set(path["cert_key"][-1], key_path) elif self.version < (2, 4, 8): logger.debug("Apache version (%s) is < 2.4.8", - ".".join(map(str,self.version))) + ".".join(str(i) for i in self.version)) set_cert_path = cert_path self.aug.set(path["cert_path"][-1], cert_path) self.aug.set(path["cert_key"][-1], key_path) @@ -663,8 +663,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): directive_tree = self.parser.find_dir(directive, None, vh_path) logger.debug("Parser found %s", directive_tree) if directive_tree: - logger.debug("Removing directive %s", directive) - self.aug.remove(re.sub(r"/\w*$", "", directive_tree[-1])) + logger.debug("Removing directive %s", directive) + self.aug.remove(re.sub(r"/\w*$", "", directive_tree[-1])) def _add_dummy_ssl_directives(self, vh_path): self.parser.add_dir(vh_path, "SSLCertificateFile", From 188068906554b213853914cf27fb2875b7220e96 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 10 Nov 2015 06:41:59 +0000 Subject: [PATCH 13/87] 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 1f6ef1f4b158018d732cbfc52144cfef221822fb Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Tue, 10 Nov 2015 15:54:54 -0600 Subject: [PATCH 14/87] Add tests for existing cert removal and newcert directives --- .../tests/configurator_test.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 7c2137c45..b44b8bdda 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -230,6 +230,38 @@ class TwoVhost80Test(util.ApacheTest): self.config.enable_site, obj.VirtualHost("asdf", "afsaf", set(), False, False)) + def test_deploy_cert_newssl(self): + self.config = util.get_apache_configurator( + self.config_path, self.config_dir, self.work_dir, version=(2, 4, 16)) + + self.config.parser.modules.add("ssl_module") + self.config.parser.modules.add("mod_ssl.c") + + # Get the default 443 vhost + self.config.assoc["random.demo"] = self.vh_truth[1] + self.config.deploy_cert( + "random.demo", "example/cert.pem", "example/key.pem", + "example/cert_chain.pem", "example/fullchain.pem") + self.config.save() + + # Verify ssl_module was enabled. + self.assertTrue(self.vh_truth[1].enabled) + self.assertTrue("ssl_module" in self.config.parser.modules) + + loc_cert = self.config.parser.find_dir( + "sslcertificatefile", "example/fullchain.pem", self.vh_truth[1].path) + loc_key = self.config.parser.find_dir( + "sslcertificateKeyfile", "example/key.pem", self.vh_truth[1].path) + + # Verify one directive was found in the correct file + self.assertEqual(len(loc_cert), 1) + self.assertEqual(configurator.get_file_path(loc_cert[0]), + self.vh_truth[1].filep) + + self.assertEqual(len(loc_key), 1) + self.assertEqual(configurator.get_file_path(loc_key[0]), + self.vh_truth[1].filep) + def test_deploy_cert(self): self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") @@ -347,6 +379,21 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(len(self.config.vhosts), 5) + def test_remove_existing_ssl_directives(self): + # pylint: disable=protected-access + BOGUS_DIRECTIVES = ["SSLCertificateKeyFile", "SSLCertificateChainFile", + "SSLCACertificatePath", "SSLCertificateFile"] + for directive in BOGUS_DIRECTIVES: + self.config.parser.add_dir(self.vh_truth[0].path, directive, ["bogus"]) + self.config.save() + self.config._remove_existing_ssl_directives(self.vh_truth[0].path) + self.config.save() + + for directive in BOGUS_DIRECTIVES: + self.assertEqual( + self.config.parser.find_dir(directive, None, self.vh_truth[0].path), + []) + def test_make_vhost_ssl_extra_vhs(self): self.config.aug.match = mock.Mock(return_value=["p1", "p2"]) self.assertRaises( From 211c2bb33d668c13f8d2e89fb95c88806737d1dd Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Tue, 10 Nov 2015 19:41:30 -0600 Subject: [PATCH 15/87] Remove SSLCACertificatePath from removed directives SSLCACertificatePath is sometimes important to preserve. --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- .../letsencrypt_apache/tests/configurator_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 5d5907895..d8e929079 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -658,7 +658,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def _remove_existing_ssl_directives(self, vh_path): for directive in ["SSLCertificateKeyFile", "SSLCertificateChainFile", - "SSLCACertificatePath", "SSLCertificateFile"]: + "SSLCertificateFile"]: logger.debug("Trying to delete directive '%s'", directive) directive_tree = self.parser.find_dir(directive, None, vh_path) logger.debug("Parser found %s", directive_tree) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index b44b8bdda..f6cef0470 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -381,8 +381,8 @@ class TwoVhost80Test(util.ApacheTest): def test_remove_existing_ssl_directives(self): # pylint: disable=protected-access - BOGUS_DIRECTIVES = ["SSLCertificateKeyFile", "SSLCertificateChainFile", - "SSLCACertificatePath", "SSLCertificateFile"] + BOGUS_DIRECTIVES = ["SSLCertificateKeyFile", + "SSLCertificateChainFile", "SSLCertificateFile"] for directive in BOGUS_DIRECTIVES: self.config.parser.add_dir(self.vh_truth[0].path, directive, ["bogus"]) self.config.save() From 9ad38e9b37b57a542b7070396bef4c3d7985167b Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 11 Nov 2015 19:04:07 +0000 Subject: [PATCH 16/87] 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 17/87] 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 18/87] 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 19/87] 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 20/87] 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 21/87] 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 22/87] 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 23/87] 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 f972cf19d37d6fc7197567699fbd1e3acfd73cac Mon Sep 17 00:00:00 2001 From: Nav Date: Thu, 12 Nov 2015 15:34:00 +0200 Subject: [PATCH 24/87] Adding storage logging --- letsencrypt/storage.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 52be94f68..87e6a5cce 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -7,6 +7,9 @@ import configobj import parsedatetime import pytz +import logging +import logging.handlers + from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors From 108757e3323bf236ac567559b1d8eb0230fb7a15 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Thu, 12 Nov 2015 17:45:33 -0600 Subject: [PATCH 25/87] Fall back to old cert method if fullchain isn't provided --- 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 d8e929079..d891c39a9 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -212,13 +212,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) # Assign the final directives; order is maintained in find_dir - if self.version >= (2, 4, 8): + if self.version >= (2, 4, 8) and fullchain_path is not None: logger.debug("Apache version (%s) is >= 2.4.8", ".".join(str(i) for i in self.version)) set_cert_path = fullchain_path self.aug.set(path["cert_path"][-1], fullchain_path) self.aug.set(path["cert_key"][-1], key_path) - elif self.version < (2, 4, 8): + else: # fall back to old SSL cert method logger.debug("Apache version (%s) is < 2.4.8", ".".join(str(i) for i in self.version)) set_cert_path = cert_path From 0af0beaeb7f83eff4fd17078f9df8688e08abeca Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Thu, 12 Nov 2015 22:27:05 -0600 Subject: [PATCH 26/87] Remove useless SSL removal on non-SSL vhosts --- letsencrypt-apache/letsencrypt_apache/configurator.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d891c39a9..154b19f5a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -584,10 +584,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Update Addresses self._update_ssl_vhosts_addrs(vh_p) - # Remove existing SSL directives - logger.info("Removing existing SSL directives") - self._remove_existing_ssl_directives(vh_p) - # Add directives self._add_dummy_ssl_directives(vh_p) From cec5bb8b8400f709bd64f21f52444b34f01dc417 Mon Sep 17 00:00:00 2001 From: Nav Date: Thu, 12 Nov 2015 21:31:00 +0200 Subject: [PATCH 27/87] Adding logging for _consistent --- letsencrypt/storage.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 87e6a5cce..ef4f502ac 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -16,6 +16,8 @@ from letsencrypt import errors from letsencrypt import error_handler from letsencrypt import le_util +logger = logging.getLogger(__name__) + ALL_FOUR = ("cert", "privkey", "chain", "fullchain") @@ -141,11 +143,13 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Each element must be referenced with an absolute path if any(not os.path.isabs(x) for x in (self.cert, self.privkey, self.chain, self.fullchain)): + logger.debug("Element is not reference with an absolute file") return False # Each element must exist and be a symbolic link if any(not os.path.islink(x) for x in (self.cert, self.privkey, self.chain, self.fullchain)): + logger.debug("Element is not a symbolic link") return False for kind in ALL_FOUR: link = getattr(self, kind) @@ -160,16 +164,23 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes self.cli_config.archive_dir, self.lineagename) if not os.path.samefile(os.path.dirname(target), desired_directory): + #TODO: Split next line correctly + logger.debug("Element does not point within the cert " + "lineage's directory within the official " + "archive directory") return False # The link must point to a file that exists if not os.path.exists(target): + logger.debug("File does not exist") return False # The link must point to a file that follows the archive # naming convention pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) if not pattern.match(os.path.basename(target)): + logger.debug("Files does not follow the archive naming " + "convention") return False # It is NOT required that the link's target be a regular From 6760355a239068d8b744ecd785503c2df5f6559e Mon Sep 17 00:00:00 2001 From: Nav Date: Fri, 13 Nov 2015 11:44:10 +0200 Subject: [PATCH 28/87] Added more logging --- letsencrypt/storage.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index ef4f502ac..83aeb628b 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -265,6 +265,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes raise errors.CertStorageError("unknown kind of item") link = getattr(self, kind) if not os.path.exists(link): + logger.debug("File does not exist") return None target = os.readlink(link) if not os.path.isabs(target): @@ -289,11 +290,13 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) target = self.current_target(kind) if target is None or not os.path.exists(target): + logger.debug("File does not exist") target = "" matches = pattern.match(os.path.basename(target)) if matches: return int(matches.groups()[0]) else: + logger.debug("No matches") return None def version(self, kind, version): @@ -543,6 +546,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Renewals on the basis of revocation if self.ocsp_revoked(self.latest_common_version()): + logger.debug("Should renew, certificate is revoked") return True # Renewals on the basis of expiry time @@ -551,6 +555,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "cert", self.latest_common_version())) now = pytz.UTC.fromutc(datetime.datetime.utcnow()) if expiry < add_time_interval(now, interval): + logger.debug("Should renew, certificate is expired") return True return False From 16659b54333c379f44270c62b4c6ab3791531623 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Fri, 13 Nov 2015 15:49:59 -0600 Subject: [PATCH 29/87] Add `minus` option to _remove_existing_ssl_directives() Add test case as well. --- .../letsencrypt_apache/configurator.py | 7 ++--- .../tests/configurator_test.py | 26 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 154b19f5a..c52f3fc70 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -652,9 +652,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return ssl_addrs - def _remove_existing_ssl_directives(self, vh_path): - for directive in ["SSLCertificateKeyFile", "SSLCertificateChainFile", - "SSLCertificateFile"]: + def _remove_existing_ssl_directives(self, vh_path, minus={}): + directives_to_remove = list({"SSLCertificateKeyFile", "SSLCertificateChainFile", + "SSLCertificateFile"} - set(minus)) + for directive in directives_to_remove: logger.debug("Trying to delete directive '%s'", directive) directive_tree = self.parser.find_dir(directive, None, vh_path) logger.debug("Parser found %s", directive_tree) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index f6cef0470..0632db30a 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -386,6 +386,7 @@ class TwoVhost80Test(util.ApacheTest): for directive in BOGUS_DIRECTIVES: self.config.parser.add_dir(self.vh_truth[0].path, directive, ["bogus"]) self.config.save() + self.config._remove_existing_ssl_directives(self.vh_truth[0].path) self.config.save() @@ -394,6 +395,31 @@ class TwoVhost80Test(util.ApacheTest): self.config.parser.find_dir(directive, None, self.vh_truth[0].path), []) + def test_remove_existing_ssl_directives_minus_some(self): + # pylint: disable=protected-access + BOGUS_DIRECTIVES = ["SSLCertificateKeyFile", + "SSLCertificateChainFile", "SSLCertificateFile"] + MINUS_DIRECTIVES = ["SSLCertificateKeyFile", "SSLCertificateFile"] + for directive in BOGUS_DIRECTIVES: + self.config.parser.add_dir(self.vh_truth[0].path, directive, ["bogus"]) + self.config.save() + + self.config._remove_existing_ssl_directives(self.vh_truth[0].path, + minus=MINUS_DIRECTIVES) + self.config.save() + + for directive in BOGUS_DIRECTIVES: + if directive not in MINUS_DIRECTIVES: + self.assertEqual( + self.config.parser.find_dir(directive, None, self.vh_truth[0].path), + [], + msg="directive %s should have been removed" % directive) + else: + self.assertNotEqual( + self.config.parser.find_dir(directive, None, self.vh_truth[0].path), + [], + msg="directive %s should still exist" % directive) + def test_make_vhost_ssl_extra_vhs(self): self.config.aug.match = mock.Mock(return_value=["p1", "p2"]) self.assertRaises( From 9bf1b99b5bfcb5d7ab743b8b49068ba5bd8a463b Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Fri, 13 Nov 2015 17:16:50 -0600 Subject: [PATCH 30/87] Remove existing SSL directives for SSL vhosts --- letsencrypt-apache/letsencrypt_apache/configurator.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index c52f3fc70..ccb0ebc62 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -265,6 +265,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Try to find a reasonable vhost vhost = self._find_best_vhost(target_name) if vhost is not None: + if vhost.ssl: + # remove existing SSL directives (minus the ones we'll use anyway, + # since we want to preserve order) + self._remove_existing_ssl_directives( + vhost, + minus=['SSLCertificatePath', 'SSLCertificateKeyFile']) if not vhost.ssl: vhost = self.make_vhost_ssl(vhost) From 361b67276ebea50c19ea3c291410f048ea34cfc4 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 14 Nov 2015 11:26:42 -0600 Subject: [PATCH 31/87] Rewrite certificate install logic Tests are being written --- .../letsencrypt_apache/configurator.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ccb0ebc62..44821b262 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -210,25 +210,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "Unable to find cert and/or key directives") logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) + logger.debug("Apache version is %s", + ".".join(str(i) for i in self.version)) - # Assign the final directives; order is maintained in find_dir - if self.version >= (2, 4, 8) and fullchain_path is not None: - logger.debug("Apache version (%s) is >= 2.4.8", - ".".join(str(i) for i in self.version)) - set_cert_path = fullchain_path - self.aug.set(path["cert_path"][-1], fullchain_path) - self.aug.set(path["cert_key"][-1], key_path) - else: # fall back to old SSL cert method - logger.debug("Apache version (%s) is < 2.4.8", - ".".join(str(i) for i in self.version)) + if self.version < (2, 4, 8) or (chain_path and not fullchain_path): + # install SSLCertificateFile, SSLCertificateKeyFile, and SSLCertificateChainFile directives set_cert_path = cert_path self.aug.set(path["cert_path"][-1], cert_path) self.aug.set(path["cert_key"][-1], key_path) - if not path["chain_path"]: - self.parser.add_dir(vhost.path, - "SSLCertificateChainFile", chain_path) - else: - self.aug.set(path["chain_path"][-1], chain_path) + if chain_path is not None: + if not path["chain_path"]: + self.parser.add_dir(vhost.path, + "SSLCertificateChainFile", chain_path) + else: + self.aug.set(path["chain_path"][-1], chain_path) + else: + if not fullchain_path: + raise errors.PluginError("Please provide the --fullchain-path\ + option pointing to your full chain file") + set_cert_path = fullchain_path + self.aug.set(path["cert_path"][-1], fullchain_path) + self.aug.set(path["cert_key"][-1], key_path) # Save notes about the transaction that took place self.save_notes += ("Changed vhost at %s with addresses of %s\n" From 425bb98bed32c904ec696e9174556305d0fbb40b Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 14 Nov 2015 11:40:51 -0600 Subject: [PATCH 32/87] Fix lint warnings --- 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 44821b262..4f4e61d9f 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -214,7 +214,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ".".join(str(i) for i in self.version)) if self.version < (2, 4, 8) or (chain_path and not fullchain_path): - # install SSLCertificateFile, SSLCertificateKeyFile, and SSLCertificateChainFile directives + # install SSLCertificateFile, SSLCertificateKeyFile, + # and SSLCertificateChainFile directives set_cert_path = cert_path self.aug.set(path["cert_path"][-1], cert_path) self.aug.set(path["cert_key"][-1], key_path) @@ -660,7 +661,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return ssl_addrs - def _remove_existing_ssl_directives(self, vh_path, minus={}): + def _remove_existing_ssl_directives(self, vh_path, minus=None): + minus = minus or {} directives_to_remove = list({"SSLCertificateKeyFile", "SSLCertificateChainFile", "SSLCertificateFile"} - set(minus)) for directive in directives_to_remove: From 691abdc377ec38226f25d17e8ee03b84c4578988 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 14 Nov 2015 12:00:08 -0600 Subject: [PATCH 33/87] Fix for py26 (it doesn't have set literals) --- letsencrypt-apache/letsencrypt_apache/configurator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 4f4e61d9f..ed58b770a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -662,9 +662,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return ssl_addrs def _remove_existing_ssl_directives(self, vh_path, minus=None): - minus = minus or {} - directives_to_remove = list({"SSLCertificateKeyFile", "SSLCertificateChainFile", - "SSLCertificateFile"} - set(minus)) + minus = minus or set() + directives_to_remove = list(set(["SSLCertificateKeyFile", "SSLCertificateChainFile", + "SSLCertificateFile"]) - set(minus)) for directive in directives_to_remove: logger.debug("Trying to delete directive '%s'", directive) directive_tree = self.parser.find_dir(directive, None, vh_path) From a1e6db2144bd80bf35a667bd8e9c3193c133f7b2 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 14 Nov 2015 14:27:38 -0600 Subject: [PATCH 34/87] Fix logic in which the --fullchain error would never be hit --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ed58b770a..17617b1d2 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -213,7 +213,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.debug("Apache version is %s", ".".join(str(i) for i in self.version)) - if self.version < (2, 4, 8) or (chain_path and not fullchain_path): + if self.version < (2, 4, 8): # install SSLCertificateFile, SSLCertificateKeyFile, # and SSLCertificateChainFile directives set_cert_path = cert_path From e6113698f23d347b14c579ec65f61baddbeb7383 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 14 Nov 2015 14:28:17 -0600 Subject: [PATCH 35/87] Test that no fullchain throws an error --- .../letsencrypt_apache/tests/configurator_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 0632db30a..d792ea59d 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -262,6 +262,20 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(configurator.get_file_path(loc_key[0]), self.vh_truth[1].filep) + def test_deploy_cert_newssl_no_fullchain(self): + self.config = util.get_apache_configurator( + self.config_path, self.config_dir, self.work_dir, version=(2, 4, 16)) + + self.config.parser.modules.add("ssl_module") + self.config.parser.modules.add("mod_ssl.c") + + # Get the default 443 vhost + self.config.assoc["random.demo"] = self.vh_truth[1] + self.assertRaises(errors.PluginError, + lambda: self.config.deploy_cert( + "random.demo", "example/cert.pem", "example/key.pem", + "example/cert_chain.pem")) + def test_deploy_cert(self): self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") From 62f19496da76b7bf4ab5a86eda5acdbd5d705232 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 15 Nov 2015 23:00:18 -0600 Subject: [PATCH 36/87] Rewrite vhost cleaning logic --- .../letsencrypt_apache/configurator.py | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 17617b1d2..e610010a0 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -268,19 +268,21 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Try to find a reasonable vhost vhost = self._find_best_vhost(target_name) if vhost is not None: - if vhost.ssl: - # remove existing SSL directives (minus the ones we'll use anyway, - # since we want to preserve order) - self._remove_existing_ssl_directives( - vhost, - minus=['SSLCertificatePath', 'SSLCertificateKeyFile']) if not vhost.ssl: vhost = self.make_vhost_ssl(vhost) self.assoc[target_name] = vhost return vhost - return self._choose_vhost_from_list(target_name) + vhost = self._choose_vhost_from_list(target_name) + if vhost.ssl: + # remove duplicated or conflicting ssl directives + self._deduplicate_directives(vhost.path, + ["SSLCertificateFile", "SSLCertificateKeyFile"]) + # remove all problematic directives + self._remove_directives(vhost.path, ["SSLCertificateChainFile"]) + + return vhost def _choose_vhost_from_list(self, target_name): # Select a vhost from a list @@ -661,17 +663,17 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return ssl_addrs - def _remove_existing_ssl_directives(self, vh_path, minus=None): - minus = minus or set() - directives_to_remove = list(set(["SSLCertificateKeyFile", "SSLCertificateChainFile", - "SSLCertificateFile"]) - set(minus)) - for directive in directives_to_remove: - logger.debug("Trying to delete directive '%s'", directive) - directive_tree = self.parser.find_dir(directive, None, vh_path) - logger.debug("Parser found %s", directive_tree) - if directive_tree: - logger.debug("Removing directive %s", directive) - self.aug.remove(re.sub(r"/\w*$", "", directive_tree[-1])) + def _deduplicate_directives(self, vh_path, directives): + for directive in directives: + while len(self.parser.find_dir(directive, None, vh_path, False)) > 1: + directive_path = self.parser.find_dir(directive, None, vh_path, False) + self.aug.remove(re.sub(r"/\w*$", "", directive_path[0])) + + def _remove_directives(self, vh_path, directives): + for directive in directives: + while len(self.parser.find_dir(directive, None, vh_path, False)) > 0: + directive_path = self.parser.find_dir(directive, None, vh_path, False) + self.aug.remove(re.sub(r"/\w*$", "", directive_path[0])) def _add_dummy_ssl_directives(self, vh_path): self.parser.add_dir(vh_path, "SSLCertificateFile", From 76320c2d3758f645310fb91b7d7292fb4edc3afb Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 15 Nov 2015 23:00:42 -0600 Subject: [PATCH 37/87] Test vhost cleaning --- .../tests/configurator_test.py | 90 ++++++++++++------- 1 file changed, 56 insertions(+), 34 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index d792ea59d..e70c797bc 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -122,6 +122,37 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual( self.vh_truth[1], self.config.choose_vhost("none.com")) + @mock.patch("letsencrypt_apache.display_ops.select_vhost") + def test_choose_vhost_cleans_vhost_ssl(self, mock_select): + for directive in ["SSLCertificateFile", "SSLCertificateKeyFile", + "SSLCertificateChainFile", "SSLCACertificatePath"]: + for _ in range(10): + self.config.parser.add_dir(self.vh_truth[1].path, directive, ["bogus"]) + self.config.save() + mock_select.return_value = self.vh_truth[1] + + vhost = self.config.choose_vhost("none.com") + self.config.save() + + with open(vhost.filep) as f: + print f.read() + + loc_cert = self.config.parser.find_dir( + 'SSLCertificateFile', None, self.vh_truth[1].path, False) + loc_key = self.config.parser.find_dir( + 'SSLCertificateKeyFile', None, self.vh_truth[1].path, False) + loc_chain = self.config.parser.find_dir( + 'SSLCertificateChainFile', None, self.vh_truth[1].path, False) + loc_cacert = self.config.parser.find_dir( + 'SSLCACertificatePath', None, self.vh_truth[1].path, False) + + self.assertEqual(len(loc_cert), 1) + self.assertEqual(len(loc_key), 1) + + self.assertEqual(len(loc_chain), 0) + + self.assertEqual(len(loc_cacert), 10) + @mock.patch("letsencrypt_apache.display_ops.select_vhost") def test_choose_vhost_select_vhost_non_ssl(self, mock_select): mock_select.return_value = self.vh_truth[0] @@ -393,46 +424,37 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(len(self.config.vhosts), 5) - def test_remove_existing_ssl_directives(self): + def test_deduplicate_directives(self): # pylint: disable=protected-access - BOGUS_DIRECTIVES = ["SSLCertificateKeyFile", - "SSLCertificateChainFile", "SSLCertificateFile"] - for directive in BOGUS_DIRECTIVES: - self.config.parser.add_dir(self.vh_truth[0].path, directive, ["bogus"]) + DIRECTIVE = "Foo" + for _ in range(10): + self.config.parser.add_dir(self.vh_truth[1].path, DIRECTIVE, ["bar"]) self.config.save() - self.config._remove_existing_ssl_directives(self.vh_truth[0].path) + self.config._deduplicate_directives(self.vh_truth[1].path, [DIRECTIVE]) self.config.save() - for directive in BOGUS_DIRECTIVES: + self.assertEqual( + len(self.config.parser.find_dir( + DIRECTIVE, None, self.vh_truth[1].path, False)), + 1) + + def test_remove_directives(self): + # pylint: disable=protected-access + DIRECTIVES = ["Foo", "Bar"] + for directive in DIRECTIVES: + for _ in range(10): + self.config.parser.add_dir(self.vh_truth[1].path, directive, ["baz"]) + self.config.save() + + self.config._remove_directives(self.vh_truth[1].path, DIRECTIVES) + self.config.save() + + for directive in DIRECTIVES: self.assertEqual( - self.config.parser.find_dir(directive, None, self.vh_truth[0].path), - []) - - def test_remove_existing_ssl_directives_minus_some(self): - # pylint: disable=protected-access - BOGUS_DIRECTIVES = ["SSLCertificateKeyFile", - "SSLCertificateChainFile", "SSLCertificateFile"] - MINUS_DIRECTIVES = ["SSLCertificateKeyFile", "SSLCertificateFile"] - for directive in BOGUS_DIRECTIVES: - self.config.parser.add_dir(self.vh_truth[0].path, directive, ["bogus"]) - self.config.save() - - self.config._remove_existing_ssl_directives(self.vh_truth[0].path, - minus=MINUS_DIRECTIVES) - self.config.save() - - for directive in BOGUS_DIRECTIVES: - if directive not in MINUS_DIRECTIVES: - self.assertEqual( - self.config.parser.find_dir(directive, None, self.vh_truth[0].path), - [], - msg="directive %s should have been removed" % directive) - else: - self.assertNotEqual( - self.config.parser.find_dir(directive, None, self.vh_truth[0].path), - [], - msg="directive %s should still exist" % directive) + 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"]) From 062b4722e238471d53bef20b54f573959b8b1801 Mon Sep 17 00:00:00 2001 From: Nav Date: Mon, 16 Nov 2015 15:01:45 +0200 Subject: [PATCH 38/87] Adding even more logging to storage.py --- letsencrypt/storage.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 83aeb628b..d1c8a8314 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -164,7 +164,6 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes self.cli_config.archive_dir, self.lineagename) if not os.path.samefile(os.path.dirname(target), desired_directory): - #TODO: Split next line correctly logger.debug("Element does not point within the cert " "lineage's directory within the official " "archive directory") @@ -607,6 +606,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes cli_config.live_dir): if not os.path.exists(i): os.makedirs(i, 0700) + logger.debug("Creating CLI config directories") config_file, config_filename = le_util.unique_lineage_name( cli_config.renewal_configs_dir, lineagename) if not config_filename.endswith(".conf"): @@ -627,6 +627,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "live directory exists for " + lineagename) os.mkdir(archive) os.mkdir(live_dir) + logger.debug("Archive and live directories created") relative_archive = os.path.join("..", "..", "archive", lineagename) # Put the data into the appropriate files on disk @@ -636,15 +637,19 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes os.symlink(os.path.join(relative_archive, kind + "1.pem"), target[kind]) with open(target["cert"], "w") as f: + logger.debug("Writing certificate") f.write(cert) with open(target["privkey"], "w") as f: + logger.debug("Writing private key") f.write(privkey) # XXX: Let's make sure to get the file permissions right here with open(target["chain"], "w") as f: + logger.debug("Writing chain") f.write(chain) with open(target["fullchain"], "w") as f: # assumes that OpenSSL.crypto.dump_certificate includes # ending newline character + logger.debug("Writing full chain") f.write(cert + chain) # Document what we've done in a new renewal config file @@ -659,6 +664,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes " in the renewal process"] # TODO: add human-readable comments explaining other available # parameters + logger.debug("Writing new config") new_config.write() return cls(new_config.filename, cli_config) @@ -712,13 +718,17 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes os.symlink(old_privkey, target["privkey"]) else: with open(target["privkey"], "w") as f: + logger.debug("Writing new private key") f.write(new_privkey) # Save everything else with open(target["cert"], "w") as f: + logger.debug("Writing certificate") f.write(new_cert) with open(target["chain"], "w") as f: + logger.debug("Writing chain") f.write(new_chain) with open(target["fullchain"], "w") as f: + logger.debug("Writing full chain") f.write(new_cert + new_chain) return target_version From ddf5b28f7db5f2fd312f2a9e6a901f3bd9a8e6f3 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 16 Nov 2015 20:06:16 +0000 Subject: [PATCH 39/87] 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 40/87] 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 41/87] 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 1bba382c05aa988ad7eb48981ac5e2e14dd3e171 Mon Sep 17 00:00:00 2001 From: Felix Yan Date: Tue, 17 Nov 2015 10:47:20 +0800 Subject: [PATCH 42/87] letsencrypt-auto: Add instructions to use pacman on Arch --- letsencrypt-auto | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index a3009fe52..64af92ebe 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -126,8 +126,17 @@ then echo "Bootstrapping dependencies for openSUSE-based OSes..." $SUDO $BOOTSTRAP/_suse_common.sh elif [ -f /etc/arch-release ] ; then - echo "Bootstrapping dependencies for Archlinux..." - $SUDO $BOOTSTRAP/archlinux.sh + if [ "$DEBUG" = 1 ] ; then + echo "Bootstrapping dependencies for Archlinux..." + $SUDO $BOOTSTRAP/archlinux.sh + else + echo "Please use pacman to install letsencrypt packages:" + echo "# pacman -S letsencrypt letsencrypt-nginx letsencrypt-apache letshelp-letsencrypt" + echo + echo "If you would like to use the virtualenv way, please run the script again with the" + echo "--debug flag." + exit 1 + fi elif [ -f /etc/manjaro-release ] ; then ExperimentalBootstrap "Manjaro Linux" manjaro.sh "$SUDO" elif [ -f /etc/gentoo-release ] ; then From 58110a69f4820fcf70604c8c260caf03ad498fe8 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 17 Nov 2015 07:23:19 +0000 Subject: [PATCH 43/87] 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 9fd1b1f38ad65e346c79d118eed703c41a753695 Mon Sep 17 00:00:00 2001 From: Nav Date: Tue, 17 Nov 2015 11:06:12 +0200 Subject: [PATCH 44/87] Fixes #1176 --- letsencrypt/plugins/manual.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 07f06ccec..62d754e0b 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -90,6 +90,8 @@ s.serve_forever()" """ def add_parser_arguments(cls, add): add("test-mode", action="store_true", help="Test mode. Executes the manual command in subprocess.") + add("public-ip-logging-ok", action="store_true", + help="Automatically allows public IP logging.") def prepare(self): # pylint: disable=missing-docstring,no-self-use pass # pragma: no cover @@ -164,9 +166,10 @@ s.serve_forever()" """ if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") else: - if not zope.component.getUtility(interfaces.IDisplay).yesno( - self.IP_DISCLAIMER, "Yes", "No"): - raise errors.PluginError("Must agree to IP logging to proceed") + if not self.conf("public-ip-logging-ok"): + if not zope.component.getUtility(interfaces.IDisplay).yesno( + self.IP_DISCLAIMER, "Yes", "No"): + raise errors.PluginError("Must agree to IP logging to proceed") self._notify_and_wait(self.MESSAGE_TEMPLATE.format( validation=validation, response=response, From 569962b40d884d1f8b277f1694b253890c9b4042 Mon Sep 17 00:00:00 2001 From: Nav Date: Tue, 17 Nov 2015 11:50:32 +0200 Subject: [PATCH 45/87] Fixing manual authenticator tests --- letsencrypt/plugins/manual_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index a9281902f..4afd44cac 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -23,7 +23,7 @@ class AuthenticatorTest(unittest.TestCase): def setUp(self): from letsencrypt.plugins.manual import Authenticator self.config = mock.MagicMock( - http01_port=8080, manual_test_mode=False) + http01_port=8080, manual_test_mode=False, manual_public_ip_logging_ok=False) self.auth = Authenticator(config=self.config, name="manual") self.achalls = [achallenges.KeyAuthorizationAnnotatedChallenge( challb=acme_util.HTTP01_P, domain="foo.com", account_key=KEY)] From 0ae1f3053207674dedf0542656463122b9faae09 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 17 Nov 2015 18:01:20 -0800 Subject: [PATCH 46/87] Don't install all Arch packages --- docs/using.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 334e2e197..cf5336cd9 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -103,8 +103,7 @@ Operating System Packages .. code-block:: shell - sudo pacman -S letsencrypt letsencrypt-nginx letsencrypt-apache \ - letshelp-letsencrypt + sudo pacman -S letsencrypt **Other Operating Systems** From 678fb555c0e9b96203da872f9909f5cf82d8113d Mon Sep 17 00:00:00 2001 From: Nav Date: Wed, 18 Nov 2015 12:39:43 +0200 Subject: [PATCH 47/87] Improving content of logging messages --- letsencrypt/storage.py | 55 ++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index d1c8a8314..bc6e49124 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -143,13 +143,14 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Each element must be referenced with an absolute path if any(not os.path.isabs(x) for x in (self.cert, self.privkey, self.chain, self.fullchain)): - logger.debug("Element is not reference with an absolute file") + logger.debug("Element %s is not referenced with an " + "absolute file.", x) return False # Each element must exist and be a symbolic link if any(not os.path.islink(x) for x in (self.cert, self.privkey, self.chain, self.fullchain)): - logger.debug("Element is not a symbolic link") + logger.debug("Element %s is not a symbolic link.", x) return False for kind in ALL_FOUR: link = getattr(self, kind) @@ -164,22 +165,24 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes self.cli_config.archive_dir, self.lineagename) if not os.path.samefile(os.path.dirname(target), desired_directory): - logger.debug("Element does not point within the cert " - "lineage's directory within the official " - "archive directory") + logger.debug("Element's link does not point within the " + "cert lineage's directory within the " + "official archive directory. Link: %s, " + "archive directory: %s.", + os.path.dirname(target), desired_directory) return False # The link must point to a file that exists if not os.path.exists(target): - logger.debug("File does not exist") + logger.debug("Link %s points to a file that does not exist.", target) return False # The link must point to a file that follows the archive # naming convention pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) if not pattern.match(os.path.basename(target)): - logger.debug("Files does not follow the archive naming " - "convention") + logger.debug("Link %s does not follow the archive naming " + "convention.", os.path.basename(target)) return False # It is NOT required that the link's target be a regular @@ -264,7 +267,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes raise errors.CertStorageError("unknown kind of item") link = getattr(self, kind) if not os.path.exists(link): - logger.debug("File does not exist") + logger.debug("Target %s of kind %s does not exist.", link, kind) return None target = os.readlink(link) if not os.path.isabs(target): @@ -289,13 +292,14 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) target = self.current_target(kind) if target is None or not os.path.exists(target): - logger.debug("File does not exist") + logger.debug("Current-version target for %s " + "does not exist at %s.", kind, target) target = "" matches = pattern.match(os.path.basename(target)) if matches: return int(matches.groups()[0]) else: - logger.debug("No matches") + logger.debug("No matches for target %s.", kind) return None def version(self, kind, version): @@ -545,7 +549,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Renewals on the basis of revocation if self.ocsp_revoked(self.latest_common_version()): - logger.debug("Should renew, certificate is revoked") + logger.debug("Should renew, certificate is revoked.") return True # Renewals on the basis of expiry time @@ -554,7 +558,9 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "cert", self.latest_common_version())) now = pytz.UTC.fromutc(datetime.datetime.utcnow()) if expiry < add_time_interval(now, interval): - logger.debug("Should renew, certificate is expired") + logger.debug("Should renew, certificate " + "has expired since %s.", + expiry.strftime("%Y-%m-%d %H:%M:%S %Z")) return True return False @@ -606,7 +612,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes cli_config.live_dir): if not os.path.exists(i): os.makedirs(i, 0700) - logger.debug("Creating CLI config directories") + logger.debug("Creating CLI config directory %s.", i) config_file, config_filename = le_util.unique_lineage_name( cli_config.renewal_configs_dir, lineagename) if not config_filename.endswith(".conf"): @@ -627,7 +633,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "live directory exists for " + lineagename) os.mkdir(archive) os.mkdir(live_dir) - logger.debug("Archive and live directories created") + logger.debug("Archive directory %s and live " + "directory %s created.", archive, live_dir) relative_archive = os.path.join("..", "..", "archive", lineagename) # Put the data into the appropriate files on disk @@ -637,19 +644,19 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes os.symlink(os.path.join(relative_archive, kind + "1.pem"), target[kind]) with open(target["cert"], "w") as f: - logger.debug("Writing certificate") + logger.debug("Writing certificate.") f.write(cert) with open(target["privkey"], "w") as f: - logger.debug("Writing private key") + logger.debug("Writing private key.") f.write(privkey) # XXX: Let's make sure to get the file permissions right here with open(target["chain"], "w") as f: - logger.debug("Writing chain") + logger.debug("Writing chain.") f.write(chain) with open(target["fullchain"], "w") as f: # assumes that OpenSSL.crypto.dump_certificate includes # ending newline character - logger.debug("Writing full chain") + logger.debug("Writing full chain.") f.write(cert + chain) # Document what we've done in a new renewal config file @@ -664,7 +671,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes " in the renewal process"] # TODO: add human-readable comments explaining other available # parameters - logger.debug("Writing new config") + logger.debug("Writing new config %s.", config_filename) new_config.write() return cls(new_config.filename, cli_config) @@ -718,17 +725,17 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes os.symlink(old_privkey, target["privkey"]) else: with open(target["privkey"], "w") as f: - logger.debug("Writing new private key") + logger.debug("Writing new private key.") f.write(new_privkey) # Save everything else with open(target["cert"], "w") as f: - logger.debug("Writing certificate") + logger.debug("Writing certificate.") f.write(new_cert) with open(target["chain"], "w") as f: - logger.debug("Writing chain") + logger.debug("Writing chain.") f.write(new_chain) with open(target["fullchain"], "w") as f: - logger.debug("Writing full chain") + logger.debug("Writing full chain.") f.write(new_cert + new_chain) return target_version From 24e5e3d8e59e9af4b706f76030a4d12dc115c384 Mon Sep 17 00:00:00 2001 From: Nav Date: Wed, 18 Nov 2015 12:49:06 +0200 Subject: [PATCH 48/87] Fixing tests broken by including the variable x in the logger debug messages --- letsencrypt/storage.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index bc6e49124..b04536c1c 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -141,17 +141,17 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ # Each element must be referenced with an absolute path - if any(not os.path.isabs(x) for x in - (self.cert, self.privkey, self.chain, self.fullchain)): - logger.debug("Element %s is not referenced with an " - "absolute file.", x) - return False + for x in (self.cert, self.privkey, self.chain, self.fullchain): + if not os.path.isabs(x): + logger.debug("Element %s is not referenced with an " + "absolute file.", x) + return False # Each element must exist and be a symbolic link - if any(not os.path.islink(x) for x in - (self.cert, self.privkey, self.chain, self.fullchain)): - logger.debug("Element %s is not a symbolic link.", x) - return False + for x in (self.cert, self.privkey, self.chain, self.fullchain): + if not os.path.islink(x): + logger.debug("Element %s is not a symbolic link.", x) + return False for kind in ALL_FOUR: link = getattr(self, kind) where = os.path.dirname(link) From 25e6502aac7fcb7370cc331bba451dbe8a95ef9d Mon Sep 17 00:00:00 2001 From: Nav Date: Wed, 18 Nov 2015 21:28:57 +0200 Subject: [PATCH 49/87] Adding paths to cert and chain logging messages --- letsencrypt/storage.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index b04536c1c..cc7ab4313 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -644,19 +644,19 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes os.symlink(os.path.join(relative_archive, kind + "1.pem"), target[kind]) with open(target["cert"], "w") as f: - logger.debug("Writing certificate.") + logger.debug("Writing certificate to %s.", target["cert"]) f.write(cert) with open(target["privkey"], "w") as f: - logger.debug("Writing private key.") + logger.debug("Writing private key to %s.", target["privkey"]) f.write(privkey) # XXX: Let's make sure to get the file permissions right here with open(target["chain"], "w") as f: - logger.debug("Writing chain.") + logger.debug("Writing chain to %s.", target["chain"]) f.write(chain) with open(target["fullchain"], "w") as f: # assumes that OpenSSL.crypto.dump_certificate includes # ending newline character - logger.debug("Writing full chain.") + logger.debug("Writing full chain to %s.", target["fullchain"]) f.write(cert + chain) # Document what we've done in a new renewal config file @@ -722,20 +722,21 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes old_privkey = os.readlink(old_privkey) else: old_privkey = "privkey{0}.pem".format(prior_version) + logger.debug("Writing symlink to old private key, %s.", old_privkey) os.symlink(old_privkey, target["privkey"]) else: with open(target["privkey"], "w") as f: - logger.debug("Writing new private key.") + logger.debug("Writing new private key to %s.", target["privkey"]) f.write(new_privkey) # Save everything else with open(target["cert"], "w") as f: - logger.debug("Writing certificate.") + logger.debug("Writing certificate to %s.", target["cert"]) f.write(new_cert) with open(target["chain"], "w") as f: - logger.debug("Writing chain.") + logger.debug("Writing chain to %s.", target["chain"]) f.write(new_chain) with open(target["fullchain"], "w") as f: - logger.debug("Writing full chain.") + logger.debug("Writing full chain to %s.", target["fullchain"]) f.write(new_cert + new_chain) return target_version From e5e7cef6d688e1699082dd8aed84853bc94655b3 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Wed, 18 Nov 2015 19:22:14 -0600 Subject: [PATCH 50/87] Fix conditional for fullchain_path edge cases --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- .../letsencrypt_apache/tests/configurator_test.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index e610010a0..8ec7cda8d 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -213,7 +213,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.debug("Apache version is %s", ".".join(str(i) for i in self.version)) - if self.version < (2, 4, 8): + if self.version < (2, 4, 8) or (chain_path and not fullchain_path): # install SSLCertificateFile, SSLCertificateKeyFile, # and SSLCertificateChainFile directives set_cert_path = cert_path diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index e70c797bc..96ac2c023 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -304,8 +304,7 @@ class TwoVhost80Test(util.ApacheTest): self.config.assoc["random.demo"] = self.vh_truth[1] self.assertRaises(errors.PluginError, lambda: self.config.deploy_cert( - "random.demo", "example/cert.pem", "example/key.pem", - "example/cert_chain.pem")) + "random.demo", "example/cert.pem", "example/key.pem")) def test_deploy_cert(self): self.config.parser.modules.add("ssl_module") From b19c9d858cbbb29a4a3a81fcc5e8366d88987d15 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Wed, 18 Nov 2015 21:12:53 -0600 Subject: [PATCH 51/87] Fix a few nits, coverage --- .../letsencrypt_apache/configurator.py | 2 ++ .../letsencrypt_apache/tests/configurator_test.py | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 8ec7cda8d..d80d27d1c 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -225,6 +225,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "SSLCertificateChainFile", chain_path) else: self.aug.set(path["chain_path"][-1], chain_path) + else: + raise errors.PluginError("--chain-path is required for your version of Apache") else: if not fullchain_path: raise errors.PluginError("Please provide the --fullchain-path\ diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 96ac2c023..9f30153c3 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -306,6 +306,19 @@ class TwoVhost80Test(util.ApacheTest): lambda: self.config.deploy_cert( "random.demo", "example/cert.pem", "example/key.pem")) + def test_deploy_cert_old_apache_no_chain(self): + self.config = util.get_apache_configurator( + self.config_path, self.config_dir, self.work_dir, version=(2, 4, 7)) + + self.config.parser.modules.add("ssl_module") + self.config.parser.modules.add("mod_ssl.c") + + # Get the default 443 vhost + self.config.assoc["random.demo"] = self.vh_truth[1] + self.assertRaises(errors.PluginError, + lambda: self.config.deploy_cert( + "random.demo", "example/cert.pem", "example/key.pem")) + def test_deploy_cert(self): self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") From ca6a77bb1dfd8114123cab1837f86ff4693ee48a Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Wed, 18 Nov 2015 21:30:46 -0600 Subject: [PATCH 52/87] Fix tests Remove debugging print from tests Fix lint warnings --- .../letsencrypt_apache/tests/configurator_test.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 9f30153c3..58aac1216 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -131,12 +131,9 @@ class TwoVhost80Test(util.ApacheTest): self.config.save() mock_select.return_value = self.vh_truth[1] - vhost = self.config.choose_vhost("none.com") + self.config.choose_vhost("none.com") self.config.save() - with open(vhost.filep) as f: - print f.read() - loc_cert = self.config.parser.find_dir( 'SSLCertificateFile', None, self.vh_truth[1].path, False) loc_key = self.config.parser.find_dir( From 9205b9c9872f17b6b342a968516990fe2db71d66 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Thu, 19 Nov 2015 12:12:25 -0500 Subject: [PATCH 53/87] Remove remaining "DVSNI" wording, changing it to reference TLS-SNI-01, which it changed into. Close #1417. Also make _get_addrs() private, since it's called only internally. --- letsencrypt-apache/docs/api/dvsni.rst | 5 -- letsencrypt-apache/docs/api/tls_sni_01.rst | 5 ++ .../letsencrypt_apache/configurator.py | 12 ++-- .../tests/configurator_test.py | 12 ++-- .../{dvsni_test.py => tls_sni_01_test.py} | 16 +++--- .../{dvsni.py => tls_sni_01.py} | 55 ++++++++++--------- letsencrypt-nginx/docs/api/dvsni.rst | 5 -- letsencrypt-nginx/docs/api/tls_sni_01.rst | 5 ++ .../letsencrypt_nginx/configurator.py | 12 ++-- .../tests/configurator_test.py | 12 ++-- .../{dvsni_test.py => tls_sni_01_test.py} | 12 ++-- .../{dvsni.py => tls_sni_01.py} | 38 +++++++------ letsencrypt/plugins/common.py | 2 +- 13 files changed, 98 insertions(+), 93 deletions(-) delete mode 100644 letsencrypt-apache/docs/api/dvsni.rst create mode 100644 letsencrypt-apache/docs/api/tls_sni_01.rst rename letsencrypt-apache/letsencrypt_apache/tests/{dvsni_test.py => tls_sni_01_test.py} (91%) rename letsencrypt-apache/letsencrypt_apache/{dvsni.py => tls_sni_01.py} (78%) delete mode 100644 letsencrypt-nginx/docs/api/dvsni.rst create mode 100644 letsencrypt-nginx/docs/api/tls_sni_01.rst rename letsencrypt-nginx/letsencrypt_nginx/tests/{dvsni_test.py => tls_sni_01_test.py} (95%) rename letsencrypt-nginx/letsencrypt_nginx/{dvsni.py => tls_sni_01.py} (82%) diff --git a/letsencrypt-apache/docs/api/dvsni.rst b/letsencrypt-apache/docs/api/dvsni.rst deleted file mode 100644 index 945771db8..000000000 --- a/letsencrypt-apache/docs/api/dvsni.rst +++ /dev/null @@ -1,5 +0,0 @@ -:mod:`letsencrypt_apache.dvsni` -------------------------------- - -.. automodule:: letsencrypt_apache.dvsni - :members: diff --git a/letsencrypt-apache/docs/api/tls_sni_01.rst b/letsencrypt-apache/docs/api/tls_sni_01.rst new file mode 100644 index 000000000..ee1072e96 --- /dev/null +++ b/letsencrypt-apache/docs/api/tls_sni_01.rst @@ -0,0 +1,5 @@ +:mod:`letsencrypt_apache.tls_sni_01` +------------------------------- + +.. automodule:: letsencrypt_apache.tls_sni_01 + :members: diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index f10f0c241..ef7ff03c6 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -22,7 +22,7 @@ from letsencrypt.plugins import common from letsencrypt_apache import augeas_configurator from letsencrypt_apache import constants from letsencrypt_apache import display_ops -from letsencrypt_apache import dvsni +from letsencrypt_apache import tls_sni_01 from letsencrypt_apache import obj from letsencrypt_apache import parser @@ -1152,15 +1152,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ self._chall_out.update(achalls) responses = [None] * len(achalls) - apache_dvsni = dvsni.ApacheDvsni(self) + authenticator = tls_sni_01.ApacheTlsSni01(self) for i, achall in enumerate(achalls): - # Currently also have dvsni hold associated index + # Currently also have authenticator hold associated index # of the challenge. This helps to put all of the responses back # together when they are all complete. - apache_dvsni.add_chall(achall, i) + authenticator.add_chall(achall, i) - sni_response = apache_dvsni.perform() + sni_response = authenticator.perform() if sni_response: # Must restart in order to activate the challenges. # Handled here because we may be able to load up other challenge @@ -1171,7 +1171,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # place in the responses return value. All responses must be in the # same order as the original challenges. for i, resp in enumerate(sni_response): - responses[apache_dvsni.indices[i]] = resp + responses[authenticator.indices[i]] = resp return responses diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 0350a32ec..b86011e90 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -380,23 +380,23 @@ class TwoVhost80Test(util.ApacheTest): self.config._add_name_vhost_if_necessary(self.vh_truth[0]) self.assertTrue(self.config.save.called) - @mock.patch("letsencrypt_apache.configurator.dvsni.ApacheDvsni.perform") + @mock.patch("letsencrypt_apache.configurator.tls_sni_01.ApacheTlsSni01.perform") @mock.patch("letsencrypt_apache.configurator.ApacheConfigurator.restart") - def test_perform(self, mock_restart, mock_dvsni_perform): + def test_perform(self, mock_restart, mock_perform): # Only tests functionality specific to configurator.perform # Note: As more challenges are offered this will have to be expanded account_key, achall1, achall2 = self.get_achalls() - dvsni_ret_val = [ + expected = [ achall1.response(account_key), achall2.response(account_key), ] - mock_dvsni_perform.return_value = dvsni_ret_val + mock_perform.return_value = expected responses = self.config.perform([achall1, achall2]) - self.assertEqual(mock_dvsni_perform.call_count, 1) - self.assertEqual(responses, dvsni_ret_val) + self.assertEqual(mock_perform.call_count, 1) + self.assertEqual(responses, expected) self.assertEqual(mock_restart.call_count, 1) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py similarity index 91% rename from letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py rename to letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py index 911c2a36b..f4dff7734 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py @@ -1,4 +1,4 @@ -"""Test for letsencrypt_apache.dvsni.""" +"""Test for letsencrypt_apache.tls_sni_01.""" import unittest import shutil @@ -10,21 +10,21 @@ from letsencrypt_apache import obj from letsencrypt_apache.tests import util -class DvsniPerformTest(util.ApacheTest): - """Test the ApacheDVSNI challenge.""" +class TlsSniPerformTest(util.ApacheTest): + """Test the ApacheTlsSni01 challenge.""" auth_key = common_test.TLSSNI01Test.auth_key achalls = common_test.TLSSNI01Test.achalls def setUp(self): # pylint: disable=arguments-differ - super(DvsniPerformTest, self).setUp() + super(TlsSniPerformTest, self).setUp() config = util.get_apache_configurator( self.config_path, self.config_dir, self.work_dir) config.config.tls_sni_01_port = 443 - from letsencrypt_apache import dvsni - self.sni = dvsni.ApacheDvsni(config) + from letsencrypt_apache import tls_sni_01 + self.sni = tls_sni_01.ApacheTlsSni01(config) def tearDown(self): shutil.rmtree(self.temp_dir) @@ -121,7 +121,7 @@ class DvsniPerformTest(util.ApacheTest): names = vhost.get_names() self.assertTrue(names in z_domains) - def test_get_dvsni_addrs_default(self): + def test_get_addrs_default(self): self.sni.configurator.choose_vhost = mock.Mock( return_value=obj.VirtualHost( "path", "aug_path", set([obj.Addr.fromstring("_default_:443")]), @@ -130,7 +130,7 @@ class DvsniPerformTest(util.ApacheTest): self.assertEqual( set([obj.Addr.fromstring("*:443")]), - self.sni.get_dvsni_addrs(self.achalls[0])) + self.sni._get_addrs(self.achalls[0])) # pylint: disable=protected-access if __name__ == "__main__": diff --git a/letsencrypt-apache/letsencrypt_apache/dvsni.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py similarity index 78% rename from letsencrypt-apache/letsencrypt_apache/dvsni.py rename to letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index 2f9e9ed18..38ca1d390 100644 --- a/letsencrypt-apache/letsencrypt_apache/dvsni.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -1,4 +1,5 @@ -"""ApacheDVSNI""" +"""A TLS-SNI-01 authenticator for Apache""" + import os from letsencrypt.plugins import common @@ -7,22 +8,22 @@ from letsencrypt_apache import obj from letsencrypt_apache import parser -class ApacheDvsni(common.TLSSNI01): - """Class performs DVSNI challenges within the Apache configurator. +class ApacheTlsSni01(common.TLSSNI01): + """Class that performs TLS-SNI-01 challenges within the Apache configurator :ivar configurator: ApacheConfigurator object :type configurator: :class:`~apache.configurator.ApacheConfigurator` - :ivar list achalls: Annotated tls-sni-01 + :ivar list achalls: Annotated TLS-SNI-01 (`.KeyAuthorizationAnnotatedChallenge`) challenges. :param list indices: Meant to hold indices of challenges in a - larger array. ApacheDvsni is capable of solving many challenges + larger array. ApacheTlsSni01 is capable of solving many challenges at once which causes an indexing issue within ApacheConfigurator who must return all responses in order. Imagine ApacheConfigurator maintaining state about where all of the http-01 Challenges, - Dvsni Challenges belong in the response array. This is an optional - utility. + TLS-SNI-01 Challenges belong in the response array. This is an + optional utility. :param str challenge_conf: location of the challenge config file @@ -46,14 +47,14 @@ class ApacheDvsni(common.TLSSNI01): """ def __init__(self, *args, **kwargs): - super(ApacheDvsni, self).__init__(*args, **kwargs) + super(ApacheTlsSni01, self).__init__(*args, **kwargs) self.challenge_conf = os.path.join( self.configurator.conf("server-root"), - "le_dvsni_cert_challenge.conf") + "le_tls_sni_01_cert_challenge.conf") def perform(self): - """Perform a DVSNI challenge.""" + """Perform a TLS-SNI-01 challenge.""" if not self.achalls: return [] # Save any changes to the configuration as a precaution @@ -71,8 +72,8 @@ class ApacheDvsni(common.TLSSNI01): responses.append(self._setup_challenge_cert(achall)) # Setup the configuration - dvsni_addrs = self._mod_config() - self.configurator.make_addrs_sni_ready(dvsni_addrs) + addrs = self._mod_config() + self.configurator.make_addrs_sni_ready(addrs) # Save reversible changes self.configurator.save("SNI Challenge", True) @@ -84,16 +85,16 @@ class ApacheDvsni(common.TLSSNI01): Result: Apache config includes virtual servers for issued challs - :returns: All DVSNI addresses used + :returns: All TLS-SNI-01 addresses used :rtype: set """ - dvsni_addrs = set() + addrs = set() config_text = "\n" for achall in self.achalls: - achall_addrs = self.get_dvsni_addrs(achall) - dvsni_addrs.update(achall_addrs) + achall_addrs = self._get_addrs(achall) + addrs.update(achall_addrs) config_text += self._get_config_text(achall, achall_addrs) @@ -106,30 +107,30 @@ class ApacheDvsni(common.TLSSNI01): with open(self.challenge_conf, "w") as new_conf: new_conf.write(config_text) - return dvsni_addrs + return addrs - def get_dvsni_addrs(self, achall): - """Return the Apache addresses needed for DVSNI.""" + def _get_addrs(self, achall): + """Return the Apache addresses needed for TLS-SNI-01.""" vhost = self.configurator.choose_vhost(achall.domain) # TODO: Checkout _default_ rules. - dvsni_addrs = set() + addrs = set() default_addr = obj.Addr(("*", str( self.configurator.config.tls_sni_01_port))) for addr in vhost.addrs: if "_default_" == addr.get_addr(): - dvsni_addrs.add(default_addr) + addrs.add(default_addr) else: - dvsni_addrs.add( + addrs.add( addr.get_sni_addr(self.configurator.config.tls_sni_01_port)) - return dvsni_addrs + return addrs def _conf_include_check(self, main_config): - """Adds DVSNI challenge conf file into configuration. + """Add TLS-SNI-01 challenge conf file into configuration. - Adds DVSNI challenge include file if it does not already exist + Adds TLS-SNI-01 challenge include file if it does not already exist within mainConfig :param str main_config: file path to main user apache config file @@ -146,7 +147,7 @@ class ApacheDvsni(common.TLSSNI01): """Chocolate virtual server configuration text :param .KeyAuthorizationAnnotatedChallenge achall: Annotated - DVSNI challenge. + TLS-SNI-01 challenge. :param list ip_addrs: addresses of challenged domain :class:`list` of type `~.obj.Addr` @@ -157,7 +158,7 @@ class ApacheDvsni(common.TLSSNI01): """ ips = " ".join(str(i) for i in ip_addrs) document_root = os.path.join( - self.configurator.config.work_dir, "dvsni_page/") + self.configurator.config.work_dir, "tls_sni_01_page/") # TODO: Python docs is not clear how mutliline string literal # newlines are parsed on different platforms. At least on # Linux (Debian sid), when source file uses CRLF, Python still diff --git a/letsencrypt-nginx/docs/api/dvsni.rst b/letsencrypt-nginx/docs/api/dvsni.rst deleted file mode 100644 index 4f5f9d7e3..000000000 --- a/letsencrypt-nginx/docs/api/dvsni.rst +++ /dev/null @@ -1,5 +0,0 @@ -:mod:`letsencrypt_nginx.dvsni` ------------------------------- - -.. automodule:: letsencrypt_nginx.dvsni - :members: diff --git a/letsencrypt-nginx/docs/api/tls_sni_01.rst b/letsencrypt-nginx/docs/api/tls_sni_01.rst new file mode 100644 index 000000000..2860231b5 --- /dev/null +++ b/letsencrypt-nginx/docs/api/tls_sni_01.rst @@ -0,0 +1,5 @@ +:mod:`letsencrypt_nginx.tls_sni_01` +------------------------------ + +.. automodule:: letsencrypt_nginx.tls_sni_01 + :members: diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 29445a9d4..42ad34fec 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -24,7 +24,7 @@ from letsencrypt import reverter from letsencrypt.plugins import common from letsencrypt_nginx import constants -from letsencrypt_nginx import dvsni +from letsencrypt_nginx import tls_sni_01 from letsencrypt_nginx import obj from letsencrypt_nginx import parser @@ -573,15 +573,15 @@ class NginxConfigurator(common.Plugin): """ self._chall_out += len(achalls) responses = [None] * len(achalls) - nginx_dvsni = dvsni.NginxDvsni(self) + authenticator = tls_sni_01.NginxTlsSni01(self) for i, achall in enumerate(achalls): - # Currently also have dvsni hold associated index + # Currently also have authenticator hold associated index # of the challenge. This helps to put all of the responses back # together when they are all complete. - nginx_dvsni.add_chall(achall, i) + authenticator.add_chall(achall, i) - sni_response = nginx_dvsni.perform() + sni_response = authenticator.perform() # Must restart in order to activate the challenges. # Handled here because we may be able to load up other challenge types self.restart() @@ -590,7 +590,7 @@ class NginxConfigurator(common.Plugin): # in the responses return value. All responses must be in the same order # as the original challenges. for i, resp in enumerate(sni_response): - responses[nginx_dvsni.indices[i]] = resp + responses[authenticator.indices[i]] = resp return responses diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index ff720ea85..56ad5110c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -212,9 +212,9 @@ class NginxConfiguratorTest(util.NginxTest): ('/etc/nginx/fullchain.pem', '/etc/nginx/key.pem', nginx_conf), ]), self.config.get_all_certs_keys()) - @mock.patch("letsencrypt_nginx.configurator.dvsni.NginxDvsni.perform") + @mock.patch("letsencrypt_nginx.configurator.tls_sni_01.NginxTlsSni01.perform") @mock.patch("letsencrypt_nginx.configurator.NginxConfigurator.restart") - def test_perform(self, mock_restart, mock_dvsni_perform): + def test_perform(self, mock_restart, mock_perform): # Only tests functionality specific to configurator.perform # Note: As more challenges are offered this will have to be expanded achall1 = achallenges.KeyAuthorizationAnnotatedChallenge( @@ -230,16 +230,16 @@ class NginxConfiguratorTest(util.NginxTest): status=messages.Status("pending"), ), domain="example.com", account_key=self.rsa512jwk) - dvsni_ret_val = [ + expected = [ achall1.response(self.rsa512jwk), achall2.response(self.rsa512jwk), ] - mock_dvsni_perform.return_value = dvsni_ret_val + mock_perform.return_value = expected responses = self.config.perform([achall1, achall2]) - self.assertEqual(mock_dvsni_perform.call_count, 1) - self.assertEqual(responses, dvsni_ret_val) + self.assertEqual(mock_perform.call_count, 1) + self.assertEqual(responses, expected) self.assertEqual(mock_restart.call_count, 1) @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen") diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/tls_sni_01_test.py similarity index 95% rename from letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py rename to letsencrypt-nginx/letsencrypt_nginx/tests/tls_sni_01_test.py index d32e3d98f..04fe01bc4 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/tls_sni_01_test.py @@ -1,4 +1,4 @@ -"""Test for letsencrypt_nginx.dvsni.""" +"""Tests for letsencrypt_nginx.tls_sni_01""" import unittest import shutil @@ -16,8 +16,8 @@ from letsencrypt_nginx import obj from letsencrypt_nginx.tests import util -class DvsniPerformTest(util.NginxTest): - """Test the NginxDVSNI challenge.""" +class TlsSniPerformTest(util.NginxTest): + """Test the NginxTlsSni01 challenge.""" account_key = common_test.TLSSNI01Test.auth_key achalls = [ @@ -42,13 +42,13 @@ class DvsniPerformTest(util.NginxTest): ] def setUp(self): - super(DvsniPerformTest, self).setUp() + super(TlsSniPerformTest, self).setUp() config = util.get_nginx_configurator( self.config_path, self.config_dir, self.work_dir) - from letsencrypt_nginx import dvsni - self.sni = dvsni.NginxDvsni(config) + from letsencrypt_nginx import tls_sni_01 + self.sni = tls_sni_01.NginxTlsSni01(config) def tearDown(self): shutil.rmtree(self.temp_dir) diff --git a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py similarity index 82% rename from letsencrypt-nginx/letsencrypt_nginx/dvsni.py rename to letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py index 8fd705f08..c1bd434f6 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py @@ -1,4 +1,5 @@ -"""NginxDVSNI""" +"""A TLS-SNI-01 authenticator for Nginx""" + import itertools import logging import os @@ -13,31 +14,32 @@ from letsencrypt_nginx import nginxparser logger = logging.getLogger(__name__) -class NginxDvsni(common.TLSSNI01): - """Class performs DVSNI challenges within the Nginx configurator. +class NginxTlsSni01(common.TLSSNI01): + """TLS-SNI-01 authenticator for Nginx :ivar configurator: NginxConfigurator object :type configurator: :class:`~nginx.configurator.NginxConfigurator` - :ivar list achalls: Annotated :class:`~letsencrypt.achallenges.DVSNI` - challenges. + :ivar list achalls: Annotated + class:`~letsencrypt.achallenges.KeyAuthorizationAnnotatedChallenge` + challenges :param list indices: Meant to hold indices of challenges in a - larger array. NginxDvsni is capable of solving many challenges + larger array. NginxTlsSni01 is capable of solving many challenges at once which causes an indexing issue within NginxConfigurator who must return all responses in order. Imagine NginxConfigurator maintaining state about where all of the http-01 Challenges, - Dvsni Challenges belong in the response array. This is an optional - utility. + TLS-SNI-01 Challenges belong in the response array. This is an + optional utility. :param str challenge_conf: location of the challenge config file """ def perform(self): - """Perform a DVSNI challenge on Nginx. + """Perform a challenge on Nginx. - :returns: list of :class:`letsencrypt.acme.challenges.DVSNIResponse` + :returns: list of :class:`letsencrypt.acme.challenges.TLSSNI01Response` :rtype: list """ @@ -84,7 +86,8 @@ class NginxDvsni(common.TLSSNI01): :class:`letsencrypt_nginx.obj.Addr` to apply :raises .MisconfigurationError: - Unable to find a suitable HTTP block to include DVSNI hosts. + Unable to find a suitable HTTP block in which to include + authenticator hosts. """ # Add the 'include' statement for the challenges if it doesn't exist @@ -110,8 +113,8 @@ class NginxDvsni(common.TLSSNI01): break if not included: raise errors.MisconfigurationError( - 'LetsEncrypt could not find an HTTP block to include DVSNI ' - 'challenges in %s.' % root) + 'LetsEncrypt could not find an HTTP block to include ' + 'TLS-SNI-01 challenges in %s.' % root) config = [self._make_server_block(pair[0], pair[1]) for pair in itertools.izip(self.achalls, ll_addrs)] @@ -123,10 +126,11 @@ class NginxDvsni(common.TLSSNI01): nginxparser.dump(config, new_conf) def _make_server_block(self, achall, addrs): - """Creates a server block for a DVSNI challenge. + """Creates a server block for a challenge. - :param achall: Annotated DVSNI challenge. - :type achall: :class:`letsencrypt.achallenges.DVSNI` + :param achall: Annotated TLS-SNI-01 challenge + :type achall: + :class:`letsencrypt.achallenges.KeyAuthorizationAnnotatedChallenge` :param list addrs: addresses of challenged domain :class:`list` of type :class:`~nginx.obj.Addr` @@ -136,7 +140,7 @@ class NginxDvsni(common.TLSSNI01): """ document_root = os.path.join( - self.configurator.config.work_dir, "dvsni_page") + self.configurator.config.work_dir, "tls_sni_01_page") block = [['listen', str(addr)] for addr in addrs] diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py index 93daa90ff..d414dd146 100644 --- a/letsencrypt/plugins/common.py +++ b/letsencrypt/plugins/common.py @@ -136,7 +136,7 @@ class Addr(object): class TLSSNI01(object): - """Class that performs tls-sni-01 challenges.""" + """Abstract base for TLS-SNI-01 authenticators""" def __init__(self, configurator): self.configurator = configurator From 1d30bba0c2896dcc9b1e5f8767281444cc696836 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Thu, 19 Nov 2015 13:04:37 -0500 Subject: [PATCH 54/87] Correct pep8 errors across codebase. --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 7 ++++--- letsencrypt-nginx/letsencrypt_nginx/parser.py | 2 +- letsencrypt-nginx/letsencrypt_nginx/tests/util.py | 2 +- letsencrypt/cli.py | 2 +- letsencrypt/tests/cli_test.py | 6 +++--- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 42ad34fec..c1ac9db66 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -379,11 +379,12 @@ class NginxConfigurator(common.Plugin): :param unused_options: Not currently used :type unused_options: Not Available """ - redirect_block = [[['if', '($scheme != "https")'], + redirect_block = [[ + ['if', '($scheme != "https")'], [['return', '301 https://$host$request_uri']] ]] - self.parser.add_server_directives(vhost.filep, vhost.names, - redirect_block) + self.parser.add_server_directives( + vhost.filep, vhost.names, redirect_block) logger.info("Redirecting all traffic to ssl in %s", vhost.filep) ###################################### diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index 705257c16..d17370748 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -413,7 +413,7 @@ def _regex_match(target_name, name): return True else: return False - except re.error: # pragma: no cover + except re.error: # pragma: no cover # perl-compatible regexes are sometimes not recognized by python return False diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index e60feb3d3..3d70f7ac7 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -50,7 +50,7 @@ def get_nginx_configurator( backups = os.path.join(work_dir, "backups") with mock.patch("letsencrypt_nginx.configurator.le_util." - "exe_exists") as mock_exe_exists: + "exe_exists") as mock_exe_exists: mock_exe_exists.return_value = True config = configurator.NginxConfigurator( diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index d641578ed..51d326f1f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -381,7 +381,7 @@ def diagnose_configurator_problem(cfg_type, requested, plugins): raise errors.PluginSelectionError(msg) -def choose_configurator_plugins(args, config, plugins, verb): # pylint: disable=too-many-branches +def choose_configurator_plugins(args, config, plugins, verb): # pylint: disable=too-many-branches """ Figure out which configurator we're going to use diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 71b580cf0..b8fafc264 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -40,8 +40,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.work_dir = os.path.join(self.tmp_dir, 'work') self.logs_dir = os.path.join(self.tmp_dir, 'logs') self.standard_args = ['--text', '--config-dir', self.config_dir, - '--work-dir', self.work_dir, '--logs-dir', self.logs_dir, - '--agree-dev-preview'] + '--work-dir', self.work_dir, '--logs-dir', + self.logs_dir, '--agree-dev-preview'] def tearDown(self): shutil.rmtree(self.tmp_dir) @@ -57,7 +57,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args = self.standard_args + args with mock.patch('letsencrypt.cli.sys.stdout') as stdout: with mock.patch('letsencrypt.cli.sys.stderr') as stderr: - ret = cli.main(args[:]) # NOTE: parser can alter its args! + ret = cli.main(args[:]) # NOTE: parser can alter its args! return ret, stdout, stderr def _call_stdout(self, args): From 87a5fef90c886a26884942f2b752a6ee6b8e19e1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 14:52:45 -0800 Subject: [PATCH 55/87] Install letsencrypt-apache --- docs/using.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/using.rst b/docs/using.rst index cf5336cd9..d5e7ab606 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -103,7 +103,7 @@ Operating System Packages .. code-block:: shell - sudo pacman -S letsencrypt + sudo pacman -S letsencrypt letsencrypt-apache **Other Operating Systems** From d737546dd709fe5a1f3b8b99ca973b4d3e08a2dc Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Fri, 20 Nov 2015 16:31:01 -0600 Subject: [PATCH 56/87] Split off cleaning into a method (fixes a subtle bug) --- .../letsencrypt_apache/configurator.py | 18 +++--- .../tests/configurator_test.py | 55 +++++++++---------- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d80d27d1c..ff95eef95 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -183,6 +183,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ vhost = self.choose_vhost(domain) + self._clean_vhost(vhost) # This is done first so that ssl module is enabled and cert_path, # cert_key... can all be parsed appropriately @@ -276,15 +277,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.assoc[target_name] = vhost return vhost - vhost = self._choose_vhost_from_list(target_name) - if vhost.ssl: - # remove duplicated or conflicting ssl directives - self._deduplicate_directives(vhost.path, - ["SSLCertificateFile", "SSLCertificateKeyFile"]) - # remove all problematic directives - self._remove_directives(vhost.path, ["SSLCertificateChainFile"]) - - return vhost + return self._choose_vhost_from_list(target_name) def _choose_vhost_from_list(self, target_name): # Select a vhost from a list @@ -665,6 +658,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return ssl_addrs + def _clean_vhost(self, vhost): + # remove duplicated or conflicting ssl directives + self._deduplicate_directives(vhost.path, + ["SSLCertificateFile", "SSLCertificateKeyFile"]) + # remove all problematic directives + self._remove_directives(vhost.path, ["SSLCertificateChainFile"]) + def _deduplicate_directives(self, vh_path, directives): for directive in directives: while len(self.parser.find_dir(directive, None, vh_path, False)) > 1: diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 58aac1216..d5ea540c5 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -122,34 +122,6 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual( self.vh_truth[1], self.config.choose_vhost("none.com")) - @mock.patch("letsencrypt_apache.display_ops.select_vhost") - def test_choose_vhost_cleans_vhost_ssl(self, mock_select): - for directive in ["SSLCertificateFile", "SSLCertificateKeyFile", - "SSLCertificateChainFile", "SSLCACertificatePath"]: - for _ in range(10): - self.config.parser.add_dir(self.vh_truth[1].path, directive, ["bogus"]) - self.config.save() - mock_select.return_value = self.vh_truth[1] - - self.config.choose_vhost("none.com") - self.config.save() - - loc_cert = self.config.parser.find_dir( - 'SSLCertificateFile', None, self.vh_truth[1].path, False) - loc_key = self.config.parser.find_dir( - 'SSLCertificateKeyFile', None, self.vh_truth[1].path, False) - loc_chain = self.config.parser.find_dir( - 'SSLCertificateChainFile', None, self.vh_truth[1].path, False) - loc_cacert = self.config.parser.find_dir( - 'SSLCACertificatePath', None, self.vh_truth[1].path, False) - - self.assertEqual(len(loc_cert), 1) - self.assertEqual(len(loc_key), 1) - - self.assertEqual(len(loc_chain), 0) - - self.assertEqual(len(loc_cacert), 10) - @mock.patch("letsencrypt_apache.display_ops.select_vhost") def test_choose_vhost_select_vhost_non_ssl(self, mock_select): mock_select.return_value = self.vh_truth[0] @@ -433,6 +405,33 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(len(self.config.vhosts), 5) + def test_clean_vhost_ssl(self): + # pylint: disable=protected-access + for directive in ["SSLCertificateFile", "SSLCertificateKeyFile", + "SSLCertificateChainFile", "SSLCACertificatePath"]: + for _ in range(10): + self.config.parser.add_dir(self.vh_truth[1].path, directive, ["bogus"]) + self.config.save() + + self.config._clean_vhost(self.vh_truth[1]) + self.config.save() + + loc_cert = self.config.parser.find_dir( + 'SSLCertificateFile', None, self.vh_truth[1].path, False) + loc_key = self.config.parser.find_dir( + 'SSLCertificateKeyFile', None, self.vh_truth[1].path, False) + loc_chain = self.config.parser.find_dir( + 'SSLCertificateChainFile', None, self.vh_truth[1].path, False) + loc_cacert = self.config.parser.find_dir( + 'SSLCACertificatePath', None, self.vh_truth[1].path, False) + + self.assertEqual(len(loc_cert), 1) + self.assertEqual(len(loc_key), 1) + + self.assertEqual(len(loc_chain), 0) + + self.assertEqual(len(loc_cacert), 10) + def test_deduplicate_directives(self): # pylint: disable=protected-access DIRECTIVE = "Foo" From 368f208b7fc9fdaef62e703b1f54680e6b779d7b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 Nov 2015 18:26:44 -0800 Subject: [PATCH 57/87] Log not fail --- letsencrypt/plugins/manual.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 07f06ccec..72c5bc259 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -173,17 +173,12 @@ s.serve_forever()" """ uri=achall.chall.uri(achall.domain), ct=achall.CONTENT_TYPE, command=command)) - if response.simple_verify( + if not response.simple_verify( achall.chall, achall.domain, achall.account_key.public_key(), self.config.http01_port): - return response - else: - logger.error( - "Self-verify of challenge failed, authorization abandoned.") - if self.conf("test-mode") and self._httpd.poll() is not None: - # simply verify cause command failure... - return False - return None + logger.warning("Self-verify of challenge failed.") + + return response def _notify_and_wait(self, message): # pylint: disable=no-self-use # TODO: IDisplay wraps messages, breaking the command From 6b23fe160e25a1a5b4a48136fee9b0a03ac60d03 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 Nov 2015 18:34:35 -0800 Subject: [PATCH 58/87] Added tests --- letsencrypt/plugins/manual_test.py | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index a9281902f..5bde1f909 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -61,7 +61,9 @@ class AuthenticatorTest(unittest.TestCase): self.assertTrue(self.achalls[0].chall.encode("token") in message) mock_verify.return_value = False - self.assertEqual([None], self.auth.perform(self.achalls)) + with mock.patch("letsencrypt.plugins.manual.logger") as mock_logger: + self.auth.perform(self.achalls) + mock_logger.warning.assert_called_once_with(mock.ANY) @mock.patch("letsencrypt.plugins.manual.zope.component.getUtility") @mock.patch("letsencrypt.plugins.manual.Authenticator._notify_and_wait") @@ -87,20 +89,6 @@ class AuthenticatorTest(unittest.TestCase): self.assertRaises( errors.Error, self.auth_test_mode.perform, self.achalls) - @mock.patch("letsencrypt.plugins.manual.socket.socket") - @mock.patch("letsencrypt.plugins.manual.time.sleep", autospec=True) - @mock.patch("acme.challenges.HTTP01Response.simple_verify", - autospec=True) - @mock.patch("letsencrypt.plugins.manual.subprocess.Popen", autospec=True) - def test_perform_test_mode(self, mock_popen, mock_verify, mock_sleep, - mock_socket): - mock_popen.return_value.poll.side_effect = [None, 10] - mock_popen.return_value.pid = 1234 - mock_verify.return_value = False - self.assertEqual([False], self.auth_test_mode.perform(self.achalls)) - self.assertEqual(1, mock_sleep.call_count) - self.assertEqual(1, mock_socket.call_count) - def test_cleanup_test_mode_already_terminated(self): # pylint: disable=protected-access self.auth_test_mode._httpd = httpd = mock.Mock() From 27de932747f040375b18b16d3d4c26d42d3a45b7 Mon Sep 17 00:00:00 2001 From: Felix Yan Date: Sat, 21 Nov 2015 10:40:20 +0800 Subject: [PATCH 59/87] letsencrypt-auto: Remove nginx plugin and letshelp to keep the same behavior as the virtualenv way --- letsencrypt-auto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index 64af92ebe..083de58c4 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -131,7 +131,7 @@ then $SUDO $BOOTSTRAP/archlinux.sh else echo "Please use pacman to install letsencrypt packages:" - echo "# pacman -S letsencrypt letsencrypt-nginx letsencrypt-apache letshelp-letsencrypt" + echo "# pacman -S letsencrypt letsencrypt-apache" echo echo "If you would like to use the virtualenv way, please run the script again with the" echo "--debug flag." From 2bc0c31f2ef23bdb13d4ef0e4484670e74897b15 Mon Sep 17 00:00:00 2001 From: Patrick Figel Date: Sat, 21 Nov 2015 01:39:03 +0100 Subject: [PATCH 60/87] Trim trailing whitespace during challenge self-verification fixes #1322 --- acme/acme/challenges.py | 8 ++++++-- acme/acme/challenges_test.py | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 976d7ab12..336e6c4e5 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -228,6 +228,9 @@ class HTTP01Response(KeyAuthorizationChallengeResponse): """ + WHITESPACE_CUTSET = "\n\r\t " + """Whitespace characters which should be ignored at the end of the body.""" + def simple_verify(self, chall, domain, account_public_key, port=None): """Simple verify. @@ -273,10 +276,11 @@ class HTTP01Response(KeyAuthorizationChallengeResponse): found_ct, chall.CONTENT_TYPE) return False - if self.key_authorization != http_response.text: + challenge_response = http_response.text.rstrip(self.WHITESPACE_CUTSET) + if self.key_authorization != challenge_response: logger.debug("Key authorization from response (%r) doesn't match " "HTTP response (%r)", self.key_authorization, - http_response.text) + challenge_response) return False return True diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index c4f3d6c61..7cf387ece 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -126,6 +126,16 @@ class HTTP01ResponseTest(unittest.TestCase): self.assertFalse(self.response.simple_verify( self.chall, "local", KEY.public_key())) + @mock.patch("acme.challenges.requests.get") + def test_simple_verify_whitespace_validation(self, mock_get): + from acme.challenges import HTTP01Response + mock_get.return_value = mock.MagicMock( + text=(self.chall.validation(KEY) + + HTTP01Response.WHITESPACE_CUTSET), headers=self.good_headers) + self.assertTrue(self.response.simple_verify( + self.chall, "local", KEY.public_key())) + mock_get.assert_called_once_with(self.chall.uri("local")) + @mock.patch("acme.challenges.requests.get") def test_simple_verify_bad_content_type(self, mock_get): mock_get().text = self.chall.token From 5667acc558b1ab1492d86d42bf46e8b4c3932e41 Mon Sep 17 00:00:00 2001 From: Nav Date: Sat, 21 Nov 2015 17:57:03 +0200 Subject: [PATCH 61/87] Refining importing libraries --- letsencrypt/storage.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index cc7ab4313..71550e855 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -1,5 +1,6 @@ """Renewable certificates storage.""" import datetime +import logging import os import re @@ -7,9 +8,6 @@ import configobj import parsedatetime import pytz -import logging -import logging.handlers - from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors From b42b5d0f08d82825f0a359e9ffd9692f4f0b2f5e Mon Sep 17 00:00:00 2001 From: Nav Date: Sat, 21 Nov 2015 18:07:59 +0200 Subject: [PATCH 62/87] Refining content of logging messages --- letsencrypt/storage.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 71550e855..7e2802b14 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -142,7 +142,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes for x in (self.cert, self.privkey, self.chain, self.fullchain): if not os.path.isabs(x): logger.debug("Element %s is not referenced with an " - "absolute file.", x) + "absolute path.", x) return False # Each element must exist and be a symbolic link @@ -166,21 +166,23 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes logger.debug("Element's link does not point within the " "cert lineage's directory within the " "official archive directory. Link: %s, " + "target directory: %s, " "archive directory: %s.", - os.path.dirname(target), desired_directory) + link, os.path.dirname(target), desired_directory) return False # The link must point to a file that exists if not os.path.exists(target): - logger.debug("Link %s points to a file that does not exist.", target) + logger.debug("Link %s points to file %s that does not exist.", + link, target) return False # The link must point to a file that follows the archive # naming convention pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) if not pattern.match(os.path.basename(target)): - logger.debug("Link %s does not follow the archive naming " - "convention.", os.path.basename(target)) + logger.debug("%s does not follow the archive naming " + "convention.", target) return False # It is NOT required that the link's target be a regular @@ -265,7 +267,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes raise errors.CertStorageError("unknown kind of item") link = getattr(self, kind) if not os.path.exists(link): - logger.debug("Target %s of kind %s does not exist.", link, kind) + logger.debug("Expected symlink %s for %s does not exist.", + link, kind) return None target = os.readlink(link) if not os.path.isabs(target): @@ -557,7 +560,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes now = pytz.UTC.fromutc(datetime.datetime.utcnow()) if expiry < add_time_interval(now, interval): logger.debug("Should renew, certificate " - "has expired since %s.", + "has been expired since %s.", expiry.strftime("%Y-%m-%d %H:%M:%S %Z")) return True return False @@ -610,7 +613,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes cli_config.live_dir): if not os.path.exists(i): os.makedirs(i, 0700) - logger.debug("Creating CLI config directory %s.", i) + logger.debug("Creating directory %s.", i) config_file, config_filename = le_util.unique_lineage_name( cli_config.renewal_configs_dir, lineagename) if not config_filename.endswith(".conf"): From eb5e345c3ec25d82d7bb519e2d6fc82448dc6433 Mon Sep 17 00:00:00 2001 From: sagi Date: Sun, 22 Nov 2015 18:40:19 +0000 Subject: [PATCH 63/87] 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 f2ccc228a3eaf5731d62169683539b005a3cc21c Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Mon, 23 Nov 2015 13:17:24 -0600 Subject: [PATCH 64/87] Remove code path that will never get hit --- letsencrypt-apache/letsencrypt_apache/configurator.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ff95eef95..98cd5b407 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -221,11 +221,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.aug.set(path["cert_path"][-1], cert_path) self.aug.set(path["cert_key"][-1], key_path) if chain_path is not None: - if not path["chain_path"]: - self.parser.add_dir(vhost.path, - "SSLCertificateChainFile", chain_path) - else: - self.aug.set(path["chain_path"][-1], chain_path) + self.parser.add_dir(vhost.path, + "SSLCertificateChainFile", chain_path) else: raise errors.PluginError("--chain-path is required for your version of Apache") else: From f8a32160820a4fb0886c5091a4c825f9905a5bad Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 23 Nov 2015 20:11:47 +0000 Subject: [PATCH 65/87] 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 66/87] 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 67/87] 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 f504b0622d2d20288ea479750a5166c0a3fa8788 Mon Sep 17 00:00:00 2001 From: Francois Marier Date: Wed, 11 Nov 2015 18:22:36 -0800 Subject: [PATCH 68/87] Add the nginxparser copyright statement to letsencrypt-nginx --- LICENSE.txt | 27 +-------------------------- letsencrypt-nginx/LICENSE.txt | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/LICENSE.txt b/LICENSE.txt index 2ed752521..1a89cd8d9 100644 --- a/LICENSE.txt +++ b/LICENSE.txt @@ -2,13 +2,6 @@ Let's Encrypt Python Client Copyright (c) Electronic Frontier Foundation and others Licensed Apache Version 2.0 -Incorporating code from nginxparser -Copyright (c) 2014 Fatih Erikli -Licensed MIT - - -Text of Apache License -====================== Apache License Version 2.0, January 2004 http://www.apache.org/licenses/ @@ -184,22 +177,4 @@ Text of Apache License incurred by, or claims asserted against, such Contributor by reason of your accepting any such warranty or additional liability. - -Text of MIT License -=================== -Permission is hereby granted, free of charge, to any person obtaining a copy of -this software and associated documentation files (the "Software"), to deal in -the Software without restriction, including without limitation the rights to -use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of -the Software, and to permit persons to whom the Software is furnished to do so, -subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS -FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR -COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER -IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN -CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + END OF TERMS AND CONDITIONS diff --git a/letsencrypt-nginx/LICENSE.txt b/letsencrypt-nginx/LICENSE.txt index 981c46c9f..02a1459be 100644 --- a/letsencrypt-nginx/LICENSE.txt +++ b/letsencrypt-nginx/LICENSE.txt @@ -12,6 +12,13 @@ See the License for the specific language governing permissions and limitations under the License. + Incorporating code from nginxparser + Copyright 2014 Fatih Erikli + Licensed MIT + + +Text of Apache License +====================== Apache License Version 2.0, January 2004 http://www.apache.org/licenses/ @@ -188,3 +195,22 @@ of your accepting any such warranty or additional liability. END OF TERMS AND CONDITIONS + +Text of MIT License +=================== +Permission is hereby granted, free of charge, to any person obtaining a copy of +this software and associated documentation files (the "Software"), to deal in +the Software without restriction, including without limitation the rights to +use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of +the Software, and to permit persons to whom the Software is furnished to do so, +subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS +FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR +COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. From 651dbd12cc67d618e9d76e32d2eb92bb65715496 Mon Sep 17 00:00:00 2001 From: Francois Marier Date: Mon, 23 Nov 2015 14:57:21 -0800 Subject: [PATCH 69/87] Clarify the part of letsencrypt that uses nginxparser --- LICENSE.txt | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/LICENSE.txt b/LICENSE.txt index 1a89cd8d9..5965ec2ef 100644 --- a/LICENSE.txt +++ b/LICENSE.txt @@ -2,6 +2,13 @@ Let's Encrypt Python Client Copyright (c) Electronic Frontier Foundation and others Licensed Apache Version 2.0 +The nginx plugin incorporates code from nginxparser +Copyright (c) 2014 Fatih Erikli +Licensed MIT + + +Text of Apache License +====================== Apache License Version 2.0, January 2004 http://www.apache.org/licenses/ @@ -177,4 +184,22 @@ Licensed Apache Version 2.0 incurred by, or claims asserted against, such Contributor by reason of your accepting any such warranty or additional liability. - END OF TERMS AND CONDITIONS + +Text of MIT License +=================== +Permission is hereby granted, free of charge, to any person obtaining a copy of +this software and associated documentation files (the "Software"), to deal in +the Software without restriction, including without limitation the rights to +use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of +the Software, and to permit persons to whom the Software is furnished to do so, +subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS +FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR +COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. From 0c283b39efc0f7f4caaa4e78f9cd0492b1984e0d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 Nov 2015 18:29:41 -0500 Subject: [PATCH 70/87] s/restart/reload --- .../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 c811501a9..d5d75120a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -96,7 +96,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): help="Path to the Apache 'a2enmod' binary.") add("init-script", default=constants.CLI_DEFAULTS["init_script"], help="Path to the Apache init script (used for server " - "reload/restart).") + "reload).") add("le-vhost-ext", default=constants.CLI_DEFAULTS["le_vhost_ext"], help="SSL vhost configuration extension.") add("server-root", default=constants.CLI_DEFAULTS["server_root"], @@ -974,7 +974,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return False def enable_site(self, vhost): - """Enables an available site, Apache restart required. + """Enables an available site, Apache reload required. .. note:: Does not make sure that the site correctly works or that all modules are enabled appropriately. @@ -1009,7 +1009,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def enable_mod(self, mod_name, temp=False): """Enables module in Apache. - Both enables and restarts Apache so module is active. + Both enables and reloads Apache so module is active. :param str mod_name: Name of the module to enable. (e.g. 'ssl') :param bool temp: Whether or not this is a temporary action. @@ -1051,7 +1051,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Modules can enable additional config files. Variables may be defined # within these new configuration sections. - # Restart is not necessary as DUMP_RUN_CFG uses latest config. + # Reload is not necessary as DUMP_RUN_CFG uses latest config. self.parser.update_runtime_variables(self.conf("ctl")) def _add_parser_mod(self, mod_name): @@ -1074,16 +1074,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): le_util.run_script([self.conf("enmod"), mod_name]) def restart(self): - """Restarts apache server. + """Reloads apache server. .. todo:: This function will be converted to using reload - :raises .errors.MisconfigurationError: If unable to restart due - to a configuration problem, or if the restart subprocess + :raises .errors.MisconfigurationError: If unable to reload due + to a configuration problem, or if the reload subprocess cannot be run. """ - return apache_restart(self.conf("init-script")) + return apache_reload(self.conf("init-script")) def config_test(self): # pylint: disable=no-self-use """Check the configuration of Apache for errors. @@ -1158,7 +1158,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): sni_response = apache_dvsni.perform() if sni_response: - # Must restart in order to activate the challenges. + # Must reload in order to activate the challenges. # Handled here because we may be able to load up other challenge # types self.restart() @@ -1201,42 +1201,42 @@ def _get_mod_deps(mod_name): return deps.get(mod_name, []) -def apache_restart(apache_init_script): - """Restarts the Apache Server. +def apache_reload(apache_init_script): + """Reloads the Apache Server. :param str apache_init_script: Path to the Apache init script. .. todo:: Try to use reload instead. (This caused timing problems before) .. todo:: On failure, this should be a recovery_routine call with another - restart. This will confuse and inhibit developers from testing code + reload. This will confuse and inhibit developers from testing code though. This change should happen after the ApacheConfigurator has been thoroughly tested. The function will need to be moved into the class again. Perhaps this version can live on... for testing purposes. - :raises .errors.MisconfigurationError: If unable to restart due to a - configuration problem, or if the restart subprocess cannot be run. + :raises .errors.MisconfigurationError: If unable to reload due to a + configuration problem, or if the reload subprocess cannot be run. """ try: - proc = subprocess.Popen([apache_init_script, "restart"], + proc = subprocess.Popen([apache_init_script, "reload"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) except (OSError, ValueError): logger.fatal( - "Unable to restart the Apache process with %s", apache_init_script) + "Unable to reload the Apache process with %s", apache_init_script) raise errors.MisconfigurationError( - "Unable to restart Apache process with %s" % apache_init_script) + "Unable to reload Apache process with %s" % apache_init_script) stdout, stderr = proc.communicate() if proc.returncode != 0: # Enter recovery routine... - logger.error("Apache Restart Failed!\n%s\n%s", stdout, stderr) + logger.error("Apache Reload Failed!\n%s\n%s", stdout, stderr) raise errors.MisconfigurationError( - "Error while restarting Apache:\n%s\n%s" % (stdout, stderr)) + "Error while reloading Apache:\n%s\n%s" % (stdout, stderr)) def get_file_path(vhost_path): From 9e52b8200dd686773424be91b4f28393c942a899 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 Nov 2015 18:34:42 -0500 Subject: [PATCH 71/87] Sleeping is easier than polling --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d5d75120a..512e1bdc6 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -8,6 +8,7 @@ import re import shutil import socket import subprocess +import time import zope.interface @@ -1163,6 +1164,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # types self.restart() + # TODO: Remove this dirty hack. We need to determine a reliable way + # of identifying when the new configuration is being used. + time.sleep(3) + # Go through all of the challenges and assign them to the proper # place in the responses return value. All responses must be in the # same order as the original challenges. From 72fcee42649f643fa9486a41524d92f339359e82 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 23 Nov 2015 23:58:58 +0000 Subject: [PATCH 72/87] 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 a1cf4357906529be282d6fb97e485b626f903346 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 Nov 2015 19:28:36 -0500 Subject: [PATCH 73/87] Revert "Remove references to --manual and --webroot" This reverts commit 02562c75a3688c1cebdc0567bbc381440cc7f204. --- docs/using.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 3f04fc5fa..f6fb82f52 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -184,7 +184,7 @@ Webroot If you're running a webserver that you don't want to stop to use standalone, you can use the webroot plugin to obtain a cert by -including ``certonly`` and ``-a webroot`` on the command line. In +including ``certonly`` and ``--webroot`` on the command line. In addition, you'll need to specify ``--webroot-path`` with the root directory of the files served by your webserver. For example, ``--webroot-path /var/www/html`` or @@ -200,7 +200,7 @@ If you'd like to obtain a cert running ``letsencrypt`` on a machine other than your target webserver or perform the steps for domain validation yourself, you can use the manual plugin. While hidden from the UI, you can use the plugin to obtain a cert by specifying -``certonly`` and ``-a manual`` on the command line. This requires you +``certonly`` and ``--manual`` on the command line. This requires you to copy and paste commands into another terminal session. Nginx From f5c353217746beb7a50fa4a63c0d8e3dd1ba6d45 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 Nov 2015 19:44:00 -0500 Subject: [PATCH 74/87] Improve error message --- letsencrypt-apache/letsencrypt_apache/configurator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index c811501a9..9ccb3ca1f 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -586,7 +586,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): (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("Only one vhost per file is allowed") + raise errors.PluginError("Currently, we only support " + "configurations with one vhost per file") else: # This simplifies the process vh_p = vh_p[0] From f908e8bdafd091e791a5d73b8c7b04143340a0d6 Mon Sep 17 00:00:00 2001 From: Patrick Figel Date: Tue, 24 Nov 2015 06:19:42 +0100 Subject: [PATCH 75/87] Detect SSL vhosts by port SSLEngine on can be set outside of . Treat any vhost using port 443 as a SSL vhost. fixes #1602 --- .../letsencrypt_apache/configurator.py | 6 ++++ .../tests/configurator_test.py | 14 ++++---- .../default-ssl-port-only.conf | 36 +++++++++++++++++++ .../letsencrypt_apache/tests/util.py | 6 +++- 4 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl-port-only.conf diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 16e8dd606..91a2e4925 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -445,6 +445,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if self.parser.find_dir("SSLEngine", "on", start=path, exclude=False): is_ssl = True + # "SSLEngine on" might be set outside of + # Treat vhosts with port 443 as ssl vhosts + for addr in addrs: + if addr.get_port() == "443": + is_ssl = True + filename = get_file_path(path) is_enabled = self.is_site_enabled(filename) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 442229e8d..f7c3ba1ba 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -103,7 +103,7 @@ class TwoVhost80Test(util.ApacheTest): """ vhs = self.config.get_virtual_hosts() - self.assertEqual(len(vhs), 5) + self.assertEqual(len(vhs), 6) found = 0 for vhost in vhs: @@ -114,7 +114,7 @@ class TwoVhost80Test(util.ApacheTest): else: raise Exception("Missed: %s" % vhost) # pragma: no cover - self.assertEqual(found, 5) + self.assertEqual(found, 6) @mock.patch("letsencrypt_apache.display_ops.select_vhost") def test_choose_vhost_none_avail(self, mock_select): @@ -409,7 +409,7 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]), self.config.is_name_vhost(ssl_vhost)) - self.assertEqual(len(self.config.vhosts), 6) + self.assertEqual(len(self.config.vhosts), 7) def test_clean_vhost_ssl(self): # pylint: disable=protected-access @@ -597,14 +597,14 @@ class TwoVhost80Test(util.ApacheTest): def test_get_all_certs_keys(self): c_k = self.config.get_all_certs_keys() - self.assertEqual(len(c_k), 1) + self.assertEqual(len(c_k), 2) cert, key, path = next(iter(c_k)) self.assertTrue("cert" in cert) self.assertTrue("key" in key) - self.assertTrue("default-ssl.conf" in path) + self.assertTrue("default-ssl" in path) def test_get_all_certs_keys_malformed_conf(self): - self.config.parser.find_dir = mock.Mock(side_effect=[["path"], []]) + self.config.parser.find_dir = mock.Mock(side_effect=[["path"], [], ["path"], []]) c_k = self.config.get_all_certs_keys() self.assertFalse(c_k) @@ -710,7 +710,7 @@ class TwoVhost80Test(util.ApacheTest): 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), 6) + self.assertEqual(len(self.config.vhosts), 7) def get_achalls(self): """Return testing achallenges.""" diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl-port-only.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl-port-only.conf new file mode 100644 index 000000000..5a50c536e --- /dev/null +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl-port-only.conf @@ -0,0 +1,36 @@ + + + ServerAdmin webmaster@localhost + + DocumentRoot /var/www/html + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + # A self-signed (snakeoil) certificate can be created by installing + # the ssl-cert package. See + # /usr/share/doc/apache2/README.Debian.gz for more info. + # If both key and certificate are stored in the same file, only the + # SSLCertificateFile directive is needed. + SSLCertificateFile /etc/apache2/certs/letsencrypt-cert_5.pem + SSLCertificateKeyFile /etc/apache2/ssl/key-letsencrypt_15.pem + + + #SSLOptions +FakeBasicAuth +ExportCertData +StrictRequire + + SSLOptions +StdEnvVars + + + SSLOptions +StdEnvVars + + + BrowserMatch "MSIE [2-6]" \ + nokeepalive ssl-unclean-shutdown \ + downgrade-1.0 force-response-1.0 + # MSIE 7 and newer should be able to use keepalive + BrowserMatch "MSIE [17-9]" ssl-unclean-shutdown + + + + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index a8bfe0e4b..1bc1fbe17 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -128,7 +128,11 @@ def get_vh_truth(temp_dir, config_name): os.path.join(prefix, "mod_macro-example.conf"), os.path.join(aug_pre, "mod_macro-example.conf/Macro/VirtualHost"), - set([obj.Addr.fromstring("*:80")]), False, True, modmacro=True) + set([obj.Addr.fromstring("*:80")]), False, True, modmacro=True), + obj.VirtualHost( + os.path.join(prefix, "default-ssl-port-only.conf"), + os.path.join(aug_pre, "default-ssl-port-only.conf/IfModule/VirtualHost"), + set([obj.Addr.fromstring("_default_:443")]), True, False), ] return vh_truth From c175ff955efcf3659d294827c6d49a55200a20b8 Mon Sep 17 00:00:00 2001 From: Patrick Figel Date: Tue, 24 Nov 2015 09:42:59 +0100 Subject: [PATCH 76/87] Remove Content-Type checks from http-01 Content-Type type restrictions were removed in ACME, see https://github.com/ietf-wg-acme/acme/commit/69ac2baade014796e5258a077e7600921cd1879d fixes #1595 --- acme/acme/challenges.py | 10 --------- acme/acme/challenges_test.py | 15 +++---------- acme/acme/standalone.py | 1 - letsencrypt/plugins/manual.py | 7 ++---- letsencrypt/plugins/webroot.py | 41 +--------------------------------- 5 files changed, 6 insertions(+), 68 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 336e6c4e5..1e456d325 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -269,13 +269,6 @@ class HTTP01Response(KeyAuthorizationChallengeResponse): logger.debug("Received %s: %s. Headers: %s", http_response, http_response.text, http_response.headers) - found_ct = http_response.headers.get( - "Content-Type", chall.CONTENT_TYPE) - if found_ct != chall.CONTENT_TYPE: - logger.debug("Wrong Content-Type: found %r, expected %r", - found_ct, chall.CONTENT_TYPE) - return False - challenge_response = http_response.text.rstrip(self.WHITESPACE_CUTSET) if self.key_authorization != challenge_response: logger.debug("Key authorization from response (%r) doesn't match " @@ -292,9 +285,6 @@ class HTTP01(KeyAuthorizationChallenge): response_cls = HTTP01Response typ = response_cls.typ - CONTENT_TYPE = "text/plain" - """Only valid value for Content-Type if the header is included.""" - URI_ROOT_PATH = ".well-known/acme-challenge" """URI root path for the server provisioned resource.""" diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 7cf387ece..a4e78ebe9 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -92,7 +92,6 @@ class HTTP01ResponseTest(unittest.TestCase): from acme.challenges import HTTP01 self.chall = HTTP01(token=(b'x' * 16)) self.response = self.chall.response(KEY) - self.good_headers = {'Content-Type': HTTP01.CONTENT_TYPE} def test_to_partial_json(self): self.assertEqual(self.jmsg, self.msg.to_partial_json()) @@ -113,16 +112,14 @@ class HTTP01ResponseTest(unittest.TestCase): @mock.patch("acme.challenges.requests.get") def test_simple_verify_good_validation(self, mock_get): validation = self.chall.validation(KEY) - mock_get.return_value = mock.MagicMock( - text=validation, headers=self.good_headers) + mock_get.return_value = mock.MagicMock(text=validation) self.assertTrue(self.response.simple_verify( self.chall, "local", KEY.public_key())) mock_get.assert_called_once_with(self.chall.uri("local")) @mock.patch("acme.challenges.requests.get") def test_simple_verify_bad_validation(self, mock_get): - mock_get.return_value = mock.MagicMock( - text="!", headers=self.good_headers) + mock_get.return_value = mock.MagicMock(text="!") self.assertFalse(self.response.simple_verify( self.chall, "local", KEY.public_key())) @@ -131,17 +128,11 @@ class HTTP01ResponseTest(unittest.TestCase): from acme.challenges import HTTP01Response mock_get.return_value = mock.MagicMock( text=(self.chall.validation(KEY) + - HTTP01Response.WHITESPACE_CUTSET), headers=self.good_headers) + HTTP01Response.WHITESPACE_CUTSET)) self.assertTrue(self.response.simple_verify( self.chall, "local", KEY.public_key())) mock_get.assert_called_once_with(self.chall.uri("local")) - @mock.patch("acme.challenges.requests.get") - def test_simple_verify_bad_content_type(self, mock_get): - mock_get().text = self.chall.token - self.assertFalse(self.response.simple_verify( - self.chall, "local", KEY.public_key())) - @mock.patch("acme.challenges.requests.get") def test_simple_verify_connection_error(self, mock_get): mock_get.side_effect = requests.exceptions.RequestException diff --git a/acme/acme/standalone.py b/acme/acme/standalone.py index 3ddb21beb..02cc2daf5 100644 --- a/acme/acme/standalone.py +++ b/acme/acme/standalone.py @@ -133,7 +133,6 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): self.log_message("Serving HTTP01 with token %r", resource.chall.encode("token")) self.send_response(http_client.OK) - self.send_header("Content-type", resource.chall.CONTENT_TYPE) self.end_headers() self.wfile.write(resource.validation.encode()) return diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index a590d83f9..793285e62 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -46,8 +46,6 @@ Make sure your web server displays the following content at {validation} -Content-Type header MUST be set to {ct}. - If you don't have HTTP server configured, you can run the following command on the target server (as root): @@ -75,7 +73,6 @@ printf "%s" {validation} > {achall.URI_ROOT_PATH}/{encoded_token} # run only once per server: $(command -v python2 || command -v python2.7 || command -v python2.6) -c \\ "import BaseHTTPServer, SimpleHTTPServer; \\ -SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {{'': '{ct}'}}; \\ s = BaseHTTPServer.HTTPServer(('', {port}), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ s.serve_forever()" """ """Command template.""" @@ -142,7 +139,7 @@ s.serve_forever()" """ # TODO(kuba): pipes still necessary? validation=pipes.quote(validation), encoded_token=achall.chall.encode("token"), - ct=achall.CONTENT_TYPE, port=port) + port=port) if self.conf("test-mode"): logger.debug("Test mode. Executing the manual command: %s", command) # sh shipped with OS X does't support echo -n, but supports printf @@ -174,7 +171,7 @@ s.serve_forever()" """ self._notify_and_wait(self.MESSAGE_TEMPLATE.format( validation=validation, response=response, uri=achall.chall.uri(achall.domain), - ct=achall.CONTENT_TYPE, command=command)) + command=command)) if not response.simple_verify( achall.chall, achall.domain, diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 42bfe312b..879da2527 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -1,43 +1,4 @@ -"""Webroot plugin. - -Content-Type ------------- - -This plugin requires your webserver to use a specific `Content-Type` -header in the HTTP response. - -Apache2 -~~~~~~~ - -.. note:: Instructions written and tested for Debian Jessie. Other - operating systems might use something very similar, but you might - still need to readjust some commands. - -Create ``/etc/apache2/conf-available/letsencrypt.conf``, with -the following contents:: - - - - Header set Content-Type "text/plain" - - - -and then run ``a2enmod headers; a2enconf letsencrypt``; depending on the -output you will have to either ``service apache2 restart`` or ``service -apache2 reload``. - -nginx -~~~~~ - -Use the following snippet in your ``server{...}`` stanza:: - - location ~ /.well-known/acme-challenge/(.*) { - default_type text/plain; - } - -and reload your daemon. - -""" +"""Webroot plugin.""" import errno import logging import os From a97a702210526b37d7c2ae84a76e2f3bc188ad6e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 Nov 2015 16:04:00 -0500 Subject: [PATCH 77/87] Quikfix --- acme/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/setup.py b/acme/setup.py index a6551a023..a8375150d 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -17,7 +17,7 @@ install_requires = [ 'pyrfc3339', 'pytz', 'requests', - 'setuptools', # pkg_resources + 'setuptools==18.5', # pkg_resources 'six', 'werkzeug', ] From 7467496984b444047866831240d8ba25b67102e7 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 24 Nov 2015 23:33:21 +0000 Subject: [PATCH 78/87] 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 79/87] 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""" From b2ca861a27b2eb9715644fd9dd9cf58f4d9493e0 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 25 Nov 2015 09:44:28 -0500 Subject: [PATCH 80/87] Revert "Quikfix" This reverts commit a97a702210526b37d7c2ae84a76e2f3bc188ad6e. --- acme/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/setup.py b/acme/setup.py index a8375150d..a6551a023 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -17,7 +17,7 @@ install_requires = [ 'pyrfc3339', 'pytz', 'requests', - 'setuptools==18.5', # pkg_resources + 'setuptools', # pkg_resources 'six', 'werkzeug', ] From 8147216f1a57cbf4ac8c60c61fff3c7faf12e2b0 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Wed, 25 Nov 2015 12:43:29 -0500 Subject: [PATCH 81/87] Fix some underline lengths in docs. --- letsencrypt-apache/docs/api/tls_sni_01.rst | 2 +- letsencrypt-nginx/docs/api/tls_sni_01.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/docs/api/tls_sni_01.rst b/letsencrypt-apache/docs/api/tls_sni_01.rst index ee1072e96..2c11a3394 100644 --- a/letsencrypt-apache/docs/api/tls_sni_01.rst +++ b/letsencrypt-apache/docs/api/tls_sni_01.rst @@ -1,5 +1,5 @@ :mod:`letsencrypt_apache.tls_sni_01` -------------------------------- +------------------------------------ .. automodule:: letsencrypt_apache.tls_sni_01 :members: diff --git a/letsencrypt-nginx/docs/api/tls_sni_01.rst b/letsencrypt-nginx/docs/api/tls_sni_01.rst index 2860231b5..f9f584b0c 100644 --- a/letsencrypt-nginx/docs/api/tls_sni_01.rst +++ b/letsencrypt-nginx/docs/api/tls_sni_01.rst @@ -1,5 +1,5 @@ :mod:`letsencrypt_nginx.tls_sni_01` ------------------------------- +----------------------------------- .. automodule:: letsencrypt_nginx.tls_sni_01 :members: From e75dc965596bfd8b52019bbfef6cdd681978fc89 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Wed, 25 Nov 2015 12:44:17 -0500 Subject: [PATCH 82/87] Stop calling things that don't implement IAuthenticator authenticators. --- .../letsencrypt_apache/configurator.py | 14 +++++++------- .../letsencrypt_apache/tls_sni_01.py | 2 +- .../letsencrypt_nginx/configurator.py | 14 +++++++------- letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py | 2 +- letsencrypt/plugins/common.py | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ef7ff03c6..c7c9a98b5 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1152,15 +1152,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ self._chall_out.update(achalls) responses = [None] * len(achalls) - authenticator = tls_sni_01.ApacheTlsSni01(self) + chall_doer = tls_sni_01.ApacheTlsSni01(self) for i, achall in enumerate(achalls): - # Currently also have authenticator hold associated index - # of the challenge. This helps to put all of the responses back - # together when they are all complete. - authenticator.add_chall(achall, i) + # Currently also have chall_doer hold associated index of the + # challenge. This helps to put all of the responses back together + # when they are all complete. + chall_doer.add_chall(achall, i) - sni_response = authenticator.perform() + sni_response = chall_doer.perform() if sni_response: # Must restart in order to activate the challenges. # Handled here because we may be able to load up other challenge @@ -1171,7 +1171,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # place in the responses return value. All responses must be in the # same order as the original challenges. for i, resp in enumerate(sni_response): - responses[authenticator.indices[i]] = resp + responses[chall_doer.indices[i]] = resp return responses diff --git a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index 38ca1d390..e1a7d2d53 100644 --- a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -1,4 +1,4 @@ -"""A TLS-SNI-01 authenticator for Apache""" +"""A class that performs TLS-SNI-01 challenges for Apache""" import os diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index c1ac9db66..aaaf43c5f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -574,15 +574,15 @@ class NginxConfigurator(common.Plugin): """ self._chall_out += len(achalls) responses = [None] * len(achalls) - authenticator = tls_sni_01.NginxTlsSni01(self) + chall_doer = tls_sni_01.NginxTlsSni01(self) for i, achall in enumerate(achalls): - # Currently also have authenticator hold associated index - # of the challenge. This helps to put all of the responses back - # together when they are all complete. - authenticator.add_chall(achall, i) + # Currently also have chall_doer hold associated index of the + # challenge. This helps to put all of the responses back together + # when they are all complete. + chall_doer.add_chall(achall, i) - sni_response = authenticator.perform() + sni_response = chall_doer.perform() # Must restart in order to activate the challenges. # Handled here because we may be able to load up other challenge types self.restart() @@ -591,7 +591,7 @@ class NginxConfigurator(common.Plugin): # in the responses return value. All responses must be in the same order # as the original challenges. for i, resp in enumerate(sni_response): - responses[authenticator.indices[i]] = resp + responses[chall_doer.indices[i]] = resp return responses diff --git a/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py index c1bd434f6..e59281c4c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py @@ -1,4 +1,4 @@ -"""A TLS-SNI-01 authenticator for Nginx""" +"""A class that performs TLS-SNI-01 challenges for Nginx""" import itertools import logging diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py index d414dd146..f18b1fb3b 100644 --- a/letsencrypt/plugins/common.py +++ b/letsencrypt/plugins/common.py @@ -136,7 +136,7 @@ class Addr(object): class TLSSNI01(object): - """Abstract base for TLS-SNI-01 authenticators""" + """Abstract base for TLS-SNI-01 challenge performers""" def __init__(self, configurator): self.configurator = configurator From ef131b9bb92ee469899a588f15c0060faaf609fb Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 25 Nov 2015 23:28:58 -0800 Subject: [PATCH 83/87] Specify how long after updating SA it takes effect. --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a231f9db0..24a2baba3 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -853,7 +853,7 @@ def prepare_and_parse_args(plugins, args): "lose access to your account. You will also be unable to receive " "notice about impending expiration of revocation of your " "certificates. Updates to the Subscriber Agreement will still " - "affect you, and will be effective N days after posting an " + "affect you, and will be effective 14 days after posting an " "update to the web site.") helpful.add(None, "-m", "--email", help=config_help("email")) # positional arg shadows --domains, instead of appending, and From c48ee677df03f285fcffe1abfab484970a0e3b18 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 26 Nov 2015 16:59:06 -0800 Subject: [PATCH 84/87] Merge Augeas lens fix for backslashes in regexps https://github.com/hercules-team/augeas/issues/307 https://github.com/hercules-team/augeas/commit/155746c72f76937a21b1a035da5c56090a54ed13 --- letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 9b50a8f0e..30d8ca501 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -59,7 +59,7 @@ let empty = Util.empty_dos let indent = Util.indent (* borrowed from shellvars.aug *) -let char_arg_dir = /[^\\ '"\t\r\n]|\\\\"|\\\\'/ +let char_arg_dir = /([^\\ '"\t\r\n]|[^\\ '"\t\r\n][^ '"\t\r\n]*[^\\ '"\t\r\n])|\\\\"|\\\\'/ let char_arg_sec = /[^ '"\t\r\n>]|\\\\"|\\\\'/ let cdot = /\\\\./ let cl = /\\\\\n/ From dcca05e537b12a774302425a1e4b420d464814c2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 27 Nov 2015 10:30:23 -0800 Subject: [PATCH 85/87] py26reqs.txt needs to be path-relative Fixes: #1630 --- letsencrypt-auto | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index 083de58c4..e9b7739d2 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -14,6 +14,10 @@ XDG_DATA_HOME=${XDG_DATA_HOME:-~/.local/share} VENV_NAME="letsencrypt" VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"} VENV_BIN=${VENV_PATH}/bin +# The path to the letsencrypt-auto script. Everything that uses these might +# at some point be inlined... +LEA_PATH=`dirname "$0"` +BOOTSTRAP=${LEA_PATH}/bootstrap # This script takes the same arguments as the main letsencrypt program, but it # additionally responds to --verbose (more output) and --debug (allow support @@ -110,7 +114,6 @@ DeterminePythonVersion() { # later steps, causing "ImportError: cannot import name unpack_url" if [ ! -d $VENV_PATH ] then - BOOTSTRAP=`dirname $0`/bootstrap if [ ! -f $BOOTSTRAP/debian.sh ] ; then echo "Cannot find the letsencrypt bootstrap scripts in $BOOTSTRAP" exit 1 @@ -172,7 +175,7 @@ if [ "$VERBOSE" = 1 ] ; then echo $VENV_BIN/pip install -U setuptools $VENV_BIN/pip install -U pip - $VENV_BIN/pip install -r py26reqs.txt -U letsencrypt letsencrypt-apache + $VENV_BIN/pip install -r "$LEA_PATH"/py26reqs.txt -U letsencrypt letsencrypt-apache # nginx is buggy / disabled for now, but upgrade it if the user has # installed it manually if $VENV_BIN/pip freeze | grep -q letsencrypt-nginx ; then @@ -184,7 +187,7 @@ else $VENV_BIN/pip install -U pip > /dev/null printf . # nginx is buggy / disabled for now... - $VENV_BIN/pip install -r py26reqs.txt > /dev/null + $VENV_BIN/pip install -r "$LEA_PATH"/py26reqs.txt > /dev/null printf . $VENV_BIN/pip install -U letsencrypt > /dev/null printf . From 218379c2be8c5d7b9cb3e13d1dba57ece40dd9bc Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 29 Nov 2015 09:26:03 +0000 Subject: [PATCH 86/87] poll_and_ri: handle STATUS_INVALID, add max_attempts (fixes #1634) --- acme/acme/client.py | 30 ++++++++++++++++++------------ acme/acme/client_test.py | 24 +++++++++++++++++++----- acme/acme/errors.py | 28 ++++++++++++++++++++++++++++ acme/acme/errors_test.py | 21 +++++++++++++++++++++ 4 files changed, 86 insertions(+), 17 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 0e9319f9c..08d476783 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -246,9 +246,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes def retry_after(cls, response, default): """Compute next `poll` time based on response ``Retry-After`` header. - :param response: Response from `poll`. - :type response: `requests.Response` - + :param requests.Response response: Response from `poll`. :param int default: Default value (in seconds), used when ``Retry-After`` header is not present or invalid. @@ -323,22 +321,21 @@ class Client(object): # pylint: disable=too-many-instance-attributes body=jose.ComparableX509(OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_ASN1, response.content))) - def poll_and_request_issuance(self, csr, authzrs, mintime=5): + def poll_and_request_issuance( + self, csr, authzrs, mintime=5, max_attempts=10): """Poll and request issuance. This function polls all provided Authorization Resource URIs until all challenges are valid, respecting ``Retry-After`` HTTP headers, and then calls `request_issuance`. - .. todo:: add `max_attempts` or `timeout` - - :param csr: CSR. - :type csr: `OpenSSL.crypto.X509Req` wrapped in `.ComparableX509` - + :param .ComparableX509 csr: CSR (`OpenSSL.crypto.X509Req` + wrapped in `.ComparableX509`) :param authzrs: `list` of `.AuthorizationResource` - :param int mintime: Minimum time before next attempt, used if ``Retry-After`` is not present in the response. + :param int max_attempts: Maximum number of attempts before + `PollError` with non-empty ``waiting`` is raised. :returns: ``(cert, updated_authzrs)`` `tuple` where ``cert`` is the issued certificate (`.messages.CertificateResource.), @@ -348,6 +345,9 @@ class Client(object): # pylint: disable=too-many-instance-attributes as the input ``authzrs``. :rtype: `tuple` + :raises PollError: in case of timeout or if some authorization + was marked by the CA as invalid + """ # priority queue with datetime (based on Retry-After) as key, # and original Authorization Resource as value @@ -356,7 +356,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes # recently updated one updated = dict((authzr, authzr) for authzr in authzrs) - while waiting: + while waiting and max_attempts: + max_attempts -= 1 # find the smallest Retry-After, and sleep if necessary when, authzr = heapq.heappop(waiting) now = datetime.datetime.now() @@ -371,11 +372,16 @@ class Client(object): # pylint: disable=too-many-instance-attributes updated[authzr] = updated_authzr # pylint: disable=no-member - if updated_authzr.body.status != messages.STATUS_VALID: + if updated_authzr.body.status not in ( + messages.STATUS_VALID, messages.STATUS_INVALID): # push back to the priority queue, with updated retry_after heapq.heappush(waiting, (self.retry_after( response, default=mintime), authzr)) + if not max_attempts or any(authzr.body.status == messages.STATUS_INVALID + for authzr in six.itervalues(updated)): + raise errors.PollError(waiting, updated) + updated_authzrs = tuple(updated[authzr] for authzr in authzrs) return self.request_issuance(csr, updated_authzrs), updated_authzrs diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 2df7b5313..58f55b293 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -271,9 +271,9 @@ class ClientTest(unittest.TestCase): # result, increment clock clock.dt += datetime.timedelta(seconds=2) - if not authzr.retries: # no more retries + if len(authzr.retries) == 1: # no more retries done = mock.MagicMock(uri=authzr.uri, times=authzr.times) - done.body.status = messages.STATUS_VALID + done.body.status = authzr.retries[0] return done, [] # response (2nd result tuple element) is reduced to only @@ -289,7 +289,8 @@ class ClientTest(unittest.TestCase): mintime = 7 - def retry_after(response, default): # pylint: disable=missing-docstring + def retry_after(response, default): + # pylint: disable=missing-docstring # check that poll_and_request_issuance correctly passes mintime self.assertEqual(default, mintime) return clock.dt + datetime.timedelta(seconds=response) @@ -302,8 +303,10 @@ class ClientTest(unittest.TestCase): csr = mock.MagicMock() authzrs = ( - mock.MagicMock(uri='a', times=[], retries=(8, 20, 30)), - mock.MagicMock(uri='b', times=[], retries=(5,)), + mock.MagicMock(uri='a', times=[], retries=( + 8, 20, 30, messages.STATUS_VALID)), + mock.MagicMock(uri='b', times=[], retries=( + 5, messages.STATUS_VALID)), ) cert, updated_authzrs = self.client.poll_and_request_issuance( @@ -327,6 +330,17 @@ class ClientTest(unittest.TestCase): ]) self.assertEqual(clock.dt, datetime.datetime(2015, 3, 27, 0, 1, 7)) + # CA sets invalid | TODO: move to a separate test + invalid_authzr = mock.MagicMock(times=[], retries=[messages.STATUS_INVALID]) + self.assertRaises( + errors.PollError, self.client.poll_and_request_issuance, + csr, authzrs=(invalid_authzr,), mintime=mintime) + + # exceeded max_attemps | TODO: move to a separate test + self.assertRaises( + errors.PollError, self.client.poll_and_request_issuance, + csr, authzrs, mintime=mintime, max_attempts=2) + def test_check_cert(self): self.response.headers['Location'] = self.certr.uri self.response.content = CERT_DER diff --git a/acme/acme/errors.py b/acme/acme/errors.py index 9a96ec43a..0385667c7 100644 --- a/acme/acme/errors.py +++ b/acme/acme/errors.py @@ -51,3 +51,31 @@ class MissingNonce(NonceError): return ('Server {0} response did not include a replay ' 'nonce, headers: {1}'.format( self.response.request.method, self.response.headers)) + + +class PollError(ClientError): + """Generic error when polling for authorization fails. + + This might be caused by either timeout (`waiting` will be non-empty) + or by some authorization being invalid. + + :ivar waiting: Priority queue with `datetime.datatime` (based on + ``Retry-After``) as key, and original `.AuthorizationResource` + as value. + :ivar updated: Mapping from original `.AuthorizationResource` + to the most recently updated one + + """ + def __init__(self, waiting, updated): + self.waiting = waiting + self.updated = updated + super(PollError, self).__init__() + + @property + def timeout(self): + """Was the error caused by timeout?""" + return bool(self.waiting) + + def __repr__(self): + return '{0}(waiting={1!r}, updated={2!r})'.format( + self.__class__.__name__, self.waiting, self.updated) diff --git a/acme/acme/errors_test.py b/acme/acme/errors_test.py index 3790d91ed..45b269a0b 100644 --- a/acme/acme/errors_test.py +++ b/acme/acme/errors_test.py @@ -1,4 +1,5 @@ """Tests for acme.errors.""" +import datetime import unittest import mock @@ -29,5 +30,25 @@ class MissingNonceTest(unittest.TestCase): self.assertTrue("{}" in str(self.error)) +class PollErrorTest(unittest.TestCase): + """Tests for acme.errors.PollError.""" + + def setUp(self): + from acme.errors import PollError + self.timeout = PollError( + waiting=[(datetime.datetime(2015, 11, 29), mock.sentinel.AR)], + updated={}) + self.invalid = PollError(waiting=[], updated={ + mock.sentinel.AR: mock.sentinel.AR2}) + + def test_timeout(self): + self.assertTrue(self.timeout.timeout) + self.assertFalse(self.invalid.timeout) + + def test_repr(self): + self.assertEqual('PollError(waiting=[], updated={sentinel.AR: ' + 'sentinel.AR2})', repr(self.invalid)) + + if __name__ == "__main__": unittest.main() # pragma: no cover From 4072ff3e1af6438f66972ccd03c00183619d4586 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 30 Nov 2015 19:53:22 -0800 Subject: [PATCH 87/87] Run RabbitMQ setup during test setup. This was recently introduced on the Boulder side. Note: long-term we want to have the client tests run the same setup steps as Boulder does, with the same script. This is a quick fix to unbreak the build. --- tests/boulder-fetch.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index a2c31b1d9..0d8a3de38 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -33,6 +33,7 @@ wget https://github.com/jsha/boulder-tools/raw/master/goose.gz && \ zcat goose.gz > $GOPATH/bin/goose && \ chmod +x $GOPATH/bin/goose ./test/create_db.sh +go run cmd/rabbitmq-setup/main.go -server amqp://localhost # listenbuddy is needed for ./start.py go get github.com/jsha/listenbuddy cd -