Merge pull request #2094 from twstrike/enforce-required-fields

Warn the user when --chain-path or --fullchain-path are missing but are required [minor revision requested]

docs/using.rst

@@ -248,7 +248,9 @@ The following files are available:
   server certificate, i.e. root and intermediate certificates only.
 
   This is what Apache < 2.4.8 needs for `SSLCertificateChainFile
-  <https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile>`_.
+  <https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile>`_,
+  and what nginx >= 1.3.7 needs for `ssl_trusted_certificate
+  <http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_trusted_certificate>`_.
 
 ``fullchain.pem``
   All certificates, **including** server certificate. This is

letsencrypt-nginx/letsencrypt_nginx/configurator.py

@@ -122,7 +122,7 @@ class NginxConfigurator(common.Plugin):
 
     # Entry point in main.py for installing cert
     def deploy_cert(self, domain, cert_path, key_path,
-                    chain_path, fullchain_path):
+                    chain_path=None, fullchain_path=None):
         # pylint: disable=unused-argument
         """Deploys certificate to specified virtual host.
 
@@ -136,7 +136,15 @@ class NginxConfigurator(common.Plugin):
 
         .. note:: This doesn't save the config files!
 
+        :raises errors.PluginError: When unable to deploy certificate due to
+            a lack of directives or configuration
+
         """
+        if not fullchain_path:
+            raise errors.PluginError(
+                "The nginx plugin currently requires --fullchain-path to "
+                "install a cert.")
+
         vhost = self.choose_vhost(domain)
         cert_directives = [['ssl_certificate', fullchain_path],
                            ['ssl_certificate_key', key_path]]
@@ -150,6 +158,12 @@ class NginxConfigurator(common.Plugin):
                 ['ssl_stapling', 'on'],
                 ['ssl_stapling_verify', 'on']]
 
+        if len(stapling_directives) != 0 and not chain_path:
+            raise errors.PluginError(
+                "--chain-path is required to enable "
+                "Online Certificate Status Protocol (OCSP) stapling "
+                "on nginx >= 1.3.7.")
+
         try:
             self.parser.add_server_directives(vhost.filep, vhost.names,
                                               cert_directives, replace=True)
@@ -168,7 +182,7 @@ class NginxConfigurator(common.Plugin):
         self.save_notes += ("Changed vhost at %s with addresses of %s\n" %
                             (vhost.filep,
                              ", ".join(str(addr) for addr in vhost.addrs)))
-        self.save_notes += "\tssl_certificate %s\n" % cert_path
+        self.save_notes += "\tssl_certificate %s\n" % fullchain_path
         self.save_notes += "\tssl_certificate_key %s\n" % key_path
 
     #######################

letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py

@@ -159,6 +159,24 @@ class NginxConfiguratorTest(util.NginxTest):
         self.assertTrue(util.contains_at_depth(generated_conf,
                                                ['ssl_trusted_certificate', 'example/chain.pem'], 2))
 
+    def test_deploy_cert_stapling_requires_chain_path(self):
+        self.config.version = (1, 3, 7)
+        self.assertRaises(errors.PluginError, self.config.deploy_cert,
+            "www.example.com",
+            "example/cert.pem",
+            "example/key.pem",
+            None,
+            "example/fullchain.pem")
+
+    def test_deploy_cert_requires_fullchain_path(self):
+        self.config.version = (1, 3, 1)
+        self.assertRaises(errors.PluginError, self.config.deploy_cert,
+            "www.example.com",
+            "example/cert.pem",
+            "example/key.pem",
+            "example/chain.pem",
+            None)
+
     def test_deploy_cert(self):
         server_conf = self.config.parser.abs_path('server.conf')
         nginx_conf = self.config.parser.abs_path('nginx.conf')