From 578db8b8406382da21db6731f4c63dbbeb9a4b2d Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 21 Jan 2016 19:08:38 -0500 Subject: [PATCH] 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]