Nginx improvements

Add a server_names_hash_bucket_size directive during challenges to fix an nginx
crash on restart (Fixes #922).

Use fullchain instead of chain (Fixes #610).

Implement OCSP stapling (Fixes #937, Fixes #931).

Hide Boulder output in integration tests to make them more readable.

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

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):

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)

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:

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)

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")

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):

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)

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

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;

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"),

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

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
 

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):

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()
 

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...