diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index f3d2b5f9a..a3c2bd186 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -163,7 +163,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): temp_install(self.mod_ssl_conf) - def deploy_cert(self, domain, cert_path, key_path, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, + chain_path=None, fullchain_path=None): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. Currently tries to find the last directives to deploy the cert in diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py index 0d3dbb1b5..bcc66b98a 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py @@ -175,12 +175,13 @@ class Proxy(configurators_common.Proxy): else: return {"example.com"} - def deploy_cert(self, domain, cert_path, key_path, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, chain_path=None, + fullchain_path=None): """Installs cert""" cert_path, key_path, chain_path = self.copy_certs_and_keys( cert_path, key_path, chain_path) self._apache_configurator.deploy_cert( - domain, cert_path, key_path, chain_path) + domain, cert_path, key_path, chain_path, fullchain_path) def _is_apache_command(command): diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index a88607e58..ffca041ca 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -117,30 +117,44 @@ class NginxConfigurator(common.Plugin): 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): + def deploy_cert(self, domain, cert_path, key_path, + chain_path, fullchain_path): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. .. note:: Aborts if the vhost is missing ssl_certificate or ssl_certificate_key. - .. note:: Nginx doesn't have a cert chain directive, so the last - parameter is always ignored. It expects the cert file to have - the concatenated chain. + .. note:: Nginx doesn't have a cert chain directive. + It expects the cert file to have the concatenated chain. + However, we use the chain file as input to the + ssl_trusted_certificate directive, used for verify OCSP responses. .. note:: This doesn't save the config files! """ vhost = self.choose_vhost(domain) - directives = [['ssl_certificate', cert_path], - ['ssl_certificate_key', key_path]] + cert_directives = [['ssl_certificate', fullchain_path], + ['ssl_certificate_key', key_path]] + + # OCSP stapling was introduced in Nginx 1.3.7. If we have that version + # or greater, add config settings for it. + stapling_directives = [] + if self.version >= (1, 3, 7): + stapling_directives = [ + ['ssl_trusted_certificate', chain_path], + ['ssl_stapling', 'on'], + ['ssl_stapling_verify', 'on']] try: self.parser.add_server_directives(vhost.filep, vhost.names, - directives, True) + cert_directives, True) + self.parser.add_server_directives(vhost.filep, vhost.names, + stapling_directives, False) logger.info("Deployed Certificate to VirtualHost %s for %s", vhost.filep, vhost.names) - except errors.MisconfigurationError: + except errors.MisconfigurationError, e: + logger.debug(e) logger.warn( "Cannot find a cert or key directive in %s for %s. " "VirtualHost was not modified.", vhost.filep, vhost.names) diff --git a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py index bd9ca783f..f384082ef 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py +++ b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py @@ -90,14 +90,23 @@ class NginxDvsni(common.Dvsni): # Add the 'include' statement for the challenges if it doesn't exist # already in the main config included = False - directive = ['include', self.challenge_conf] + include_directive = ['include', self.challenge_conf] root = self.configurator.parser.loc["root"] + + bucket_directive = ['server_names_hash_bucket_size', '128'] + main = self.configurator.parser.parsed[root] - for entry in main: - if entry[0] == ['http']: - body = entry[1] - if directive not in body: - body.append(directive) + for key, value in main: + if key == ['http']: + body = value + found_bucket = False + for key, value in body: # pylint: disable=unused-variable + if key == bucket_directive[0]: + found_bucket = True + if not found_bucket: + body.insert(0, bucket_directive) + if include_directive not in body: + body.insert(0, include_directive) included = True break if not included: diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index ef87cc653..fc8ed95f1 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -476,8 +476,12 @@ def _add_directives(block, directives, replace=False): :param list directives: The new directives. """ - if replace: - for directive in directives: + for directive in directives: + if not replace: + # We insert new directives at the top of the block, mostly + # to work around https://trac.nginx.org/nginx/ticket/810 + block.insert(0, directive) + else: changed = False if len(directive) == 0: continue @@ -489,5 +493,3 @@ def _add_directives(block, directives, replace=False): raise errors.MisconfigurationError( 'LetsEncrypt expected directive for %s in the Nginx ' 'config but did not find it.' % directive[0]) - else: - block.extend(directives) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 977a65330..203f9920c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -63,11 +63,11 @@ 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'], + self.assertEqual([[['server'], [['listen', '5001 ssl'], + ['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], - ['server_name', 'example.*'], - ['listen', '5001 ssl']]]], + ['server_name', 'example.*']]]], parsed[0]) def test_choose_vhost(self): @@ -96,18 +96,49 @@ class NginxConfiguratorTest(util.NginxTest): def test_more_info(self): self.assertTrue('nginx.conf' in self.config.more_info()) + def test_deploy_cert_stapling(self): + # Choose a version of Nginx greater than 1.3.7 so stapling code gets + # invoked. + self.config.version = (1, 9, 6) + example_conf = self.config.parser.abs_path('sites-enabled/example.com') + self.config.deploy_cert( + "www.example.com", + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") + self.config.save() + self.config.parser.load() + generated_conf = self.config.parser.parsed[example_conf] + + self.assertTrue(util.contains_at_depth(generated_conf, + ['ssl_stapling', 'on'], 2)) + self.assertTrue(util.contains_at_depth(generated_conf, + ['ssl_stapling_verify', 'on'], 2)) + self.assertTrue(util.contains_at_depth(generated_conf, + ['ssl_trusted_certificate', 'example/chain.pem'], 2)) + def test_deploy_cert(self): server_conf = self.config.parser.abs_path('server.conf') nginx_conf = self.config.parser.abs_path('nginx.conf') example_conf = self.config.parser.abs_path('sites-enabled/example.com') + # Choose a version of Nginx less than 1.3.7 so stapling code doesn't get + # invoked. + self.config.version = (1, 3, 1) # Get the default SSL vhost self.config.deploy_cert( "www.example.com", - "example/cert.pem", "example/key.pem") + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") self.config.deploy_cert( "another.alias", - "/etc/nginx/cert.pem", "/etc/nginx/key.pem") + "/etc/nginx/cert.pem", + "/etc/nginx/key.pem", + "/etc/nginx/chain.pem", + "/etc/nginx/fullchain.pem") self.config.save() self.config.parser.load() @@ -119,35 +150,34 @@ class NginxConfiguratorTest(util.NginxTest): access_log = os.path.join(self.work_dir, "access.log") error_log = os.path.join(self.work_dir, "error.log") self.assertEqual([[['server'], - [['listen', '69.50.225.155:9000'], + [['include', self.config.parser.loc["ssl_options"]], + ['ssl_certificate_key', 'example/key.pem'], + ['ssl_certificate', 'example/fullchain.pem'], + ['error_log', error_log], + ['access_log', access_log], + + ['listen', '5001 ssl'], + ['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], - ['server_name', 'example.*'], - ['listen', '5001 ssl'], - ['access_log', access_log], - ['error_log', error_log], - ['ssl_certificate', 'example/cert.pem'], - ['ssl_certificate_key', 'example/key.pem'], - ['include', - self.config.parser.loc["ssl_options"]]]]], + ['server_name', 'example.*']]]], parsed_example_conf) self.assertEqual([['server_name', 'somename alias another.alias']], parsed_server_conf) - self.assertEqual([['server'], - [['listen', '8000'], - ['listen', 'somename:8080'], - ['include', 'server.conf'], - [['location', '/'], - [['root', 'html'], - ['index', 'index.html index.htm']]], - ['listen', '5001 ssl'], - ['access_log', access_log], - ['error_log', error_log], - ['ssl_certificate', '/etc/nginx/cert.pem'], - ['ssl_certificate_key', '/etc/nginx/key.pem'], - ['include', - self.config.parser.loc["ssl_options"]]]], - parsed_nginx_conf[-1][-1][-1]) + self.assertTrue(util.contains_at_depth(parsed_nginx_conf, + [['server'], + [['include', self.config.parser.loc["ssl_options"]], + ['ssl_certificate_key', '/etc/nginx/key.pem'], + ['ssl_certificate', '/etc/nginx/fullchain.pem'], + ['error_log', error_log], + ['access_log', access_log], + ['listen', '5001 ssl'], + ['listen', '8000'], + ['listen', 'somename:8080'], + ['include', 'server.conf'], + [['location', '/'], + [['root', 'html'], ['index', 'index.html index.htm']]]]], + 2)) def test_get_all_certs_keys(self): nginx_conf = self.config.parser.abs_path('nginx.conf') @@ -156,16 +186,22 @@ class NginxConfiguratorTest(util.NginxTest): # Get the default SSL vhost self.config.deploy_cert( "www.example.com", - "example/cert.pem", "example/key.pem") + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") self.config.deploy_cert( "another.alias", - "/etc/nginx/cert.pem", "/etc/nginx/key.pem") + "/etc/nginx/cert.pem", + "/etc/nginx/key.pem", + "/etc/nginx/chain.pem", + "/etc/nginx/fullchain.pem") self.config.save() self.config.parser.load() self.assertEqual(set([ - ('example/cert.pem', 'example/key.pem', example_conf), - ('/etc/nginx/cert.pem', '/etc/nginx/key.pem', nginx_conf), + ('example/fullchain.pem', 'example/key.pem', example_conf), + ('/etc/nginx/fullchain.pem', '/etc/nginx/key.pem', nginx_conf), ]), self.config.get_all_certs_keys()) @mock.patch("letsencrypt_nginx.configurator.dvsni.NginxDvsni.perform") diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py index a09bebba2..9fc0a1ad7 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py @@ -85,7 +85,8 @@ class DvsniPerformTest(util.NginxTest): # Make sure challenge config is included in main config http = self.sni.configurator.parser.parsed[ self.sni.configurator.parser.loc["root"]][-1] - self.assertTrue(['include', self.sni.challenge_conf] in http[1]) + self.assertTrue( + util.contains_at_depth(http, ['include', self.sni.challenge_conf], 1)) def test_perform2(self): acme_responses = [] @@ -108,7 +109,8 @@ class DvsniPerformTest(util.NginxTest): http = self.sni.configurator.parser.parsed[ self.sni.configurator.parser.loc["root"]][-1] self.assertTrue(['include', self.sni.challenge_conf] in http[1]) - self.assertTrue(['server_name', 'blah'] in http[1][-2][1]) + self.assertTrue( + util.contains_at_depth(http, ['server_name', 'blah'], 3)) self.assertEqual(len(sni_responses), 3) for i in xrange(3): diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 0af81aefe..a22d33e9c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -128,18 +128,18 @@ class NginxParserTest(util.NginxTest): r'~^(www\.)?(example|bar)\.']), [['foo', 'bar'], ['ssl_certificate', '/etc/ssl/cert.pem']]) - ssl_re = re.compile(r'foo bar;\n\s+ssl_certificate /etc/ssl/cert.pem') - self.assertEqual(1, len(re.findall(ssl_re, nginxparser.dumps( - nparser.parsed[nparser.abs_path('nginx.conf')])))) + ssl_re = re.compile(r'\n\s+ssl_certificate /etc/ssl/cert.pem') + dump = nginxparser.dumps(nparser.parsed[nparser.abs_path('nginx.conf')]) + self.assertEqual(1, len(re.findall(ssl_re, dump))) nparser.add_server_directives(nparser.abs_path('server.conf'), set(['alias', 'another.alias', 'somename']), [['foo', 'bar'], ['ssl_certificate', '/etc/ssl/cert2.pem']]) self.assertEqual(nparser.parsed[nparser.abs_path('server.conf')], - [['server_name', 'somename alias another.alias'], + [['ssl_certificate', '/etc/ssl/cert2.pem'], ['foo', 'bar'], - ['ssl_certificate', '/etc/ssl/cert2.pem']]) + ['server_name', 'somename alias another.alias']]) def test_add_http_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) @@ -148,8 +148,9 @@ class NginxParserTest(util.NginxTest): [['listen', '80'], ['server_name', 'localhost']]] nparser.add_http_directives(filep, block) - self.assertEqual(nparser.parsed[filep][-1][0], ['http']) - self.assertEqual(nparser.parsed[filep][-1][1][-1], block) + root = nparser.parsed[filep] + self.assertTrue(util.contains_at_depth(root, ['http'], 1)) + self.assertTrue(util.contains_at_depth(root, block, 2)) def test_replace_server_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index 363944490..ad4dedfe7 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -85,3 +85,18 @@ def filter_comments(tree): yield [key, values] return list(traverse(tree)) + +def contains_at_depth(haystack, needle, n): + """ + Return true if the needle is present in one of the sub-iterables in haystack + at depth n. Haystack must be an iterable. + """ + if not hasattr(haystack, '__iter__'): + return False + if n == 0: + return needle in haystack + else: + for item in haystack: + if contains_at_depth(item, needle, n - 1): + return True + return False diff --git a/letsencrypt-nginx/tests/boulder-integration.conf.sh b/letsencrypt-nginx/tests/boulder-integration.conf.sh index 006d68836..12610d895 100755 --- a/letsencrypt-nginx/tests/boulder-integration.conf.sh +++ b/letsencrypt-nginx/tests/boulder-integration.conf.sh @@ -20,7 +20,6 @@ events { } http { - server_names_hash_bucket_size 2048; # Set an array of temp and cache file options that will otherwise default to # restricted locations accessible only to root. client_body_temp_path $root/client_body; diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 64cba508d..55930e9ff 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -337,9 +337,9 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo lineage = _auth_from_domains(le_client, config, domains, plugins) - # TODO: We also need to pass the fullchain (for Nginx) le_client.deploy_certificate( - domains, lineage.privkey, lineage.cert, lineage.chain) + domains, lineage.privkey, lineage.cert, + lineage.chain, lineage.fullchain) le_client.enhance_config(domains, args.redirect) if len(lineage.available_versions("cert")) == 1: @@ -392,7 +392,8 @@ def install(args, config, plugins): args, config, authenticator=None, installer=installer) assert args.cert_path is not None # required=True in the subparser le_client.deploy_certificate( - domains, args.key_path, args.cert_path, args.chain_path) + domains, args.key_path, args.cert_path, args.chain_path, + args.fullchain_path) le_client.enhance_config(domains, args.redirect) @@ -803,6 +804,8 @@ def _paths_parser(helpful): default_cp = None if verb == "auth": default_cp = flag_default("auth_chain_path") + add("paths", "--fullchain-path", default=default_cp, + help="Accompanying path to a full certificate chain (cert plus chain).") add("paths", "--chain-path", default=default_cp, help="Accompanying path to a certificate chain.") add("paths", "--config-dir", default=flag_default("config_dir"), diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 7a78add38..123bab121 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -336,7 +336,8 @@ class Client(object): return os.path.abspath(act_cert_path), cert_chain_abspath - def deploy_certificate(self, domains, privkey_path, cert_path, chain_path): + def deploy_certificate(self, domains, privkey_path, + cert_path, chain_path, fullchain_path): """Install certificate :param list domains: list of domains to install the certificate @@ -357,8 +358,10 @@ class Client(object): # TODO: Provide a fullchain reference for installers like # nginx that want it self.installer.deploy_cert( - dom, os.path.abspath(cert_path), - os.path.abspath(privkey_path), chain_path) + domain=dom, cert_path=os.path.abspath(cert_path), + key_path=os.path.abspath(privkey_path), + chain_path=chain_path, + fullchain_path=fullchain_path) self.installer.save("Deployed Let's Encrypt Certificate") # sites may have been enabled / final cleanup diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 5e82d61aa..8bf714c88 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -241,13 +241,15 @@ class IInstaller(IPlugin): """ - def deploy_cert(domain, cert_path, key_path, chain_path=None): + def deploy_cert(domain, cert_path, key_path, chain_path, fullchain_path): """Deploy certificate. :param str domain: domain to deploy certificate file :param str cert_path: absolute path to the certificate file :param str key_path: absolute path to the private key file :param str chain_path: absolute path to the certificate chain file + :param str fullchain_path: absolute path to the certificate fullchain + file (cert plus chain) :raises .PluginError: when cert cannot be deployed diff --git a/letsencrypt/plugins/null.py b/letsencrypt/plugins/null.py index 4ba6c9d64..632fe415a 100644 --- a/letsencrypt/plugins/null.py +++ b/letsencrypt/plugins/null.py @@ -30,7 +30,8 @@ class Installer(common.Plugin): def get_all_names(self): return [] - def deploy_cert(self, domain, cert_path, key_path, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, + chain_path=None, fullchain_path=None): pass # pragma: no cover def enhance(self, domain, enhancement, options=None): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 1e63bdbb6..fddb86607 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -177,15 +177,19 @@ class ClientTest(unittest.TestCase): def test_deploy_certificate(self): self.assertRaises(errors.Error, self.client.deploy_certificate, - ["foo.bar"], "key", "cert", "chain") + ["foo.bar"], "key", "cert", "chain", "fullchain") installer = mock.MagicMock() self.client.installer = installer - self.client.deploy_certificate(["foo.bar"], "key", "cert", "chain") + self.client.deploy_certificate(["foo.bar"], "key", "cert", "chain", + "fullchain") installer.deploy_cert.assert_called_once_with( - "foo.bar", os.path.abspath("cert"), - os.path.abspath("key"), os.path.abspath("chain")) + cert_path=os.path.abspath("cert"), + chain_path=os.path.abspath("chain"), + domain='foo.bar', + fullchain_path='fullchain', + key_path=os.path.abspath("key")) self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() diff --git a/tests/boulder-start.sh b/tests/boulder-start.sh index 530f9c598..8780cac7c 100755 --- a/tests/boulder-start.sh +++ b/tests/boulder-start.sh @@ -38,5 +38,5 @@ if ! go get bitbucket.org/liamstask/goose/cmd/goose ; then exit 1 fi ./test/create_db.sh -./start.py & +./start.py > /dev/null & # Hopefully start.py bootstraps before integration test is started...