From 86ab9a2afc6de4e51301ee3dc278a17e7ea06413 Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 21 Jan 2016 19:08:38 -0500 Subject: [PATCH 01/73] Make ssl includes be an array of directives instead of an include of a file. This will be the first step to filter out the things that would conflict with earlier declarations --- .../letsencrypt_nginx/configurator.py | 3 +-- letsencrypt-nginx/letsencrypt_nginx/parser.py | 18 +++++++++++++++++- .../tests/configurator_test.py | 13 +++++++------ .../letsencrypt_nginx/tests/parser_test.py | 11 +++++++++++ .../letsencrypt_nginx/tls_sni_01.py | 4 ++-- 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index efa7e08b4..918ba53c2 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -332,8 +332,7 @@ class NginxConfigurator(common.Plugin): snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() ssl_block = [['listen', '{0} ssl'.format(self.config.tls_sni_01_port)], ['ssl_certificate', snakeoil_cert], - ['ssl_certificate_key', snakeoil_key], - ['include', self.parser.loc["ssl_options"]]] + ['ssl_certificate_key', snakeoil_key]] + self.parser.loc["ssl_options"] self.parser.add_server_directives( vhost.filep, vhost.names, ssl_block, replace=False) vhost.ssl = True diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index 3b1dd049e..935bf40dd 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -169,6 +169,18 @@ class NginxParser(object): logger.debug("Could not parse file: %s", item) return trees + def _parse_ssl_options(self, ssl_options): + if ssl_options is not None: + try: + with open(ssl_options) as _file: + return nginxparser.load(_file) + except IOError: + logger.debug("Could not open file: %s", item) + except pyparsing.ParseException: + logger.debug("Could not parse file: %s", item) + else: + return [] + def _set_locations(self, ssl_options): """Set default location for directives. @@ -188,7 +200,7 @@ class NginxParser(object): name = default return {"root": root, "default": default, "listen": listen, - "name": name, "ssl_options": ssl_options} + "name": name, "ssl_options": self._parse_ssl_options(ssl_options)} def _find_config_root(self): """Find the Nginx Configuration Root file.""" @@ -503,6 +515,10 @@ def _add_directive(block, directive, replace): See _add_directives for more documentation. """ + if directive[0] == '#': + block.append(directive) + return + location = -1 # Find the index of a config line where the name of the directive matches # the name of the directive we want to add. diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 4fce33079..f35dcdfc0 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -216,9 +216,9 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '5001 ssl'], ['ssl_certificate', 'example/fullchain.pem'], - ['ssl_certificate_key', 'example/key.pem'], - ['include', self.config.parser.loc["ssl_options"]] - ]]], + ['ssl_certificate_key', 'example/key.pem']] + + util.filter_comments(self.config.parser.loc["ssl_options"]) + ]], parsed_example_conf) self.assertEqual([['server_name', 'somename alias another.alias']], parsed_server_conf) @@ -234,8 +234,9 @@ class NginxConfiguratorTest(util.NginxTest): ['index', 'index.html index.htm']]], ['listen', '5001 ssl'], ['ssl_certificate', '/etc/nginx/fullchain.pem'], - ['ssl_certificate_key', '/etc/nginx/key.pem'], - ['include', self.config.parser.loc["ssl_options"]]]], + ['ssl_certificate_key', '/etc/nginx/key.pem']]+ + util.filter_comments(self.config.parser.loc["ssl_options"]) + ], 2)) def test_get_all_certs_keys(self): @@ -394,6 +395,6 @@ class NginxConfiguratorTest(util.NginxTest): generated_conf = self.config.parser.parsed[example_conf] self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) - + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index b64f1dee3..1190a2326 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -249,5 +249,16 @@ class NginxParserTest(util.NginxTest): ]) self.assertTrue(server['ssl']) + def test_ssl_options_should_be_parsed_ssl_directives(self): + nparser = parser.NginxParser(self.config_path, self.ssl_options) + self.assertEqual(nparser.loc["ssl_options"], + [['ssl_session_cache', 'shared:SSL:1m'], + ['ssl_session_timeout', '1440m'], + ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], + ['ssl_prefer_server_ciphers', 'on'], + ['#', ' Using list of ciphers from "Bulletproof SSL and TLS"'], + ['ssl_ciphers', '"ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384 ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384 ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"'] + ]) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py index e59281c4c..5afc950ad 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py @@ -146,7 +146,6 @@ class NginxTlsSni01(common.TLSSNI01): block.extend([['server_name', achall.response(achall.account_key).z_domain], - ['include', self.configurator.parser.loc["ssl_options"]], # access and error logs necessary for # integration testing (non-root) ['access_log', os.path.join( @@ -155,6 +154,7 @@ class NginxTlsSni01(common.TLSSNI01): self.configurator.config.work_dir, 'error.log')], ['ssl_certificate', self.get_cert_path(achall)], ['ssl_certificate_key', self.get_key_path(achall)], - [['location', '/'], [['root', document_root]]]]) + [['location', '/'], [['root', document_root]]]] + + self.configurator.parser.loc["ssl_options"]) return [['server'], block] From 25674b4d559a630e15ac2941e410bc4ca04586e6 Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Fri, 22 Jan 2016 12:12:29 -0500 Subject: [PATCH 02/73] Change name of session cache in order to minimize risk of conflict --- letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf b/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf index f0081c1fc..3faab8818 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf +++ b/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf @@ -1,4 +1,4 @@ -ssl_session_cache shared:SSL:1m; +ssl_session_cache shared:le_nginx_SSL:1m; ssl_session_timeout 1440m; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; From d53da41f007e7f8750a528554bf4000b3d7c6a03 Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Fri, 22 Jan 2016 15:03:51 -0500 Subject: [PATCH 03/73] Stupid mistake, forgot to change the test --- letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 1190a2326..3fc6a214e 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -252,7 +252,7 @@ class NginxParserTest(util.NginxTest): def test_ssl_options_should_be_parsed_ssl_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) self.assertEqual(nparser.loc["ssl_options"], - [['ssl_session_cache', 'shared:SSL:1m'], + [['ssl_session_cache', 'shared:le_nginx_SSL:1m'], ['ssl_session_timeout', '1440m'], ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], ['ssl_prefer_server_ciphers', 'on'], From 593e220353f720282e112809fddd954e575c6779 Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Fri, 22 Jan 2016 16:28:05 -0500 Subject: [PATCH 04/73] A final small fix, hopefully --- letsencrypt-nginx/letsencrypt_nginx/parser.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index 935bf40dd..bd9bebe08 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -175,11 +175,10 @@ class NginxParser(object): with open(ssl_options) as _file: return nginxparser.load(_file) except IOError: - logger.debug("Could not open file: %s", item) + logger.debug("Could not open file: %s", ssl_options) except pyparsing.ParseException: - logger.debug("Could not parse file: %s", item) - else: - return [] + logger.debug("Could not parse file: %s", ssl_options) + return [] def _set_locations(self, ssl_options): """Set default location for directives. From 578db8b8406382da21db6731f4c63dbbeb9a4b2d Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 21 Jan 2016 19:08:38 -0500 Subject: [PATCH 05/73] Make ssl includes be an array of directives instead of an include of a file. This will be the first step to filter out the things that would conflict with earlier declarations --- .../letsencrypt_nginx/configurator.py | 3 +-- letsencrypt-nginx/letsencrypt_nginx/parser.py | 18 +++++++++++++++++- .../tests/configurator_test.py | 13 +++++++------ .../letsencrypt_nginx/tests/parser_test.py | 11 +++++++++++ .../letsencrypt_nginx/tls_sni_01.py | 4 ++-- 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index efa7e08b4..918ba53c2 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -332,8 +332,7 @@ class NginxConfigurator(common.Plugin): snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() ssl_block = [['listen', '{0} ssl'.format(self.config.tls_sni_01_port)], ['ssl_certificate', snakeoil_cert], - ['ssl_certificate_key', snakeoil_key], - ['include', self.parser.loc["ssl_options"]]] + ['ssl_certificate_key', snakeoil_key]] + self.parser.loc["ssl_options"] self.parser.add_server_directives( vhost.filep, vhost.names, ssl_block, replace=False) vhost.ssl = True diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index 3b1dd049e..935bf40dd 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -169,6 +169,18 @@ class NginxParser(object): logger.debug("Could not parse file: %s", item) return trees + def _parse_ssl_options(self, ssl_options): + if ssl_options is not None: + try: + with open(ssl_options) as _file: + return nginxparser.load(_file) + except IOError: + logger.debug("Could not open file: %s", item) + except pyparsing.ParseException: + logger.debug("Could not parse file: %s", item) + else: + return [] + def _set_locations(self, ssl_options): """Set default location for directives. @@ -188,7 +200,7 @@ class NginxParser(object): name = default return {"root": root, "default": default, "listen": listen, - "name": name, "ssl_options": ssl_options} + "name": name, "ssl_options": self._parse_ssl_options(ssl_options)} def _find_config_root(self): """Find the Nginx Configuration Root file.""" @@ -503,6 +515,10 @@ def _add_directive(block, directive, replace): See _add_directives for more documentation. """ + if directive[0] == '#': + block.append(directive) + return + location = -1 # Find the index of a config line where the name of the directive matches # the name of the directive we want to add. diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 4fce33079..f35dcdfc0 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -216,9 +216,9 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '5001 ssl'], ['ssl_certificate', 'example/fullchain.pem'], - ['ssl_certificate_key', 'example/key.pem'], - ['include', self.config.parser.loc["ssl_options"]] - ]]], + ['ssl_certificate_key', 'example/key.pem']] + + util.filter_comments(self.config.parser.loc["ssl_options"]) + ]], parsed_example_conf) self.assertEqual([['server_name', 'somename alias another.alias']], parsed_server_conf) @@ -234,8 +234,9 @@ class NginxConfiguratorTest(util.NginxTest): ['index', 'index.html index.htm']]], ['listen', '5001 ssl'], ['ssl_certificate', '/etc/nginx/fullchain.pem'], - ['ssl_certificate_key', '/etc/nginx/key.pem'], - ['include', self.config.parser.loc["ssl_options"]]]], + ['ssl_certificate_key', '/etc/nginx/key.pem']]+ + util.filter_comments(self.config.parser.loc["ssl_options"]) + ], 2)) def test_get_all_certs_keys(self): @@ -394,6 +395,6 @@ class NginxConfiguratorTest(util.NginxTest): generated_conf = self.config.parser.parsed[example_conf] self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) - + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index b64f1dee3..1190a2326 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -249,5 +249,16 @@ class NginxParserTest(util.NginxTest): ]) self.assertTrue(server['ssl']) + def test_ssl_options_should_be_parsed_ssl_directives(self): + nparser = parser.NginxParser(self.config_path, self.ssl_options) + self.assertEqual(nparser.loc["ssl_options"], + [['ssl_session_cache', 'shared:SSL:1m'], + ['ssl_session_timeout', '1440m'], + ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], + ['ssl_prefer_server_ciphers', 'on'], + ['#', ' Using list of ciphers from "Bulletproof SSL and TLS"'], + ['ssl_ciphers', '"ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384 ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384 ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"'] + ]) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py index e59281c4c..5afc950ad 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tls_sni_01.py @@ -146,7 +146,6 @@ class NginxTlsSni01(common.TLSSNI01): block.extend([['server_name', achall.response(achall.account_key).z_domain], - ['include', self.configurator.parser.loc["ssl_options"]], # access and error logs necessary for # integration testing (non-root) ['access_log', os.path.join( @@ -155,6 +154,7 @@ class NginxTlsSni01(common.TLSSNI01): self.configurator.config.work_dir, 'error.log')], ['ssl_certificate', self.get_cert_path(achall)], ['ssl_certificate_key', self.get_key_path(achall)], - [['location', '/'], [['root', document_root]]]]) + [['location', '/'], [['root', document_root]]]] + + self.configurator.parser.loc["ssl_options"]) return [['server'], block] From 067e51170296fae8f908f60e357d99cbe62a09bc Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Fri, 22 Jan 2016 12:12:29 -0500 Subject: [PATCH 06/73] Change name of session cache in order to minimize risk of conflict --- letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf b/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf index f0081c1fc..3faab8818 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf +++ b/letsencrypt-nginx/letsencrypt_nginx/options-ssl-nginx.conf @@ -1,4 +1,4 @@ -ssl_session_cache shared:SSL:1m; +ssl_session_cache shared:le_nginx_SSL:1m; ssl_session_timeout 1440m; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; From 613250e1b26f4c28354932340830eb0b09e5e3cb Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Fri, 22 Jan 2016 15:03:51 -0500 Subject: [PATCH 07/73] Stupid mistake, forgot to change the test --- letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 1190a2326..3fc6a214e 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -252,7 +252,7 @@ class NginxParserTest(util.NginxTest): def test_ssl_options_should_be_parsed_ssl_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) self.assertEqual(nparser.loc["ssl_options"], - [['ssl_session_cache', 'shared:SSL:1m'], + [['ssl_session_cache', 'shared:le_nginx_SSL:1m'], ['ssl_session_timeout', '1440m'], ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], ['ssl_prefer_server_ciphers', 'on'], From f4a130eab3529fb8860669ecce3b5d2fd8393dec Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Fri, 22 Jan 2016 16:28:05 -0500 Subject: [PATCH 08/73] A final small fix, hopefully --- letsencrypt-nginx/letsencrypt_nginx/parser.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index 935bf40dd..bd9bebe08 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -175,11 +175,10 @@ class NginxParser(object): with open(ssl_options) as _file: return nginxparser.load(_file) except IOError: - logger.debug("Could not open file: %s", item) + logger.debug("Could not open file: %s", ssl_options) except pyparsing.ParseException: - logger.debug("Could not parse file: %s", item) - else: - return [] + logger.debug("Could not parse file: %s", ssl_options) + return [] def _set_locations(self, ssl_options): """Set default location for directives. From 2b097b0fb2c65939e310d3dd271f1d65a11f8c9e Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Mon, 25 Jan 2016 11:39:46 -0500 Subject: [PATCH 09/73] Fix lint issues --- letsencrypt-nginx/letsencrypt_nginx/parser.py | 2 +- .../letsencrypt_nginx/tests/configurator_test.py | 2 +- .../letsencrypt_nginx/tests/parser_test.py | 11 +++++++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index bd9bebe08..1cf805eef 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -179,7 +179,7 @@ class NginxParser(object): except pyparsing.ParseException: logger.debug("Could not parse file: %s", ssl_options) return [] - + def _set_locations(self, ssl_options): """Set default location for directives. diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index f35dcdfc0..c91372651 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -395,6 +395,6 @@ class NginxConfiguratorTest(util.NginxTest): generated_conf = self.config.parser.parsed[example_conf] self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) - + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 3fc6a214e..d66206a57 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -257,8 +257,15 @@ class NginxParserTest(util.NginxTest): ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], ['ssl_prefer_server_ciphers', 'on'], ['#', ' Using list of ciphers from "Bulletproof SSL and TLS"'], - ['ssl_ciphers', '"ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384 ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384 ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"'] + ['ssl_ciphers', '"ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-'+ + 'AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256'+ + '-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384'+ + ' ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384'+ + ' ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-'+ + 'AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM'+ + '-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-'+ + 'AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"'] ]) - + if __name__ == "__main__": unittest.main() # pragma: no cover From 6ffa14c632775e361fdce96badca09c5e195f93d Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Mon, 25 Jan 2016 14:22:03 -0500 Subject: [PATCH 10/73] Bad merge managed to introduce whitespace again. Sigh. --- letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index f35dcdfc0..c91372651 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -395,6 +395,6 @@ class NginxConfiguratorTest(util.NginxTest): generated_conf = self.config.parser.parsed[example_conf] self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) - + if __name__ == "__main__": unittest.main() # pragma: no cover From 4f2a8f86d888599a7bb12ece864fb9737b6e801f Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 13 Jun 2016 11:52:36 -0700 Subject: [PATCH 11/73] Remove unnecessary check on registration returned. Right now the ACME client checks that the returned registration matches the registation posted, but there's no guarantee this will always be the case, and this only introduces unnecessary fragility. --- acme/acme/client.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 117ee6b7d..de7eef299 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -89,8 +89,6 @@ class Client(object): # pylint: disable=too-many-instance-attributes :returns: Registration Resource. :rtype: `.RegistrationResource` - :raises .UnexpectedUpdate: - """ new_reg = messages.NewRegistration() if new_reg is None else new_reg assert isinstance(new_reg, messages.NewRegistration) @@ -101,12 +99,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes # "Instance of 'Field' has no key/contact member" bug: # pylint: disable=no-member - regr = self._regr_from_response(response) - if (regr.body.key != self.key.public_key() or - regr.body.contact != new_reg.contact): - raise errors.UnexpectedUpdate(regr) - - return regr + return self._regr_from_response(response) def _send_recv_regr(self, regr, body): response = self.net.post(regr.uri, body) From 44113a5d068ef224295994fcd60f9024b0a6854d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jul 2016 17:25:09 -0700 Subject: [PATCH 12/73] Automatically enable EPEL (after prompting users) --- letsencrypt-auto-source/letsencrypt-auto | 24 +++++++++++++++---- .../pieces/bootstrappers/rpm_common.sh | 24 +++++++++++++++---- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 22c914ed8..4abe7be38 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -281,6 +281,26 @@ BootstrapRpmCommon() { exit 1 fi + if [ "$ASSUME_YES" = 1 ]; then + yes_flag="-y" + fi + + if ! $SUDO $tool list *virtualenv > /dev/null 2>&1; then + echo "To use Certbot, packages from the EPEL repository need to be installed." + if [ "$ASSUME_YES" = 1 ]; then + /bin/echo -n "Enabling the EPEL repository in 3 seconds..." + sleep 1s + /bin/echo -ne "\e[0K\rEnabling the EPEL repository in 2 seconds..." + sleep 1s + /bin/echo -e "\e[0K\rEnabling the EPEL repository in 1 seconds..." + sleep 1s + fi + if ! $SUDO $tool install $yes_flag epel-release; then + echo "Could not enable EPEL. Aborting bootstrap!" + exit 1 + fi + fi + pkgs=" gcc dialog @@ -318,10 +338,6 @@ BootstrapRpmCommon() { " fi - if [ "$ASSUME_YES" = 1 ]; then - yes_flag="-y" - fi - if ! $SUDO $tool install $yes_flag $pkgs; then echo "Could not install OS dependencies. Aborting bootstrap!" exit 1 diff --git a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh index 0f98b4bbc..e9865aed3 100755 --- a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh +++ b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh @@ -17,6 +17,26 @@ BootstrapRpmCommon() { exit 1 fi + if [ "$ASSUME_YES" = 1 ]; then + yes_flag="-y" + fi + + if ! $SUDO $tool list *virtualenv > /dev/null 2>&1; then + echo "To use Certbot, packages from the EPEL repository need to be installed." + if [ "$ASSUME_YES" = 1 ]; then + /bin/echo -n "Enabling the EPEL repository in 3 seconds..." + sleep 1s + /bin/echo -ne "\e[0K\rEnabling the EPEL repository in 2 seconds..." + sleep 1s + /bin/echo -e "\e[0K\rEnabling the EPEL repository in 1 seconds..." + sleep 1s + fi + if ! $SUDO $tool install $yes_flag epel-release; then + echo "Could not enable EPEL. Aborting bootstrap!" + exit 1 + fi + fi + pkgs=" gcc dialog @@ -54,10 +74,6 @@ BootstrapRpmCommon() { " fi - if [ "$ASSUME_YES" = 1 ]; then - yes_flag="-y" - fi - if ! $SUDO $tool install $yes_flag $pkgs; then echo "Could not install OS dependencies. Aborting bootstrap!" exit 1 From 9c915b0ae4e42b3c4158b32efe35ae3bdfb1d476 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 14 Jul 2016 18:15:01 -0700 Subject: [PATCH 13/73] Fix tests --- certbot-nginx/certbot_nginx/parser.py | 2 +- certbot-nginx/certbot_nginx/tests/parser_test.py | 2 +- certbot-nginx/certbot_nginx/tests/util.py | 13 ++++++++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index f208d0833..5b6860690 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -177,7 +177,7 @@ class NginxParser(object): if ssl_options is not None: try: with open(ssl_options) as _file: - return nginxparser.load(_file) + return nginxparser.load(_file).spaced except IOError: logger.debug("Could not open file: %s", ssl_options) except pyparsing.ParseException: diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index ddd375d96..d0bc32297 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -251,7 +251,7 @@ class NginxParserTest(util.NginxTest): def test_ssl_options_should_be_parsed_ssl_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) - self.assertEqual(nparser.loc["ssl_options"], + self.assertEqual(nginxparser.UnspacedList(nparser.loc["ssl_options"]), [['ssl_session_cache', 'shared:le_nginx_SSL:1m'], ['ssl_session_timeout', '1440m'], ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py index 866e5a9c7..96fdac527 100644 --- a/certbot-nginx/certbot_nginx/tests/util.py +++ b/certbot-nginx/certbot_nginx/tests/util.py @@ -17,6 +17,7 @@ from certbot.plugins import common from certbot_nginx import constants from certbot_nginx import configurator +from certbot_nginx import nginxparser class NginxTest(unittest.TestCase): # pylint: disable=too-few-public-methods @@ -84,14 +85,20 @@ def filter_comments(tree): def traverse(tree): """Generator dropping comment nodes""" for entry in tree: - key, values = entry + # key, values = entry + spaceless = [e for e in entry if not nginxparser.spacey(e)] + if spaceless: + key = spaceless[0] + values = spaceless[1] if len(spaceless) > 1 else None + else: + key = values = "" if isinstance(key, list): new = copy.deepcopy(entry) new[1] = filter_comments(values) yield new else: - if key != '#': - yield entry + if key != '#' and spaceless: + yield spaceless return list(traverse(tree)) From dbb2398270e611a1f7211cf95f7b8e60ffa3aec0 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 15 Jul 2016 09:25:12 -0700 Subject: [PATCH 14/73] Add _comment_spaced_block --- certbot-nginx/certbot_nginx/options-ssl-nginx.conf | 1 - certbot-nginx/certbot_nginx/parser.py | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/options-ssl-nginx.conf b/certbot-nginx/certbot_nginx/options-ssl-nginx.conf index 3faab8818..89c920b3e 100644 --- a/certbot-nginx/certbot_nginx/options-ssl-nginx.conf +++ b/certbot-nginx/certbot_nginx/options-ssl-nginx.conf @@ -4,5 +4,4 @@ ssl_session_timeout 1440m; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; ssl_prefer_server_ciphers on; -# Using list of ciphers from "Bulletproof SSL and TLS" ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384 ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384 ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"; diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 5b6860690..5bfd9182b 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -566,3 +566,17 @@ def _add_directive(block, directive, replace): directive, block[location])) else: block.append(directive) + +def _comment_spaced_block(block): + """Adds a "managed by Certbot" comment to every directive.""" + comment = " # managed by Certbot" + indent = 80 - len(comment) + for i, entry in enumerate(block): + if isinstance(entry, list): + line = "".join(entry) + line = "".join(c for c in line if c != "\n") + linelength = len(line) + extra = indent - linelength + if extra < 0: + extra = 0 + block[i][-1] += extra * " " + comment From 5d7ef49fac778aa266a8f9d2d5ab9fc3a1030150 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 18 Jul 2016 15:25:09 -0700 Subject: [PATCH 15/73] _add_directive cleanup --- certbot-nginx/certbot_nginx/parser.py | 44 +++++++++++---------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 5bfd9182b..cd256d0c4 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -518,7 +518,9 @@ def _add_directives(block, directives, replace): for directive in directives: _add_directive(block, directive, replace) -repeatable_directives = set(['server_name', 'listen', 'include']) + +REPEATABLE_DIRECTIVES = set(['server_name', 'listen', 'include']) + def _add_directive(block, directive, replace): """Adds or replaces a single directive in a config block. @@ -527,27 +529,20 @@ def _add_directive(block, directive, replace): """ directive = nginxparser.UnspacedList(directive) - if len(directive) == 0: - # whitespace + if len(directive) == 0 or directive[0] == '#': + # whitespace or comment block.append(directive) return - if directive[0] == '#': - block.append(directive) - return - - location = -1 # Find the index of a config line where the name of the directive matches - # the name of the directive we want to add. - for index, line in enumerate(block): - if len(line) > 0 and line[0] == directive[0]: - location = index - break + # the name of the directive we want to add. If no line exists, use None. + location = next((index for index, line in enumerate(block) + if line and line[0] == directive[0]), None) if replace: - if location == -1: + if location is None: raise errors.MisconfigurationError( - 'expected directive for %s in the Nginx ' - 'config but did not find it.' % directive[0]) + 'expected directive for {0} in the Nginx ' + 'config but did not find it.'.format(directive[0])) block[location] = directive else: # Append directive. Fail if the name is not a repeatable directive name, @@ -555,17 +550,14 @@ def _add_directive(block, directive, replace): # in the config file. directive_name = directive[0] directive_value = directive[1] - if location != -1 and directive_name.__str__() not in repeatable_directives: - if block[location][1] == directive_value: - # There's a conflict, but the existing value matches the one we - # want to insert, so it's fine. - pass - else: - raise errors.MisconfigurationError( - 'tried to insert directive "%s" but found conflicting "%s".' % ( - directive, block[location])) - else: + if location is None or (isinstance(directive_name, str) and + directive_name in REPEATABLE_DIRECTIVES): block.append(directive) + elif block[location][1] != directive_value: + raise errors.MisconfigurationError( + 'tried to insert directive "{0}" but found ' + 'conflicting "{1}".'.format(directive, block[location])) + def _comment_spaced_block(block): """Adds a "managed by Certbot" comment to every directive.""" From aa33c0fa83fd7230af457074d4142fb29c452588 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 18 Jul 2016 15:33:28 -0700 Subject: [PATCH 16/73] does it work? --- certbot-nginx/certbot_nginx/parser.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index cd256d0c4..5350e04d7 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -520,6 +520,7 @@ def _add_directives(block, directives, replace): REPEATABLE_DIRECTIVES = set(['server_name', 'listen', 'include']) +COMMENT = [" ", "#", " managed by Certbot"] def _add_directive(block, directive, replace): @@ -544,6 +545,7 @@ def _add_directive(block, directive, replace): 'expected directive for {0} in the Nginx ' 'config but did not find it.'.format(directive[0])) block[location] = directive + block.insert(location + 1, COMMENT) else: # Append directive. Fail if the name is not a repeatable directive name, # and there is already a copy of that directive with a different value @@ -553,6 +555,7 @@ def _add_directive(block, directive, replace): if location is None or (isinstance(directive_name, str) and directive_name in REPEATABLE_DIRECTIVES): block.append(directive) + block.append(COMMENT) elif block[location][1] != directive_value: raise errors.MisconfigurationError( 'tried to insert directive "{0}" but found ' From bd21325fcdd13c88b443233d1d55939d6868eadd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 18 Jul 2016 18:12:44 -0700 Subject: [PATCH 17/73] newline logic --- certbot-nginx/certbot_nginx/parser.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 5350e04d7..57ea2db56 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -517,10 +517,26 @@ def _add_directives(block, directives, replace): """ for directive in directives: _add_directive(block, directive, replace) - + last = block[-1] + if not (isinstance(last, str) and '\n' in last): + block.append('\n') + REPEATABLE_DIRECTIVES = set(['server_name', 'listen', 'include']) -COMMENT = [" ", "#", " managed by Certbot"] +COMMENT_STR = ' managed by Certbot' +COMMENT = [' ', '#', ' managed by Certbot'] + + +def _comment_directive(block, location): + """Add a comment to the end of the line at location.""" + block.insert(location + 1, COMMENT[:]) + + if len(block) > location + 2: # there is a block after us + next_entry = block[location + 2] + if isinstance(next_entry, list): + next_entry = next_entry.spaced[0] + if "\n" not in next_entry: + block.insert(location + 2, '\n') def _add_directive(block, directive, replace): From 5dd8f70e567f4540e75b0cac0c3a4fbc0769b137 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 18 Jul 2016 18:19:14 -0700 Subject: [PATCH 18/73] better newline logic --- certbot-nginx/certbot_nginx/parser.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 57ea2db56..e8fd50452 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -529,14 +529,19 @@ COMMENT = [' ', '#', ' managed by Certbot'] def _comment_directive(block, location): """Add a comment to the end of the line at location.""" + if len(block) > location + 1: # there is a block after us + next_entry = block[location + 1] + else: + # we're at the end of the block, pretend there's a newline after us; it will actually be added later in + # add_directives + next_entry = "\n" + if isinstance(next_entry, list): + if COMMENT[-1] in next_entry[-1]: + return + next_entry = next_entry.spaced[0] block.insert(location + 1, COMMENT[:]) - - if len(block) > location + 2: # there is a block after us - next_entry = block[location + 2] - if isinstance(next_entry, list): - next_entry = next_entry.spaced[0] - if "\n" not in next_entry: - block.insert(location + 2, '\n') + if "\n" not in next_entry: + block.insert(location + 2, '\n') def _add_directive(block, directive, replace): From ed4fc9d2f73631b9fa57f111ac1ee51a7f0fd4c6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 18 Jul 2016 18:20:21 -0700 Subject: [PATCH 19/73] call _comment_directive --- certbot-nginx/certbot_nginx/parser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index e8fd50452..a79136836 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -566,7 +566,7 @@ def _add_directive(block, directive, replace): 'expected directive for {0} in the Nginx ' 'config but did not find it.'.format(directive[0])) block[location] = directive - block.insert(location + 1, COMMENT) + _comment_directive(block, location) else: # Append directive. Fail if the name is not a repeatable directive name, # and there is already a copy of that directive with a different value @@ -576,7 +576,7 @@ def _add_directive(block, directive, replace): if location is None or (isinstance(directive_name, str) and directive_name in REPEATABLE_DIRECTIVES): block.append(directive) - block.append(COMMENT) + _comment_directive(block, len(block)) elif block[location][1] != directive_value: raise errors.MisconfigurationError( 'tried to insert directive "{0}" but found ' From 2ce5b195e54d50881ebb506e070cb74108b38384 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 18 Jul 2016 18:23:54 -0700 Subject: [PATCH 20/73] check certbot --- certbot-nginx/certbot_nginx/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index a79136836..b7bfebb32 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -536,7 +536,7 @@ def _comment_directive(block, location): # add_directives next_entry = "\n" if isinstance(next_entry, list): - if COMMENT[-1] in next_entry[-1]: + if "Certbot" in next_entry[-1]: return next_entry = next_entry.spaced[0] block.insert(location + 1, COMMENT[:]) From e5cb04ee7da2f498ca5e08598980ca9c2e7914d3 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 21 Jul 2016 13:26:57 -0700 Subject: [PATCH 21/73] A couple of fixes --- certbot-nginx/certbot_nginx/parser.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index b7bfebb32..71521110e 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -520,7 +520,7 @@ def _add_directives(block, directives, replace): last = block[-1] if not (isinstance(last, str) and '\n' in last): block.append('\n') - + REPEATABLE_DIRECTIVES = set(['server_name', 'listen', 'include']) COMMENT_STR = ' managed by Certbot' @@ -531,7 +531,7 @@ def _comment_directive(block, location): """Add a comment to the end of the line at location.""" if len(block) > location + 1: # there is a block after us next_entry = block[location + 1] - else: + else: # we're at the end of the block, pretend there's a newline after us; it will actually be added later in # add_directives next_entry = "\n" @@ -576,7 +576,7 @@ def _add_directive(block, directive, replace): if location is None or (isinstance(directive_name, str) and directive_name in REPEATABLE_DIRECTIVES): block.append(directive) - _comment_directive(block, len(block)) + _comment_directive(block, len(block) - 1) elif block[location][1] != directive_value: raise errors.MisconfigurationError( 'tried to insert directive "{0}" but found ' From 85d9ab4d5c2d2afd3c36229a771f77e4536853cf Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 21 Jul 2016 13:39:13 -0700 Subject: [PATCH 22/73] UnspacedList._spaced_position: support the slice at the end fo the list - Which is needed for .insert()ing at the end, for instance. --- certbot-nginx/certbot_nginx/nginxparser.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 1859777d8..a4d4a452f 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -279,6 +279,9 @@ class UnspacedList(list): # Normalize indexes like list[-1] etc, and save the result if idx < 0: idx = len(self) + idx + if idx == len(self): + # not an index, but the slice at the end of the list + return len(self.spaced) if not 0 <= idx < len(self): raise IndexError("list index out of range") idx0 = idx From f98470d4a05620e09eea6992ccd6deda15ffacbf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 26 Jul 2016 17:01:24 -0700 Subject: [PATCH 23/73] Revert "UnspacedList._spaced_position: support the slice at the end fo the list" This reverts commit 85d9ab4d5c2d2afd3c36229a771f77e4536853cf. --- certbot-nginx/certbot_nginx/nginxparser.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index a4d4a452f..1859777d8 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -279,9 +279,6 @@ class UnspacedList(list): # Normalize indexes like list[-1] etc, and save the result if idx < 0: idx = len(self) + idx - if idx == len(self): - # not an index, but the slice at the end of the list - return len(self.spaced) if not 0 <= idx < len(self): raise IndexError("list index out of range") idx0 = idx From 4eb38fe1670bea6d9e49cd7cb8fd3be46a404be8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 26 Jul 2016 17:09:01 -0700 Subject: [PATCH 24/73] Make spaced list handle an insert past the end of the list --- certbot-nginx/certbot_nginx/nginxparser.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 1859777d8..edb280d08 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -215,7 +215,8 @@ class UnspacedList(list): def insert(self, i, x): item, spaced_item = self._coerce(x) - self.spaced.insert(self._spaced_position(i), spaced_item) + self.spaced.insert(self._spaced_position(i) if i < len(self) else i, + spaced_item) list.insert(self, i, item) self.dirty = True From e1f560dca3acde9f10687233eda8e2d7915e68a4 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 26 Jul 2016 17:23:24 -0700 Subject: [PATCH 25/73] Neaten --- certbot-nginx/certbot_nginx/parser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 71521110e..9dcee8bef 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -532,8 +532,8 @@ def _comment_directive(block, location): if len(block) > location + 1: # there is a block after us next_entry = block[location + 1] else: - # we're at the end of the block, pretend there's a newline after us; it will actually be added later in - # add_directives + # we're at the end of the block, pretend there's a newline after us; + # it will actually be added later in add_directives next_entry = "\n" if isinstance(next_entry, list): if "Certbot" in next_entry[-1]: From 1060ea7c3d21693bfe7cbc3aadc92dbc901febe9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 26 Jul 2016 17:36:58 -0700 Subject: [PATCH 26/73] delint --- certbot-nginx/certbot_nginx/parser.py | 4 ++-- certbot-nginx/certbot_nginx/tests/configurator_test.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index c0ce6f185..8500179b1 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -532,8 +532,8 @@ def _comment_directive(block, location): if len(block) > location + 1: # there is a block after us next_entry = block[location + 1] else: - # we're at the end of the block, pretend there's a newline after us; it will actually be added later in - # add_directives + # we're at the end of the block, pretend there's a newline after us; it + # will actually be added later in add_directives next_entry = "\n" if isinstance(next_entry, list): if "Certbot" in next_entry[-1]: diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index d8fc849b7..f77b9d7cd 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -234,7 +234,7 @@ class NginxConfiguratorTest(util.NginxTest): ['index', 'index.html index.htm']]], ['listen', '5001 ssl'], ['ssl_certificate', '/etc/nginx/fullchain.pem'], - ['ssl_certificate_key', '/etc/nginx/key.pem']]+ + ['ssl_certificate_key', '/etc/nginx/key.pem']] + util.filter_comments(self.config.parser.loc["ssl_options"]) ], 2)) From 89758decbb2b1b18e8b56c49c73a6cb1c852e5ec Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 29 Jul 2016 17:28:22 -0700 Subject: [PATCH 27/73] Fix a test --- certbot-nginx/certbot_nginx/tests/configurator_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index a6cd0d588..19dcd157a 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -94,7 +94,8 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*'], - ['listen', '5001 ssl'] + ['listen', '5001 ssl'], + ['#', ' managed by Certbot'] ]]], parsed[0]) From 3a2df72bceeada0a0a4fc7b0cc7df7d926092451 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 5 Aug 2016 15:36:24 -0700 Subject: [PATCH 28/73] Add newlines to the ends of blocks more correctly --- certbot-nginx/certbot_nginx/parser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index ca02b6316..4577f9fa5 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -518,8 +518,8 @@ def _add_directives(block, directives, replace): for directive in directives: _add_directive(block, directive, replace) last = block[-1] - if not (isinstance(last, str) and '\n' in last): - block.append('\n') + if not '\n' in last: # could be " \n " or ["\n"] ! + block.append(nginxparser.UnspacedList('\n')) REPEATABLE_DIRECTIVES = set(['server_name', 'listen', 'include']) From cdc894601c83d75e2038c5bc9d6f97fc6fbc3eef Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 5 Aug 2016 15:36:40 -0700 Subject: [PATCH 29/73] Tolerate our own added newlines --- certbot-nginx/certbot_nginx/parser.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 4577f9fa5..536602e27 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -329,6 +329,8 @@ class NginxParser(object): tup = [None, None, vhost.filep] if vhost.ssl: for directive in vhost.raw: + if not directive: + continue if directive[0] == 'ssl_certificate': tup[0] = directive[1] elif directive[0] == 'ssl_certificate_key': From 460f49778f163fc137bb8eeb64135ec9dcbc9b65 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 5 Aug 2016 15:37:01 -0700 Subject: [PATCH 30/73] Fix tests for our new spacey, commented world --- certbot-nginx/certbot_nginx/tests/parser_test.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index aec6f2a23..09b1cfd0b 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -144,7 +144,10 @@ class NginxParserTest(util.NginxTest): self.assertEqual(nparser.parsed[server_conf], [['server_name', 'somename alias another.alias'], ['foo', 'bar'], - ['ssl_certificate', '/etc/ssl/cert2.pem'] + ['#', ' managed by Certbot'], + ['ssl_certificate', '/etc/ssl/cert2.pem'], + ['#', ' managed by Certbot'], + [], [] ]) def test_add_http_directives(self): @@ -174,8 +177,8 @@ class NginxParserTest(util.NginxTest): nparser.parsed[filep], [[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], - ['server_name', 'foobar.com'], - ['server_name', 'example.*'], + ['server_name', 'foobar.com'], ['#', ' managed by Certbot'], + ['server_name', 'example.*'], [] ]]]) self.assertRaises(errors.MisconfigurationError, nparser.add_server_directives, @@ -256,7 +259,6 @@ class NginxParserTest(util.NginxTest): ['ssl_session_timeout', '1440m'], ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], ['ssl_prefer_server_ciphers', 'on'], - ['#', ' Using list of ciphers from "Bulletproof SSL and TLS"'], ['ssl_ciphers', '"ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-'+ 'AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256'+ '-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384'+ From 7deb1f0ad625ddd2d3603bcd8858ad674ee27fae Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 12:15:18 -0700 Subject: [PATCH 31/73] Fix bug with UnpsacedList.insert to final position - which only applied when the list actually contained spaces --- certbot-nginx/certbot_nginx/nginxparser.py | 5 +++-- .../certbot_nginx/tests/nginxparser_test.py | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 50cd15c29..7e2cd533a 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -227,8 +227,8 @@ class UnspacedList(list): def insert(self, i, x): item, spaced_item = self._coerce(x) - self.spaced.insert(self._spaced_position(i) if i < len(self) else i, - spaced_item) + slicepos = self._spaced_position(i) if i < len(self) else len(self.spaced) + self.spaced.insert(slicepos, spaced_item) list.insert(self, i, item) self.dirty = True @@ -292,6 +292,7 @@ class UnspacedList(list): # Normalize indexes like list[-1] etc, and save the result if idx < 0: idx = len(self) + idx + if not 0 <= idx < len(self): raise IndexError("list index out of range") idx0 = idx diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py index 5fa9a7d1e..5c8d6d215 100644 --- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py +++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py @@ -223,6 +223,26 @@ class TestUnspacedList(unittest.TestCase): self.assertRaises(IndexError, self.ul2.__getitem__, 2) self.assertRaises(IndexError, self.ul2.__getitem__, -3) + def test_insert(self): + x = UnspacedList( + [['\n ', 'listen', ' ', '69.50.225.155:9000'], + ['\n ', 'listen', ' ', '127.0.0.1'], + ['\n ', 'server_name', ' ', '.example.com'], + ['\n ', 'server_name', ' ', 'example.*'], '\n', + ['listen', ' ', '5001 ssl']]) + x.insert(5, "FROGZ") + self.assertEqual(x, + [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], + ['server_name', '.example.com'], ['server_name', 'example.*'], + ['listen', '5001 ssl'], 'FROGZ']) + self.assertEqual(x.spaced, + [['\n ', 'listen', ' ', '69.50.225.155:9000'], + ['\n ', 'listen', ' ', '127.0.0.1'], + ['\n ', 'server_name', ' ', '.example.com'], + ['\n ', 'server_name', ' ', 'example.*'], '\n', + ['listen', ' ', '5001 ssl'], + 'FROGZ']) + def test_rawlists(self): ul3 = copy.deepcopy(self.ul) ul3.insert(0, "some") From da7e429125c7100653e0479fb5ecaf459e9a55d2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 15:14:06 -0700 Subject: [PATCH 32/73] Work around horrible spaciness API usage bug --- certbot-nginx/certbot_nginx/configurator.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 0a8d2a88a..afa8b8087 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -338,11 +338,16 @@ class NginxConfigurator(common.Plugin): """ snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() + options_subblock = self.parser.loc["ssl_options"] + # the options file doesn't have a newline at the beginning, but there + # needs to be one when it's dropped into the file + if "\n" not in options_subblock[0]: + options_subblock[0].insert(0, "\n") ssl_block = ( [['\n ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)], ['\n ', 'ssl_certificate', ' ', snakeoil_cert], ['\n ', 'ssl_certificate_key', ' ', snakeoil_key]] + - self.parser.loc["ssl_options"]) + options_subblock) self.parser.add_server_directives( vhost.filep, vhost.names, ssl_block, replace=False) From f0c2ed305958a8354077090d98ffb2b2bd6d191e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 15:45:49 -0700 Subject: [PATCH 33/73] Lint, improve coverage, rm unused code --- certbot-nginx/certbot_nginx/parser.py | 16 +--------------- .../certbot_nginx/tests/parser_test.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 536602e27..cf1f3c1db 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -540,7 +540,7 @@ def _comment_directive(block, location): if isinstance(next_entry, list): if "Certbot" in next_entry[-1]: return - next_entry = next_entry.spaced[0] + next_entry = next_entry.spaced[0] # pylint: disable=no-member block.insert(location + 1, COMMENT[:]) if "\n" not in next_entry: block.insert(location + 2, '\n') @@ -584,17 +584,3 @@ def _add_directive(block, directive, replace): 'tried to insert directive "{0}" but found ' 'conflicting "{1}".'.format(directive, block[location])) - -def _comment_spaced_block(block): - """Adds a "managed by Certbot" comment to every directive.""" - comment = " # managed by Certbot" - indent = 80 - len(comment) - for i, entry in enumerate(block): - if isinstance(entry, list): - line = "".join(entry) - line = "".join(c for c in line if c != "\n") - linelength = len(line) - extra = indent - linelength - if extra < 0: - extra = 0 - block[i][-1] += extra * " " + comment diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index 09b1cfd0b..963b4c815 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -219,6 +219,23 @@ class NginxParserTest(util.NginxTest): self.assertEqual(winner, parser.get_best_match(target_name, names[i])) + def test_comment_directive(self): + # pylint: disable=protected-access + block = nginxparser.UnspacedList([ + ["\n", "a", " ", "b", "\n"], + ["c", " ", "d"], + ["\n", "e", " ", "f"]]) + from certbot_nginx.parser import _comment_directive, COMMENT + _comment_directive(block, 1) + _comment_directive(block, 0) + self.assertEqual(block.spaced, [ + ["\n", "a", " ", "b", "\n"], + COMMENT, + "\n", + ["c", " ", "d"], + COMMENT, + ["\n", "e", " ", "f"]]) + def test_get_all_certs_keys(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) filep = nparser.abs_path('sites-enabled/example.com') From 712bd9ee6b06d0964f53418fe7cb338d35f59ed1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 17:58:22 -0700 Subject: [PATCH 34/73] Copy nginx options file into integration testing environment --- certbot-nginx/tests/boulder-integration.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot-nginx/tests/boulder-integration.sh b/certbot-nginx/tests/boulder-integration.sh index bd35aee21..53dddf7e8 100755 --- a/certbot-nginx/tests/boulder-integration.sh +++ b/certbot-nginx/tests/boulder-integration.sh @@ -7,6 +7,7 @@ export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx nginx_root="$root/nginx" mkdir $nginx_root root="$nginx_root" ./certbot-nginx/tests/boulder-integration.conf.sh > $nginx_root/nginx.conf +cp certbot-nginx/certbot_nginx/options-ssl-nginx.conf $nginx_root killall nginx || true nginx -c $nginx_root/nginx.conf From b5fa0fbad758ce4680631599de12557f602692cf Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 18:08:11 -0700 Subject: [PATCH 35/73] This is reportedly the correct magic --- certbot-nginx/certbot_nginx/configurator.py | 2 +- certbot-nginx/certbot_nginx/parser.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index afa8b8087..5e415bce6 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -341,7 +341,7 @@ class NginxConfigurator(common.Plugin): options_subblock = self.parser.loc["ssl_options"] # the options file doesn't have a newline at the beginning, but there # needs to be one when it's dropped into the file - if "\n" not in options_subblock[0]: + if options_subblock and "\n" not in options_subblock[0]: options_subblock[0].insert(0, "\n") ssl_block = ( [['\n ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)], diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index cf1f3c1db..73224ea1f 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -179,7 +179,7 @@ class NginxParser(object): with open(ssl_options) as _file: return nginxparser.load(_file).spaced except IOError: - logger.debug("Could not open file: %s", ssl_options) + logger.warn("Missing NGINX TLS options file: %s", ssl_options) except pyparsing.ParseException: logger.debug("Could not parse file: %s", ssl_options) return [] From 9c168017aeccfc28b2a4f22c321753108794f75b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 18:17:02 -0700 Subject: [PATCH 36/73] That was not the correct magic --- certbot-nginx/tests/boulder-integration.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/tests/boulder-integration.sh b/certbot-nginx/tests/boulder-integration.sh index 53dddf7e8..58613d86f 100755 --- a/certbot-nginx/tests/boulder-integration.sh +++ b/certbot-nginx/tests/boulder-integration.sh @@ -7,7 +7,7 @@ export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx nginx_root="$root/nginx" mkdir $nginx_root root="$nginx_root" ./certbot-nginx/tests/boulder-integration.conf.sh > $nginx_root/nginx.conf -cp certbot-nginx/certbot_nginx/options-ssl-nginx.conf $nginx_root +cp certbot-nginx/certbot_nginx/options-ssl-nginx.conf "$root"/conf killall nginx || true nginx -c $nginx_root/nginx.conf From 4a28bb1af7dfa113578c4081f91914d9e420ee93 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 12:37:45 -0700 Subject: [PATCH 37/73] clarify invalid email error in non-interactive --- certbot/display/ops.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/certbot/display/ops.py b/certbot/display/ops.py index 4db6d71e2..901e7cd04 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -48,7 +48,10 @@ def get_email(invalid=False, optional=True): invalid_prefix + msg if invalid else msg) except errors.MissingCommandlineFlag: msg = ("You should register before running non-interactively, " - "or provide --agree-tos and --email flags") + "or provide --agree-tos and --email flags. " + "If you have specified an email with the --email flag, " + "please make sure that you entered it correctly and the " + "domain is valid.") raise errors.MissingCommandlineFlag(msg) if code != display_util.OK: From 0b0eca323cc92a42c492336921a22d7a9ac883d3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 15:36:41 -0700 Subject: [PATCH 38/73] Remove extra newline --- certbot-nginx/certbot_nginx/nginxparser.py | 1 - 1 file changed, 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 7e2cd533a..6f2a3ec70 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -292,7 +292,6 @@ class UnspacedList(list): # Normalize indexes like list[-1] etc, and save the result if idx < 0: idx = len(self) + idx - if not 0 <= idx < len(self): raise IndexError("list index out of range") idx0 = idx From 7fb5cf1cf54c5c5ab10f368d0798ca0fa5164792 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 15:46:31 -0700 Subject: [PATCH 39/73] Catch all pyparsing exceptions --- certbot-nginx/certbot_nginx/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 73224ea1f..229b720b6 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -169,7 +169,7 @@ class NginxParser(object): trees.append(parsed) except IOError: logger.warning("Could not open file: %s", item) - except pyparsing.ParseException: + except pyparsing.ParseBaseException: logger.debug("Could not parse file: %s", item) return trees From 7767975204e82c4764964dd5a84af448b4eaed08 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 16:13:31 -0700 Subject: [PATCH 40/73] move error outside of get_email --- certbot/client.py | 10 ++++++++-- certbot/display/ops.py | 5 +---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 119fb0947..cb8fc623c 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -150,8 +150,14 @@ def perform_registration(acme, config): return acme.register(messages.NewRegistration.from_data(email=config.email)) except messages.Error as e: if e.typ == "urn:acme:error:invalidEmail": - config.namespace.email = display_ops.get_email(invalid=True) - return perform_registration(acme, config) + if config.noninteractive_mode: + msg = ("The email you specified was unable to be verified " + "by acme. Please ensure it is a valid email and " + "attempt registration again.") + raise erros.MissingCommandlineFlag(msg) + else: + config.namespace.email = display_ops.get_email(invalid=True) + return perform_registration(acme, config) else: raise diff --git a/certbot/display/ops.py b/certbot/display/ops.py index 901e7cd04..e8520fe96 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -48,10 +48,7 @@ def get_email(invalid=False, optional=True): invalid_prefix + msg if invalid else msg) except errors.MissingCommandlineFlag: msg = ("You should register before running non-interactively, " - "or provide --agree-tos and --email flags. " - "If you have specified an email with the --email flag, " - "please make sure that you entered it correctly and the " - "domain is valid.") + "or provide --agree-tos and --email flags.") raise errors.MissingCommandlineFlag(msg) if code != display_util.OK: From 6e550f70b0eb6f5661b740b871dd6a6e9c7330d8 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 16:17:51 -0700 Subject: [PATCH 41/73] fix typo --- certbot/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/client.py b/certbot/client.py index cb8fc623c..03525cc0d 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -154,7 +154,7 @@ def perform_registration(acme, config): msg = ("The email you specified was unable to be verified " "by acme. Please ensure it is a valid email and " "attempt registration again.") - raise erros.MissingCommandlineFlag(msg) + raise errors.MissingCommandlineFlag(msg) else: config.namespace.email = display_ops.get_email(invalid=True) return perform_registration(acme, config) From ae23800e538473e9c335d92d15991f13c5f32641 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 16:37:40 -0700 Subject: [PATCH 42/73] Comment code that confused bmw --- certbot-nginx/certbot_nginx/parser.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 229b720b6..703a33cf1 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -329,6 +329,7 @@ class NginxParser(object): tup = [None, None, vhost.filep] if vhost.ssl: for directive in vhost.raw: + # A directive can be an empty list to preserve whitespace if not directive: continue if directive[0] == 'ssl_certificate': From 3d4f822be0e3f6ff0522a48bda8e0b673bbfb26e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 16:41:23 -0700 Subject: [PATCH 43/73] Handle case where block is empty -- not sure if it ever happens, but let's not error out unnecessarily --- certbot-nginx/certbot_nginx/parser.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 703a33cf1..afea71c26 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -520,8 +520,7 @@ def _add_directives(block, directives, replace): """ for directive in directives: _add_directive(block, directive, replace) - last = block[-1] - if not '\n' in last: # could be " \n " or ["\n"] ! + if block and '\n' not in block[-1]: # could be " \n " or ["\n"] ! block.append(nginxparser.UnspacedList('\n')) From 0bf78242148b53420bd50456a7adb07a462c2d91 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 17:28:12 -0700 Subject: [PATCH 44/73] fix test --- certbot/tests/client_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 4a8a8bdee..29718c263 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -67,8 +67,7 @@ class RegisterTest(unittest.TestCase): mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") with mock.patch("certbot.client.acme_client.Client") as mock_client: mock_client().register.side_effect = [mx_err, mock.MagicMock()] - self._call() - self.assertEqual(mock_get_email.call_count, 1) + self.assertRaises(errors.MissingCommandlineFlag, self._call) def test_needs_email(self): self.config.email = None From 7d0b71928c771dd48c8a3795d966fe2fa8882808 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 17:33:17 -0700 Subject: [PATCH 45/73] incorp peter's feedback --- certbot/client.py | 8 ++++---- certbot/tests/client_test.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 03525cc0d..ef59c6ce3 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -151,10 +151,10 @@ def perform_registration(acme, config): except messages.Error as e: if e.typ == "urn:acme:error:invalidEmail": if config.noninteractive_mode: - msg = ("The email you specified was unable to be verified " - "by acme. Please ensure it is a valid email and " - "attempt registration again.") - raise errors.MissingCommandlineFlag(msg) + msg = ("The ACME server believes %s is an invalid email address. " + "Please ensure it is a valid email and attempt " + "registration again." % config.email) + raise errors.Error(msg) else: config.namespace.email = display_ops.get_email(invalid=True) return perform_registration(acme, config) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 29718c263..1ed63f466 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -61,13 +61,13 @@ class RegisterTest(unittest.TestCase): @mock.patch("certbot.account.report_new_account") @mock.patch("certbot.client.display_ops.get_email") - def test_email_retry(self, _rep, mock_get_email): + def test_email_invalid_noninteractive(self, _rep, mock_get_email): from acme import messages msg = "DNS problem: NXDOMAIN looking up MX for example.com" mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") with mock.patch("certbot.client.acme_client.Client") as mock_client: mock_client().register.side_effect = [mx_err, mock.MagicMock()] - self.assertRaises(errors.MissingCommandlineFlag, self._call) + self.assertRaises(errors.Error, self._call) def test_needs_email(self): self.config.email = None From 671d7ee19439a97b19ab215c920f925085f781e1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 17:45:43 -0700 Subject: [PATCH 46/73] Fix up COMMENT constants --- certbot-nginx/certbot_nginx/parser.py | 6 +++--- certbot-nginx/certbot_nginx/tests/parser_test.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index afea71c26..5880074f8 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -525,8 +525,8 @@ def _add_directives(block, directives, replace): REPEATABLE_DIRECTIVES = set(['server_name', 'listen', 'include']) -COMMENT_STR = ' managed by Certbot' -COMMENT = [' ', '#', ' managed by Certbot'] +COMMENT = ' managed by Certbot' +COMMENT_BLOCK = [' ', '#', COMMENT] def _comment_directive(block, location): @@ -541,7 +541,7 @@ def _comment_directive(block, location): if "Certbot" in next_entry[-1]: return next_entry = next_entry.spaced[0] # pylint: disable=no-member - block.insert(location + 1, COMMENT[:]) + block.insert(location + 1, COMMENT_BLOCK[:]) if "\n" not in next_entry: block.insert(location + 2, '\n') diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index 963b4c815..fc6c8ff64 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -225,15 +225,15 @@ class NginxParserTest(util.NginxTest): ["\n", "a", " ", "b", "\n"], ["c", " ", "d"], ["\n", "e", " ", "f"]]) - from certbot_nginx.parser import _comment_directive, COMMENT + from certbot_nginx.parser import _comment_directive, COMMENT_BLOCK _comment_directive(block, 1) _comment_directive(block, 0) self.assertEqual(block.spaced, [ ["\n", "a", " ", "b", "\n"], - COMMENT, + COMMENT_BLOCK, "\n", ["c", " ", "d"], - COMMENT, + COMMENT_BLOCK, ["\n", "e", " ", "f"]]) def test_get_all_certs_keys(self): From ceb5207d56758feb72edfd02c7e56c9c2692ca17 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 18:06:21 -0700 Subject: [PATCH 47/73] lint --- certbot/tests/client_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 1ed63f466..7ff46be05 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -60,8 +60,7 @@ class RegisterTest(unittest.TestCase): self._call() @mock.patch("certbot.account.report_new_account") - @mock.patch("certbot.client.display_ops.get_email") - def test_email_invalid_noninteractive(self, _rep, mock_get_email): + def test_email_invalid_noninteractive(self, _rep): from acme import messages msg = "DNS problem: NXDOMAIN looking up MX for example.com" mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") From 76c2fe579a1c9d49f9ebca6e716b4a2eec76b9d9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 18:30:45 -0700 Subject: [PATCH 48/73] Make _comment_directive more defensive --- certbot-nginx/certbot_nginx/parser.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 5880074f8..d98b5225d 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -531,18 +531,17 @@ COMMENT_BLOCK = [' ', '#', COMMENT] def _comment_directive(block, location): """Add a comment to the end of the line at location.""" - if len(block) > location + 1: # there is a block after us - next_entry = block[location + 1] - else: - # we're at the end of the block, pretend there's a newline after us; - # it will actually be added later in add_directives - next_entry = "\n" - if isinstance(next_entry, list): - if "Certbot" in next_entry[-1]: + next_entry = block[location + 1] if location + 1 < len(block) else None + if isinstance(next_entry, list) and next_entry: + if COMMENT in next_entry[-1]: return - next_entry = next_entry.spaced[0] # pylint: disable=no-member + elif isinstance(next_entry, nginxparser.UnspacedList): + next_entry = next_entry.spaced[0] + else: + next_entry = next_entry[0] + block.insert(location + 1, COMMENT_BLOCK[:]) - if "\n" not in next_entry: + if next_entry is not None and "\n" not in next_entry: block.insert(location + 2, '\n') From 76d17bfd0f84fb42e71fe262cb056def533a71b1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 18:40:05 -0700 Subject: [PATCH 49/73] Avoid modifying parsed ssl_options --- certbot-nginx/certbot_nginx/configurator.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 5e415bce6..1a14a3866 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -338,16 +338,14 @@ class NginxConfigurator(common.Plugin): """ snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() - options_subblock = self.parser.loc["ssl_options"] # the options file doesn't have a newline at the beginning, but there # needs to be one when it's dropped into the file - if options_subblock and "\n" not in options_subblock[0]: - options_subblock[0].insert(0, "\n") ssl_block = ( [['\n ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)], ['\n ', 'ssl_certificate', ' ', snakeoil_cert], - ['\n ', 'ssl_certificate_key', ' ', snakeoil_key]] + - options_subblock) + ['\n ', 'ssl_certificate_key', ' ', snakeoil_key], + ['\n']] + + self.parser.loc["ssl_options"]) self.parser.add_server_directives( vhost.filep, vhost.names, ssl_block, replace=False) From 971d6d75401fa228468ea376829dcdb6072736da Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 18:50:18 -0700 Subject: [PATCH 50/73] Don't hardcode comment added by Certbot --- certbot-nginx/certbot_nginx/tests/parser_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index fc6c8ff64..71807d4f4 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -141,12 +141,13 @@ class NginxParserTest(util.NginxTest): replace=False) nparser.add_server_directives(server_conf, names, [['foo', 'bar']], replace=False) + from certbot_nginx.parser import COMMENT self.assertEqual(nparser.parsed[server_conf], [['server_name', 'somename alias another.alias'], ['foo', 'bar'], - ['#', ' managed by Certbot'], + ['#', COMMENT], ['ssl_certificate', '/etc/ssl/cert2.pem'], - ['#', ' managed by Certbot'], + ['#', COMMENT], [], [] ]) @@ -173,11 +174,12 @@ class NginxParserTest(util.NginxTest): filep = nparser.abs_path('sites-enabled/example.com') nparser.add_server_directives( filep, target, [['server_name', 'foobar.com']], replace=True) + from certbot_nginx.parser import COMMENT self.assertEqual( nparser.parsed[filep], [[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], - ['server_name', 'foobar.com'], ['#', ' managed by Certbot'], + ['server_name', 'foobar.com'], ['#', COMMENT], ['server_name', 'example.*'], [] ]]]) self.assertRaises(errors.MisconfigurationError, From 5ec22438ff24579736d94a4fa831cb3ab015866d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 19:04:05 -0700 Subject: [PATCH 51/73] Make sure mod_ssl_conf exists so it can be parsed --- certbot-nginx/certbot_nginx/configurator.py | 4 ++-- certbot-nginx/tests/boulder-integration.sh | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 1a14a3866..0383e0693 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -117,6 +117,8 @@ class NginxConfigurator(common.Plugin): # Make sure configuration is valid self.config_test() + # temp_install must be run before creating the NginxParser + temp_install(self.mod_ssl_conf) self.parser = parser.NginxParser( self.conf('server-root'), self.mod_ssl_conf) @@ -124,8 +126,6 @@ class NginxConfigurator(common.Plugin): if self.version is None: self.version = self.get_version() - temp_install(self.mod_ssl_conf) - # Entry point in main.py for installing cert def deploy_cert(self, domain, cert_path, key_path, chain_path=None, fullchain_path=None): diff --git a/certbot-nginx/tests/boulder-integration.sh b/certbot-nginx/tests/boulder-integration.sh index 58613d86f..bd35aee21 100755 --- a/certbot-nginx/tests/boulder-integration.sh +++ b/certbot-nginx/tests/boulder-integration.sh @@ -7,7 +7,6 @@ export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx nginx_root="$root/nginx" mkdir $nginx_root root="$nginx_root" ./certbot-nginx/tests/boulder-integration.conf.sh > $nginx_root/nginx.conf -cp certbot-nginx/certbot_nginx/options-ssl-nginx.conf "$root"/conf killall nginx || true nginx -c $nginx_root/nginx.conf From 1aa18a3bad5fb4d435b967a6dcb4cdc5f34809d5 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 19:10:57 -0700 Subject: [PATCH 52/73] Add test to prevent regressing and not copying ssl_options to /etc/letsencrypt --- certbot-nginx/certbot_nginx/tests/configurator_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 800387e57..ec7408f27 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -39,6 +39,8 @@ class NginxConfiguratorTest(util.NginxTest): def test_prepare(self): self.assertEqual((1, 6, 2), self.config.version) self.assertEqual(5, len(self.config.parser.parsed)) + # ensure we successfully parsed a file for ssl_options + self.assertTrue(self.config.parser.loc["ssl_options"]) @mock.patch("certbot_nginx.configurator.util.exe_exists") @mock.patch("certbot_nginx.configurator.subprocess.Popen") From 465aa38143e67d00befda3a59e51c943b1c4f6fb Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 19:33:19 -0700 Subject: [PATCH 53/73] Revert "Catch all pyparsing exceptions" This reverts commit 7fb5cf1cf54c5c5ab10f368d0798ca0fa5164792. --- certbot-nginx/certbot_nginx/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index d98b5225d..681fadc55 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -169,7 +169,7 @@ class NginxParser(object): trees.append(parsed) except IOError: logger.warning("Could not open file: %s", item) - except pyparsing.ParseBaseException: + except pyparsing.ParseException: logger.debug("Could not parse file: %s", item) return trees From 449487e8cbcf4dce88c8ffd6a7d8f64cab3cd0c3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 19:34:16 -0700 Subject: [PATCH 54/73] Catch all pyparsing exceptions --- certbot-nginx/certbot_nginx/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 681fadc55..90bb49aaf 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -180,7 +180,7 @@ class NginxParser(object): return nginxparser.load(_file).spaced except IOError: logger.warn("Missing NGINX TLS options file: %s", ssl_options) - except pyparsing.ParseException: + except pyparsing.ParseBaseException: logger.debug("Could not parse file: %s", ssl_options) return [] From 73fdc08d8348335a883f9a21a00ed082f59e3057 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 16 Aug 2016 21:04:28 -0700 Subject: [PATCH 55/73] don't hardcode certbot comment --- .../certbot_nginx/tests/configurator_test.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index ec7408f27..9e0c0dda5 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -13,6 +13,7 @@ from acme import messages from certbot import achallenges from certbot import errors +from certbot_nginx import parser from certbot_nginx.tests import util @@ -91,14 +92,13 @@ class NginxConfiguratorTest(util.NginxTest): # pylint: disable=protected-access parsed = self.config.parser._parse_files(filep, override=True) - self.assertEqual([[['server'], [ - ['listen', '69.50.225.155:9000'], - ['listen', '127.0.0.1'], - ['server_name', '.example.com'], - ['server_name', 'example.*'], - ['listen', '5001 ssl'], - ['#', ' managed by Certbot'] - ]]], + self.assertEqual([[['server'], + [['listen', '69.50.225.155:9000'], + ['listen', '127.0.0.1'], + ['server_name', '.example.com'], + ['server_name', 'example.*'], + ['listen', '5001 ssl'], + ['#', parser.COMMENT]]]], parsed[0]) def test_choose_vhost(self): From 93047d6579c4869cd4896f15b4d9ddbf47ddcbe5 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 17 Aug 2016 15:49:19 -0700 Subject: [PATCH 56/73] add back in email test --- certbot/tests/client_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 7ff46be05..98d853716 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -59,6 +59,18 @@ class RegisterTest(unittest.TestCase): with mock.patch("certbot.account.report_new_account"): self._call() + @mock.patch("certbot.account.report_new_account") + @mock.patch("certbot.client.display_ops.get_email") + def test_email_retry(self, _rep, mock_get_email): + from acme import messages + self.config.noninteractive_mode = False + msg = "DNS problem: NXDOMAIN looking up MX for example.com" + mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") + with mock.patch("certbot.client.acme_client.Client") as mock_client: + mock_client().register.side_effect = [mx_err, mock.MagicMock()] + self._call() + self.assertEqual(mock_get_email.call_count, 1) + @mock.patch("certbot.account.report_new_account") def test_email_invalid_noninteractive(self, _rep): from acme import messages From 9333be6c8856f1c8c1681f90ea4dd13834e34828 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 17 Aug 2016 16:07:37 -0700 Subject: [PATCH 57/73] Add pyparsing hashes to requirements file --- .../pieces/letsencrypt-auto-requirements.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt index 0c642a33e..d9b51ec03 100644 --- a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt +++ b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt @@ -117,6 +117,15 @@ pyasn1==0.1.9 \ pyopenssl==16.0.0 \ --hash=sha256:5add70cf00273bf957ca31fdb0df9b0ae4639e081897d5f86a0ae1f104901230 \ --hash=sha256:363d10ee43d062285facf4e465f4f5163f9f702f9134f0a5896f134cbb92d17d +pyparsing==2.1.8 \ + --hash=sha256:2f0f5ceb14eccd5aef809d6382e87df22ca1da583c79f6db01675ce7d7f49c18 \ + --hash=sha256:03a4869b9f3493807ee1f1cb405e6d576a1a2ca4d81a982677c0c1ad6177c56b \ + --hash=sha256:ab09aee814c0241ff0c503cff30018219fe1fc14501d89f406f4664a0ec9fbcd \ + --hash=sha256:6e9a7f052f8e26bcf749e4033e3115b6dc7e3c85aafcb794b9a88c9d9ef13c97 \ + --hash=sha256:9f463a6bcc4eeb6c08f1ed84439b17818e2085937c0dee0d7674ac127c67c12b \ + --hash=sha256:3626b4d81cfb300dad57f52f2f791caaf7b06c09b368c0aa7b868e53a5775424 \ + --hash=sha256:367b90cc877b46af56d4580cd0ae278062903f02b8204ab631f5a2c0f50adfd0 \ + --hash=sha256:9f1ea360086cd68681e7f4ca8f1f38df47bf81942a0d76a9673c2d23eff35b13 pyRFC3339==1.0 \ --hash=sha256:eea31835c56e2096af4363a5745a784878a61d043e247d3a6d6a0a32a9741f56 \ --hash=sha256:8dfbc6c458b8daba1c0f3620a8c78008b323a268b27b7359e92a4ae41325f535 From a89dfc7226e58b547ca6915f20c90eec0e52035e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 17 Aug 2016 16:10:21 -0700 Subject: [PATCH 58/73] Add the nginx plugin's hash to certbot-auto during the release process --- tools/release.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/release.sh b/tools/release.sh index c883e3d61..7747b0e2b 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -164,13 +164,13 @@ for module in certbot $subpkgs_modules ; do done # pin pip hashes of the things we just built -for pkg in acme certbot certbot-apache ; do +for pkg in acme certbot certbot-apache certbot-nginx ; do echo $pkg==$version \\ pip hash dist."$version/$pkg"/*.{whl,gz} | grep "^--hash" | python2 -c 'from sys import stdin; input = stdin.read(); print " ", input.replace("\n--hash", " \\\n --hash"),' done > /tmp/hashes.$$ deactivate -if ! wc -l /tmp/hashes.$$ | grep -qE "^\s*9 " ; then +if ! wc -l /tmp/hashes.$$ | grep -qE "^\s*12 " ; then echo Unexpected pip hash output exit 1 fi From 6dce950d6ddd27053c2a7b40599ddf9aa8ef7995 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 17 Aug 2016 16:12:12 -0700 Subject: [PATCH 59/73] Update comment about how to generate requirements file --- .../pieces/letsencrypt-auto-requirements.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt index d9b51ec03..342aa2f88 100644 --- a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt +++ b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt @@ -1,6 +1,7 @@ # This is the flattened list of packages certbot-auto installs. To generate -# this, do `pip install --no-cache-dir -e acme -e . -e certbot-apache`, and -# then use `hashin` or a more secure method to gather the hashes. +# this, do +# `pip install --no-cache-dir -e acme -e . -e certbot-apache -e certbot-nginx`, +# and then use `hashin` or a more secure method to gather the hashes. argparse==1.4.0 \ --hash=sha256:c31647edb69fd3d465a847ea3157d37bed1f95f19760b11a47aa91c04b666314 \ From 4e1830b372d9ca0c3c9bb8244071723e5336c1c2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 17 Aug 2016 16:27:23 -0700 Subject: [PATCH 60/73] hide the nginx plugin --- certbot-nginx/certbot_nginx/configurator.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index a1c24b5c8..298bf7f69 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -57,6 +57,8 @@ class NginxConfigurator(common.Plugin): description = "Nginx Web Server - currently doesn't work" + hidden = True + @classmethod def add_parser_arguments(cls, add): add("server-root", default=constants.CLI_DEFAULTS["server_root"], From 9fd003cd664edbf17b866a850206cfaeb0062226 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 17 Aug 2016 16:37:01 -0700 Subject: [PATCH 61/73] Mark the Nginx plugin as alpha --- certbot-nginx/certbot_nginx/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 298bf7f69..049ba9a20 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -55,7 +55,7 @@ class NginxConfigurator(common.Plugin): """ - description = "Nginx Web Server - currently doesn't work" + description = "Nginx Web Server plugin - Alpha" hidden = True From 5c16b43221a903cac615287998e4c9f6772909ac Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 17 Aug 2016 17:00:51 -0700 Subject: [PATCH 62/73] satisfy OCD by removing space --- letsencrypt-auto-source/letsencrypt-auto | 2 +- letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 4abe7be38..5aa49ccf2 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -285,7 +285,7 @@ BootstrapRpmCommon() { yes_flag="-y" fi - if ! $SUDO $tool list *virtualenv > /dev/null 2>&1; then + if ! $SUDO $tool list *virtualenv >/dev/null 2>&1; then echo "To use Certbot, packages from the EPEL repository need to be installed." if [ "$ASSUME_YES" = 1 ]; then /bin/echo -n "Enabling the EPEL repository in 3 seconds..." diff --git a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh index e9865aed3..3c12a5f4d 100755 --- a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh +++ b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh @@ -21,7 +21,7 @@ BootstrapRpmCommon() { yes_flag="-y" fi - if ! $SUDO $tool list *virtualenv > /dev/null 2>&1; then + if ! $SUDO $tool list *virtualenv >/dev/null 2>&1; then echo "To use Certbot, packages from the EPEL repository need to be installed." if [ "$ASSUME_YES" = 1 ]; then /bin/echo -n "Enabling the EPEL repository in 3 seconds..." From 156c6415c2a6472e5e96a3e1150d399fb0b6ab74 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 17 Aug 2016 17:31:56 -0700 Subject: [PATCH 63/73] error out when we can't simply install epel-release --- letsencrypt-auto-source/letsencrypt-auto | 4 ++++ letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 5aa49ccf2..7c851ffc4 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -287,6 +287,10 @@ BootstrapRpmCommon() { if ! $SUDO $tool list *virtualenv >/dev/null 2>&1; then echo "To use Certbot, packages from the EPEL repository need to be installed." + if ! $SUDO $tool list epel-release >/dev/null 2>&1; then + echo "Please enable this repository and try running Certbot again." + exit 1 + fi if [ "$ASSUME_YES" = 1 ]; then /bin/echo -n "Enabling the EPEL repository in 3 seconds..." sleep 1s diff --git a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh index 3c12a5f4d..2fd629ff8 100755 --- a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh +++ b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh @@ -23,6 +23,10 @@ BootstrapRpmCommon() { if ! $SUDO $tool list *virtualenv >/dev/null 2>&1; then echo "To use Certbot, packages from the EPEL repository need to be installed." + if ! $SUDO $tool list epel-release >/dev/null 2>&1; then + echo "Please enable this repository and try running Certbot again." + exit 1 + fi if [ "$ASSUME_YES" = 1 ]; then /bin/echo -n "Enabling the EPEL repository in 3 seconds..." sleep 1s From 5500004dd60c375efcf44d0afb86e428b42a4ed1 Mon Sep 17 00:00:00 2001 From: Jeroen Pluimers Date: Thu, 18 Aug 2016 10:38:47 +0200 Subject: [PATCH 64/73] Fix the links for #3416 and align `README.rst` & `docs/resources.rst` Fix the links for #3416 and align content of in README.rst and docs/resources.rst so it's easier to later de-dupe (I've not done this now as in README.rst does too much tag: fiddling and I'm not sure how that will work out if the fiddling is not aware of .. include::. --- README.rst | 8 +++++++- docs/resources.rst | 10 +++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/README.rst b/README.rst index 0b8e652c0..174dc7dcc 100644 --- a/README.rst +++ b/README.rst @@ -30,10 +30,12 @@ If you'd like to contribute to this project please read `Developer Guide Installation ------------ -The easiest way to install Certbot is by visiting https://certbot.eff.org, where you can +The easiest way to install Certbot is by visiting `certbot.eff.org`_, where you can find the correct installation instructions for many web server and OS combinations. For more information, see the `User Guide `_. +.. _certbot.eff.org: https://certbot.eff.org/ + How to run the client --------------------- @@ -94,6 +96,10 @@ ACME working area in github: https://github.com/ietf-wg-acme/acme Mailing list: `client-dev`_ (to subscribe without a Google account, send an email to client-dev+subscribe@letsencrypt.org) +.. _Freenode: https://webchat.freenode.net?channels=%23letsencrypt +.. _OFTC: https://webchat.oftc.net?channels=%23certbot +.. _client-dev: https://groups.google.com/a/letsencrypt.org/forum/#!forum/client-dev + |build-status| |coverage| |docs| |container| diff --git a/docs/resources.rst b/docs/resources.rst index a284f4a3d..f10aa2920 100644 --- a/docs/resources.rst +++ b/docs/resources.rst @@ -24,6 +24,10 @@ ACME working area in github: https://github.com/ietf-wg-acme/acme Mailing list: `client-dev`_ (to subscribe without a Google account, send an email to client-dev+subscribe@letsencrypt.org) +.. _Freenode: https://webchat.freenode.net?channels=%23letsencrypt +.. _OFTC: https://webchat.oftc.net?channels=%23certbot +.. _client-dev: https://groups.google.com/a/letsencrypt.org/forum/#!forum/client-dev + |build-status| |coverage| |docs| |container| @@ -45,10 +49,6 @@ email to client-dev+subscribe@letsencrypt.org) :alt: Docker Repository on Quay.io .. _`installation instructions`: - https://letsencrypt.readthedocs.org/en/latest/using.html + https://letsencrypt.readthedocs.org/en/latest/using.html#getting-certbot .. _watch demo video: https://www.youtube.com/watch?v=Gas_sSB-5SU - -.. _Freenode: https://webchat.freenode.net?channels=%23letsencrypt -.. _OFTC: https://webchat.oftc.net?channels=%23certbot -.. _client-dev: https://groups.google.com/a/letsencrypt.org/forum/#!forum/client-dev From df61b0e3497412238f841abb07e8ef148bbb4ba5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 18 Aug 2016 13:56:15 -0700 Subject: [PATCH 65/73] Check for comments more accurately --- certbot-nginx/certbot_nginx/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 90bb49aaf..3919858d9 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -533,7 +533,7 @@ def _comment_directive(block, location): """Add a comment to the end of the line at location.""" next_entry = block[location + 1] if location + 1 < len(block) else None if isinstance(next_entry, list) and next_entry: - if COMMENT in next_entry[-1]: + if len(next_entry) >= 2 and next_entry[-2] == "#" and COMMENT in next_entry[-1]: return elif isinstance(next_entry, nginxparser.UnspacedList): next_entry = next_entry.spaced[0] From 702ed89007b7458d08bd531a9c2e2879d10f2ee5 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 18 Aug 2016 14:49:11 -0700 Subject: [PATCH 66/73] use six instead of custom refresh function --- certbot/plugins/util_test.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index e1a064fb3..63a472fa0 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -4,13 +4,7 @@ import unittest import sys import mock - -try: - # Python 3.5+ - from importlib import reload as refresh # pylint: disable=no-name-in-module -except ImportError: - # Python 2-3.4 - from imp import reload as refresh +from six.moves import reload_module # pylint: disable=import-error class PathSurgeryTest(unittest.TestCase): @@ -50,14 +44,14 @@ class AlreadyListeningTestNoPsutil(unittest.TestCase): sys.modules['psutil'] = None # Reload hackery to ensure getting non-psutil version # loaded to memory - refresh(certbot.plugins.util) + reload_module(certbot.plugins.util) def tearDown(self): # Need to reload the module to ensure # getting back to normal import certbot.plugins.util sys.modules["psutil"] = self.psutil - refresh(certbot.plugins.util) + reload_module(certbot.plugins.util) @mock.patch("certbot.plugins.util.zope.component.getUtility") def test_ports_available(self, mock_getutil): From 2c411056fab281437dcaefe4bd4f9508109567a2 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 19 Aug 2016 11:54:35 -0700 Subject: [PATCH 67/73] Remove obsolete test. --- acme/acme/client_test.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index a526a0984..585576e2d 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -102,12 +102,6 @@ class ClientTest(unittest.TestCase): self.assertEqual(self.regr, self.client.register(self.new_reg)) # TODO: test POST call arguments - # TODO: split here and separate test - reg_wrong_key = self.regr.body.update(key=KEY2.public_key()) - self.response.json.return_value = reg_wrong_key.to_json() - self.assertRaises( - errors.UnexpectedUpdate, self.client.register, self.new_reg) - def test_register_missing_next(self): self.response.status_code = http_client.CREATED self.assertRaises( From df68b44d38f99a0cb80dd7056d657ec00d4587f5 Mon Sep 17 00:00:00 2001 From: DanCld Date: Sun, 21 Aug 2016 21:50:14 +0300 Subject: [PATCH 68/73] Fix apache logs dir for centos --- certbot-apache/certbot_apache/configurator.py | 7 +++++-- certbot-apache/certbot_apache/constants.py | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 17ff6c8db..30642af52 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -98,6 +98,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): help="Apache server root directory.") add("vhost-root", default=constants.os_constant("vhost_root"), help="Apache server VirtualHost configuration root") + add("logs-root", default=constants.os_constant("logs_root"), + help="Apache server logs directory") add("challenge-location", default=constants.os_constant("challenge_location"), help="Directory path for challenge configuration.") @@ -1425,13 +1427,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "RewriteEngine On\n" "RewriteRule %s\n" "\n" - "ErrorLog /var/log/apache2/redirect.error.log\n" + "ErrorLog %s/redirect.error.log\n" "LogLevel warn\n" "\n" % (" ".join(str(addr) for addr in self._get_proposed_addrs(ssl_vhost)), servername, serveralias, - " ".join(rewrite_rule_args))) + " ".join(rewrite_rule_args), + self.conf("logs-root") )) def _write_out_redirect(self, ssl_vhost, text): # This is the default name diff --git a/certbot-apache/certbot_apache/constants.py b/certbot-apache/certbot_apache/constants.py index ba545c613..dcc635c4b 100644 --- a/certbot-apache/certbot_apache/constants.py +++ b/certbot-apache/certbot_apache/constants.py @@ -6,6 +6,7 @@ CLI_DEFAULTS_DEFAULT = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", vhost_files="*", + logs_root="/var/log/apache2", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apache2ctl', 'graceful'], @@ -23,6 +24,7 @@ CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", vhost_files="*", + logs_root="/var/log/apache2", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apache2ctl', 'graceful'], @@ -40,6 +42,7 @@ CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", vhost_root="/etc/httpd/conf.d", vhost_files="*.conf", + logs_root="/var/log/httpd", version_cmd=['apachectl', '-v'], define_cmd=['apachectl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apachectl', 'graceful'], @@ -57,6 +60,7 @@ CLI_DEFAULTS_GENTOO = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", vhost_files="*.conf", + logs_root="/var/log/apache2", version_cmd=['/usr/sbin/apache2', '-v'], define_cmd=['apache2ctl', 'virtualhosts'], restart_cmd=['apache2ctl', 'graceful'], @@ -74,6 +78,7 @@ CLI_DEFAULTS_DARWIN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/other", vhost_files="*.conf", + logs_root="/var/log/apache2", version_cmd=['/usr/sbin/httpd', '-v'], define_cmd=['/usr/sbin/httpd', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apachectl', 'graceful'], @@ -91,6 +96,7 @@ CLI_DEFAULTS_SUSE = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", vhost_files="*.conf", + logs_root="/var/log/apache2", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apache2ctl', 'graceful'], From ed7c022565a9ee7290fbab35e81c176d88578d04 Mon Sep 17 00:00:00 2001 From: DanCld Date: Mon, 22 Aug 2016 08:16:20 +0300 Subject: [PATCH 69/73] Lint fix, space before parentheses --- certbot-apache/certbot_apache/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 30642af52..75fbe3456 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1434,7 +1434,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): addr in self._get_proposed_addrs(ssl_vhost)), servername, serveralias, " ".join(rewrite_rule_args), - self.conf("logs-root") )) + self.conf("logs-root"))) def _write_out_redirect(self, ssl_vhost, text): # This is the default name From 3fe5d9c3e0bd92ea82a2c6c1cf92baff79ed5e48 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 23 Aug 2016 11:39:35 -0700 Subject: [PATCH 70/73] add custom skipUnless function --- certbot/plugins/util_test.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index 63a472fa0..dcb21e037 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -75,6 +75,42 @@ class AlreadyListeningTestNoPsutil(unittest.TestCase): self.assertEqual(mock_getutil.call_count, 2) +def psutil_available(): + """Checks if psutil can be imported. + + :rtype: bool + :returns: True iff psutil is installed and can be imported + + """ + try: + import psutil # pylint: disable=unused-variable + except ImportError: + return False + return True + + +def skipUnless(condition, reason): + """Skip tests unless a condition holds. + + This implements the basic functionality of unittest.skipUnless + which is only available on Python 2.7+. + + :param bool condition: skip the test iff condition is False + :param str reason: the reason for skipping the test + + :rtype: function + :returns: a decorator that will hide tests unless condition is True + + """ + if hasattr(unittest, "skipUnless"): + return unittest.skipUnless(condition, reason) + elif condition: + return lambda x: x + else: + return lambda x: None + + +@skipUnless(psutil_available(), "optional dependency psutil is not available") class AlreadyListeningTestPsutil(unittest.TestCase): """Tests for certbot.plugins.already_listening.""" def _call(self, *args, **kwargs): From a69ad2afa80bbdd63e32f90e3bfe78d79c33eefe Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 23 Aug 2016 11:52:28 -0700 Subject: [PATCH 71/73] Give lambda parameter a more useful name --- certbot/plugins/util_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index dcb21e037..cb17cebe2 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -105,9 +105,9 @@ def skipUnless(condition, reason): if hasattr(unittest, "skipUnless"): return unittest.skipUnless(condition, reason) elif condition: - return lambda x: x + return lambda cls: cls else: - return lambda x: None + return lambda cls: None @skipUnless(psutil_available(), "optional dependency psutil is not available") From ad45a664a856b21083dcb5a2e31a3f89ea7823a2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 24 Aug 2016 08:26:23 -0700 Subject: [PATCH 72/73] use "iff" iff it is common international shorthand --- certbot/plugins/util_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index cb17cebe2..58dcfdd38 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -79,7 +79,7 @@ def psutil_available(): """Checks if psutil can be imported. :rtype: bool - :returns: True iff psutil is installed and can be imported + :returns: True if psutil can be imported, otherwise, False """ try: @@ -95,7 +95,7 @@ def skipUnless(condition, reason): This implements the basic functionality of unittest.skipUnless which is only available on Python 2.7+. - :param bool condition: skip the test iff condition is False + :param bool condition: If False, the test will be skipped :param str reason: the reason for skipping the test :rtype: function From 80bcf9a67863ffebcd8685ed264f19f4e0c8ff62 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 24 Aug 2016 08:35:53 -0700 Subject: [PATCH 73/73] make docstring prettier when converted to html --- certbot/plugins/util_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index 58dcfdd38..27ede6533 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -79,7 +79,7 @@ def psutil_available(): """Checks if psutil can be imported. :rtype: bool - :returns: True if psutil can be imported, otherwise, False + :returns: ``True`` if psutil can be imported, otherwise, ``False`` """ try: @@ -95,11 +95,11 @@ def skipUnless(condition, reason): This implements the basic functionality of unittest.skipUnless which is only available on Python 2.7+. - :param bool condition: If False, the test will be skipped + :param bool condition: If ``False``, the test will be skipped :param str reason: the reason for skipping the test - :rtype: function - :returns: a decorator that will hide tests unless condition is True + :rtype: callable + :returns: decorator that hides tests unless condition is ``True`` """ if hasattr(unittest, "skipUnless"):