Disable TLS session tickets in Nginx (#7355)

* Find OpenSSL version

* Create and update various config files

* Update logic to use new version constraints

* SSL_OPTIONS_HASHES_NEW and SSL_OPTIONS_HASHES_MEDIUM were just being used for testing, and maintaining them is becoming untenable, so remove them.

* if we don't know the openssl version, we can't turn off session tickets

* add unit test for _get_openssl_version

* add unit tests

* placate lint

* Fix docs and tests and clean up code

* use python correctly

* update changelog

* Lint

* make comment a comment
This commit is contained in:
ohemorange 2019-09-05 16:51:56 -04:00 committed by sydneyli
parent ab0e382829
commit e4af1f3319
8 changed files with 176 additions and 34 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -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;

View file

@ -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";

View file

@ -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;