diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aff9670a..ce527dbc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ More details about these changes can be found on our GitHub repo. ### Added -* +* Disable session tickets for Nginx users when appropriate. ### Changed diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index d3de83593..95715916d 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -1,4 +1,6 @@ """Nginx Configuration""" +# https://github.com/PyCQA/pylint/issues/73 +from distutils.version import LooseVersion # pylint: disable=no-name-in-module,import-error import logging import re import socket @@ -91,8 +93,12 @@ class NginxConfigurator(common.Installer): :param tup version: version of Nginx as a tuple (1, 4, 7) (used mostly for unittesting) + :param tup openssl_version: version of OpenSSL linked to Nginx as a tuple (1, 4, 7) + (used mostly for unittesting) + """ version = kwargs.pop("version", None) + openssl_version = kwargs.pop("openssl_version", None) super(NginxConfigurator, self).__init__(*args, **kwargs) # Verify that all directories and files exist with proper permissions @@ -115,6 +121,7 @@ class NginxConfigurator(common.Installer): # These will be set in the prepare function self.parser = None self.version = version + self.openssl_version = openssl_version self._enhance_func = {"redirect": self._enable_redirect, "ensure-http-header": self._set_http_header, "staple-ocsp": self._enable_ocsp_stapling} @@ -124,11 +131,33 @@ class NginxConfigurator(common.Installer): @property def mod_ssl_conf_src(self): """Full absolute path to SSL configuration file source.""" - config_filename = "options-ssl-nginx.conf" - if self.version < (1, 5, 9): - config_filename = "options-ssl-nginx-old.conf" - elif self.version < (1, 13, 0): - config_filename = "options-ssl-nginx-tls12-only.conf" + + # Why all this complexity? Well, we want to support Mozilla's intermediate + # recommendations. But TLS1.3 is only supported by newer versions of Nginx. + # And as for session tickets, our ideal is to turn them off across the board. + # But! Turning them off at all is only supported with new enough versions of + # Nginx. And older versions of OpenSSL have a bug that leads to browser errors + # given certain configurations. While we'd prefer to have forward secrecy, we'd + # rather fail open than error out. Unfortunately, Nginx can be compiled against + # many versions of OpenSSL. So we have to check both for the two different features, + # leading to four different combinations of options. + # For a complete history, check out https://github.com/certbot/certbot/issues/7322 + + use_tls13 = self.version >= (1, 13, 0) + session_tix_off = self.version >= (1, 5, 9) and self.openssl_version and\ + LooseVersion(self.openssl_version) >= LooseVersion('1.0.2l') + + if use_tls13: + if session_tix_off: + config_filename = "options-ssl-nginx.conf" + else: + config_filename = "options-ssl-nginx-tls13-session-tix-on.conf" + else: + if session_tix_off: + config_filename = "options-ssl-nginx-tls12-only.conf" + else: + config_filename = "options-ssl-nginx-old.conf" + return pkg_resources.resource_filename( "certbot_nginx", os.path.join("tls_configs", config_filename)) @@ -169,6 +198,9 @@ class NginxConfigurator(common.Installer): if self.version is None: self.version = self.get_version() + if self.openssl_version is None: + self.openssl_version = self._get_openssl_version() + self.install_ssl_options_conf(self.mod_ssl_conf, self.updated_mod_ssl_conf_digest) self.install_ssl_dhparams() @@ -909,17 +941,14 @@ class NginxConfigurator(common.Installer): util.make_or_verify_dir(self.config.backup_dir, core_constants.CONFIG_DIRS_MODE) util.make_or_verify_dir(self.config.config_dir, core_constants.CONFIG_DIRS_MODE) - def get_version(self): - """Return version of Nginx Server. + def _nginx_version(self): + """Return results of nginx -V - Version is returned as tuple. (ie. 2.4.7 = (2, 4, 7)) - - :returns: version - :rtype: tuple + :returns: version text + :rtype: str :raises .PluginError: - Unable to find Nginx version or version is unsupported - + Unable to run Nginx version command """ try: proc = subprocess.Popen( @@ -932,6 +961,21 @@ class NginxConfigurator(common.Installer): logger.debug(str(error), exc_info=True) raise errors.PluginError( "Unable to run %s -V" % self.conf('ctl')) + return text + + def get_version(self): + """Return version of Nginx Server. + + Version is returned as tuple. (ie. 2.4.7 = (2, 4, 7)) + + :returns: version + :rtype: tuple + + :raises .PluginError: + Unable to find Nginx version or version is unsupported + + """ + text = self._nginx_version() version_regex = re.compile(r"nginx version: ([^/]+)/([0-9\.]*)", re.IGNORECASE) version_matches = version_regex.findall(text) @@ -964,6 +1008,28 @@ class NginxConfigurator(common.Installer): return nginx_version + def _get_openssl_version(self): + """Return version of OpenSSL linked to Nginx. + + Version is returned as string. If no version can be found, empty string is returned. + + :returns: openssl_version + :rtype: str + + :raises .PluginError: + Unable to run Nginx version command + """ + text = self._nginx_version() + + matches = re.findall(r"running with OpenSSL ([^ ]+) ", text) + if not matches: + matches = re.findall(r"built with OpenSSL ([^ ]+) ", text) + if not matches: + logger.warning("NGINX configured with OpenSSL alternatives is not officially" + "supported by Certbot.") + return "" + return matches[0] + def more_info(self): """Human-readable string to help understand the module""" return ( diff --git a/certbot-nginx/certbot_nginx/constants.py b/certbot-nginx/certbot_nginx/constants.py index 2b22729a8..92dc9e79d 100644 --- a/certbot-nginx/certbot_nginx/constants.py +++ b/certbot-nginx/certbot_nginx/constants.py @@ -22,21 +22,6 @@ MOD_SSL_CONF_DEST = "options-ssl-nginx.conf" UPDATED_MOD_SSL_CONF_DIGEST = ".updated-options-ssl-nginx-conf-digest.txt" """Name of the hash of the updated or informed mod_ssl_conf as saved in `IConfig.config_dir`.""" -SSL_OPTIONS_HASHES_NEW = [ - '108c4555058a087496a3893aea5d9e1cee0f20a3085d44a52dc1a66522299ac3', - 'd5e021706ecdccc7090111b0ae9a29ef61523e927f020e410caf0a1fd7063981', -] -"""SHA256 hashes of the contents of versions of MOD_SSL_CONF_SRC for nginx >= 1.13.0""" - -SSL_OPTIONS_HASHES_MEDIUM = [ - '63e2bddebb174a05c9d8a7cf2adf72f7af04349ba59a1a925fe447f73b2f1abf', - '2901debc7ecbc10917edd9084c05464c9c5930b463677571eaf8c94bffd11ae2', - '30baca73ed9a5b0e9a69ea40e30482241d8b1a7343aa79b49dc5d7db0bf53b6c', - '02329eb19930af73c54b3632b3165d84571383b8c8c73361df940cb3894dd426', -] -"""SHA256 hashes of the contents of versions of MOD_SSL_CONF_SRC for nginx >= 1.5.9 - and nginx < 1.13.0""" - ALL_SSL_OPTIONS_HASHES = [ '0f81093a1465e3d4eaa8b0c14e77b2a2e93568b0fc1351c2b87893a95f0de87c', '9a7b32c49001fed4cff8ad24353329472a50e86ade1ef9b2b9e43566a619612e', @@ -46,7 +31,13 @@ ALL_SSL_OPTIONS_HASHES = [ '4b16fec2bcbcd8a2f3296d886f17f9953ffdcc0af54582452ca1e52f5f776f16', 'c052ffff0ad683f43bffe105f7c606b339536163490930e2632a335c8d191cc4', '02329eb19930af73c54b3632b3165d84571383b8c8c73361df940cb3894dd426', -] + SSL_OPTIONS_HASHES_MEDIUM + SSL_OPTIONS_HASHES_NEW + '63e2bddebb174a05c9d8a7cf2adf72f7af04349ba59a1a925fe447f73b2f1abf', + '2901debc7ecbc10917edd9084c05464c9c5930b463677571eaf8c94bffd11ae2', + '30baca73ed9a5b0e9a69ea40e30482241d8b1a7343aa79b49dc5d7db0bf53b6c', + '02329eb19930af73c54b3632b3165d84571383b8c8c73361df940cb3894dd426', + '108c4555058a087496a3893aea5d9e1cee0f20a3085d44a52dc1a66522299ac3', + 'd5e021706ecdccc7090111b0ae9a29ef61523e927f020e410caf0a1fd7063981', +] """SHA256 hashes of the contents of all versions of MOD_SSL_CONF_SRC""" def os_constant(key): diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 8db202785..19624a7a2 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -394,6 +394,68 @@ class NginxConfiguratorTest(util.NginxTest): mock_popen.side_effect = OSError("Can't find program") self.assertRaises(errors.PluginError, self.config.get_version) + @mock.patch("certbot_nginx.configurator.subprocess.Popen") + def test_get_openssl_version(self, mock_popen): + # pylint: disable=protected-access + mock_popen().communicate.return_value = ( + "", """ + nginx version: nginx/1.15.5 + built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) + built with OpenSSL 1.0.2g 1 Mar 2016 + TLS SNI support enabled + configure arguments: + """) + self.assertEqual(self.config._get_openssl_version(), "1.0.2g") + + mock_popen().communicate.return_value = ( + "", """ + nginx version: nginx/1.15.5 + built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) + built with OpenSSL 1.0.2-beta1 1 Mar 2016 + TLS SNI support enabled + configure arguments: + """) + self.assertEqual(self.config._get_openssl_version(), "1.0.2-beta1") + + mock_popen().communicate.return_value = ( + "", """ + nginx version: nginx/1.15.5 + built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) + built with OpenSSL 1.0.2 1 Mar 2016 + TLS SNI support enabled + configure arguments: + """) + self.assertEqual(self.config._get_openssl_version(), "1.0.2") + + mock_popen().communicate.return_value = ( + "", """ + nginx version: nginx/1.15.5 + built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) + built with OpenSSL 1.0.2g 1 Mar 2016 (running with OpenSSL 1.0.2a 1 Mar 2016) + TLS SNI support enabled + configure arguments: + """) + self.assertEqual(self.config._get_openssl_version(), "1.0.2a") + + mock_popen().communicate.return_value = ( + "", """ + nginx version: nginx/1.15.5 + built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) + built with LibreSSL 2.2.2 + TLS SNI support enabled + configure arguments: + """) + self.assertEqual(self.config._get_openssl_version(), "") + + mock_popen().communicate.return_value = ( + "", """ + nginx version: nginx/1.15.5 + built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) + TLS SNI support enabled + configure arguments: + """) + self.assertEqual(self.config._get_openssl_version(), "") + @mock.patch("certbot_nginx.configurator.subprocess.Popen") def test_nginx_restart(self, mock_popen): mocked = mock_popen() @@ -920,13 +982,12 @@ class InstallSslOptionsConfTest(util.NginxTest): self._assert_current_file() def test_prev_file_updates_to_current_old_nginx(self): - from certbot_nginx.constants import ALL_SSL_OPTIONS_HASHES, SSL_OPTIONS_HASHES_NEW + from certbot_nginx.constants import ALL_SSL_OPTIONS_HASHES self.config.version = (1, 5, 8) with mock.patch('certbot.crypto_util.sha256sum', new=self._mock_hash_except_ssl_conf_src(ALL_SSL_OPTIONS_HASHES[0])): self._call() self._assert_current_file() - self.assertTrue(self._current_ssl_options_hash() not in SSL_OPTIONS_HASHES_NEW) def test_manually_modified_current_file_does_not_update(self): with open(self.config.mod_ssl_conf, "a") as mod_ssl_conf: @@ -987,11 +1048,13 @@ class InstallSslOptionsConfTest(util.NginxTest): def test_nginx_version_uses_correct_config(self): self.config.version = (1, 5, 8) + self.config.openssl_version = "1.0.2g" # shouldn't matter self.assertEqual(os.path.basename(self.config.mod_ssl_conf_src), "options-ssl-nginx-old.conf") self._call() self._assert_current_file() self.config.version = (1, 5, 9) + self.config.openssl_version = "1.0.2l" self.assertEqual(os.path.basename(self.config.mod_ssl_conf_src), "options-ssl-nginx-tls12-only.conf") self._call() @@ -999,6 +1062,12 @@ class InstallSslOptionsConfTest(util.NginxTest): self.config.version = (1, 13, 0) self.assertEqual(os.path.basename(self.config.mod_ssl_conf_src), "options-ssl-nginx.conf") + self._call() + self._assert_current_file() + self.config.version = (1, 13, 0) + self.config.openssl_version = "1.0.2k" + self.assertEqual(os.path.basename(self.config.mod_ssl_conf_src), + "options-ssl-nginx-tls13-session-tix-on.conf") class DetermineDefaultServerRootTest(certbot_test_util.ConfigTestCase): diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py index c46ddabc9..c0a70368e 100644 --- a/certbot-nginx/certbot_nginx/tests/util.py +++ b/certbot-nginx/certbot_nginx/tests/util.py @@ -54,7 +54,7 @@ def get_data_filename(filename): def get_nginx_configurator( - config_path, config_dir, work_dir, logs_dir, version=(1, 6, 2)): + config_path, config_dir, work_dir, logs_dir, version=(1, 6, 2), openssl_version="1.0.2g"): """Create an Nginx Configurator with the specified options.""" backups = os.path.join(work_dir, "backups") @@ -79,7 +79,8 @@ def get_nginx_configurator( https_port=5001, ), name="nginx", - version=version) + version=version, + openssl_version=openssl_version) config.prepare() # Provide general config utility. diff --git a/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls12-only.conf b/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls12-only.conf index a678b0507..1933cbc4f 100644 --- a/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls12-only.conf +++ b/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls12-only.conf @@ -6,6 +6,7 @@ ssl_session_cache shared:le_nginx_SSL:10m; ssl_session_timeout 1440m; +ssl_session_tickets off; ssl_protocols TLSv1.2; ssl_prefer_server_ciphers off; diff --git a/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls13-session-tix-on.conf b/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls13-session-tix-on.conf new file mode 100644 index 000000000..52fdfde24 --- /dev/null +++ b/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls13-session-tix-on.conf @@ -0,0 +1,13 @@ +# This file contains important security parameters. If you modify this file +# manually, Certbot will be unable to automatically provide future security +# updates. Instead, Certbot will print and log an error message with a path to +# the up-to-date file that you will need to refer to when manually updating +# this file. + +ssl_session_cache shared:le_nginx_SSL:10m; +ssl_session_timeout 1440m; + +ssl_protocols TLSv1.2 TLSv1.3; +ssl_prefer_server_ciphers off; + +ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384"; diff --git a/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx.conf b/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx.conf index 52fdfde24..978e6e8ab 100644 --- a/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx.conf +++ b/certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx.conf @@ -6,6 +6,7 @@ ssl_session_cache shared:le_nginx_SSL:10m; ssl_session_timeout 1440m; +ssl_session_tickets off; ssl_protocols TLSv1.2 TLSv1.3; ssl_prefer_server_ciphers off;