From 74237d101041897231881a5ba55142e7fb255a4d Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 12:00:20 -0500 Subject: [PATCH 1/5] Requires chain_path for nginx versions supporting OCSP stapling --chain-path config is not mandatory, so we require this property if nginx supports OCSP stapling. Alternatively, we could disable OCSP stapling on supported nginx versions if --chain-path is missing. --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 11 ++++++++++- .../letsencrypt_nginx/tests/configurator_test.py | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index aaaf43c5f..99a864141 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/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,6 +136,9 @@ 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 + """ vhost = self.choose_vhost(domain) cert_directives = [['ssl_certificate', fullchain_path], @@ -150,6 +153,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) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 56ad5110c..35a55befd 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -125,6 +125,15 @@ 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(self): server_conf = self.config.parser.abs_path('server.conf') nginx_conf = self.config.parser.abs_path('nginx.conf') From e8fc2eca0139df1d8cde1e23c12eaca184371100 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 15:02:15 -0500 Subject: [PATCH 2/5] nginx plugin requires fullchain_path --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 4 ++++ .../letsencrypt_nginx/tests/configurator_test.py | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 99a864141..59a977f09 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -140,6 +140,10 @@ class NginxConfigurator(common.Plugin): a lack of directives or configuration """ + if not fullchain_path: + raise errors.PluginError( + "--fullchain-path is required for nginx plugin.") + vhost = self.choose_vhost(domain) cert_directives = [['ssl_certificate', fullchain_path], ['ssl_certificate_key', key_path]] diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 35a55befd..bab107582 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -134,6 +134,15 @@ class NginxConfiguratorTest(util.NginxTest): 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') From 5b5051b6ced180c10311cea89b37900d147929c1 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 15:16:30 -0500 Subject: [PATCH 3/5] The notes should display the fullchain_path See d01b17f1 and dd8c6d65 --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 59a977f09..963ae8a42 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -181,7 +181,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 ####################### From 0b9f505ed5beb7a861c9cb027b5e4349a67eb60d Mon Sep 17 00:00:00 2001 From: Fan Jiang Date: Mon, 4 Jan 2016 15:29:26 -0500 Subject: [PATCH 4/5] update document for --chain-path when required by Nginx >= 1.3.7 --- docs/using.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/using.rst b/docs/using.rst index 5da13f02c..bd40dec02 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -237,7 +237,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 - `_. + `_, + and what nginx >= 1.3.7 needs for `ssl_trusted_certificate + `_. ``fullchain.pem`` All certificates, **including** server certificate. This is From 858dadd85b2b387f8e5c0485e370efa57bb7fe18 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Wed, 6 Jan 2016 13:36:52 -0500 Subject: [PATCH 5/5] Update error message This is supposed to not happen once #1391 is fixed. --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 963ae8a42..43ad36c7f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -142,7 +142,8 @@ class NginxConfigurator(common.Plugin): """ if not fullchain_path: raise errors.PluginError( - "--fullchain-path is required for nginx plugin.") + "The nginx plugin currently requires --fullchain-path to " + "install a cert.") vhost = self.choose_vhost(domain) cert_directives = [['ssl_certificate', fullchain_path],