starred/certbot-certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-07-28 16:54:59 +02:00
Code Issues Packages Projects Releases Wiki Activity

Use ffdhe2048 Nginx DH params to fix Weak-DH bug (#4973)

Browse Source 
* Rename plugins.common.install_ssl_options_conf to plugins.common.install_version_controlled_file

* Install ssl_dhparams file

* Add installation test

* Add ssl_dhparam option when making a server block ssl

* add install_ssl_dhparams to Installer common plugin class

* Remove redundant code and tests

* update MANIFEST.in
This commit is contained in:
ohemorange
2017-09-01 07:57:30 -07:00
committed by Brad Warren
parent c6bdad4ffb
commit 8ad18cbe6e
9 changed files with 118 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MANIFEST.in
+1
View File
@@ -6,3 +6,4 @@ include linter_plugin.py
recursive-include docs *
recursive-include examples *
recursive-include certbot/tests/testdata *
include certbot/ssl-dhparams.pem
certbot-apache/certbot_apache/configurator.py
+1 -1
View File
@@ -1989,5 +1989,5 @@ def install_ssl_options_conf(options_ssl, options_ssl_digest):
# XXX if we ever try to enforce a local privilege boundary (eg, running
# certbot for unprivileged users via setuid), this function will need
# to be modified.
return common.install_ssl_options_conf(options_ssl, options_ssl_digest,
return common.install_version_controlled_file(options_ssl, options_ssl_digest,
constants.os_constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
certbot-apache/certbot_apache/tests/configurator_test.py
+1 -1
View File
@@ -1497,7 +1497,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
with mock.patch("certbot.plugins.common.logger") as mock_logger:
self._call()
self.assertEqual(mock_logger.warning.call_args[0][0],
"%s has been manually modified; updated ssl configuration options "
"%s has been manually modified; updated file "
"saved to %s. We recommend updating %s for security purposes.")
self.assertEqual(crypto_util.sha256sum(constants.os_constant("MOD_SSL_CONF_SRC")),
self._current_ssl_options_hash())
certbot-nginx/certbot_nginx/configurator.py
+10 -6
View File
@@ -157,6 +157,8 @@ class NginxConfigurator(common.Installer):
install_ssl_options_conf(self.mod_ssl_conf, self.updated_mod_ssl_conf_digest)
self.install_ssl_dhparams()
# Set Version
if self.version is None:
self.version = self.get_version()
@@ -449,11 +451,13 @@ class NginxConfigurator(common.Installer):
snakeoil_cert, snakeoil_key = self._get_snakeoil_paths()
ssl_block = (
[['\n ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)],
['\n ', 'ssl_certificate', ' ', snakeoil_cert],
['\n ', 'ssl_certificate_key', ' ', snakeoil_key],
['\n ', 'include', ' ', self.mod_ssl_conf]])
ssl_block = ([
['\n ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)],
['\n ', 'ssl_certificate', ' ', snakeoil_cert],
['\n ', 'ssl_certificate_key', ' ', snakeoil_key],
['\n ', 'include', ' ', self.mod_ssl_conf],
['\n ', 'ssl_dhparam', ' ', self.ssl_dhparams],
])
self.parser.add_server_directives(
vhost, ssl_block, replace=False)
@@ -822,5 +826,5 @@ def nginx_restart(nginx_ctl, nginx_conf):
def install_ssl_options_conf(options_ssl, options_ssl_digest):
"""Copy Certbot's SSL options file into the system's config dir if required."""
return common.install_ssl_options_conf(options_ssl, options_ssl_digest,
return common.install_version_controlled_file(options_ssl, options_ssl_digest,
constants.MOD_SSL_CONF_SRC, constants.ALL_SSL_OPTIONS_HASHES)
certbot-nginx/certbot_nginx/tests/configurator_test.py
+10 -7
View File
@@ -226,8 +226,9 @@ class NginxConfiguratorTest(util.NginxTest):
['listen', '5001', 'ssl'],
['ssl_certificate', 'example/fullchain.pem'],
['ssl_certificate_key', 'example/key.pem'],
['include', self.config.mod_ssl_conf]]
]],
['include', self.config.mod_ssl_conf],
['ssl_dhparam', self.config.ssl_dhparams],
]]],
parsed_example_conf)
self.assertEqual([['server_name', 'somename', 'alias', 'another.alias']],
parsed_server_conf)
@@ -244,8 +245,9 @@ class NginxConfiguratorTest(util.NginxTest):
['listen', '5001', 'ssl'],
['ssl_certificate', '/etc/nginx/fullchain.pem'],
['ssl_certificate_key', '/etc/nginx/key.pem'],
['include', self.config.mod_ssl_conf]]
],
['include', self.config.mod_ssl_conf],
['ssl_dhparam', self.config.ssl_dhparams],
]],
2))
def test_deploy_cert_add_explicit_listen(self):
@@ -268,8 +270,9 @@ class NginxConfiguratorTest(util.NginxTest):
['listen', '5001', 'ssl'],
['ssl_certificate', 'summer/fullchain.pem'],
['ssl_certificate_key', 'summer/key.pem'],
['include', self.config.mod_ssl_conf]]
],
['include', self.config.mod_ssl_conf],
['ssl_dhparam', self.config.ssl_dhparams],
]],
parsed_migration_conf[0])
@mock.patch("certbot_nginx.configurator.tls_sni_01.NginxTlsSni01.perform")
@@ -601,7 +604,7 @@ class InstallSslOptionsConfTest(util.NginxTest):
with mock.patch("certbot.plugins.common.logger") as mock_logger:
self._call()
self.assertEqual(mock_logger.warning.call_args[0][0],
"%s has been manually modified; updated ssl configuration options "
"%s has been manually modified; updated file "
"saved to %s. We recommend updating %s for security purposes.")
self.assertEqual(crypto_util.sha256sum(constants.MOD_SSL_CONF_SRC),
self._current_ssl_options_hash())
certbot/constants.py
+17 -1
View File
@@ -1,6 +1,7 @@
"""Certbot constants."""
import os
import logging
import os
import pkg_resources
from acme import challenges
@@ -112,3 +113,18 @@ FORCE_INTERACTIVE_FLAG = "--force-interactive"
EFF_SUBSCRIBE_URI = "https://supporters.eff.org/subscribe/certbot"
"""EFF URI used to submit the e-mail address of users who opt-in."""
SSL_DHPARAMS_DEST = "ssl-dhparams.pem"
"""Name of the ssl_dhparams file as saved in `IConfig.config_dir`."""
SSL_DHPARAMS_SRC = pkg_resources.resource_filename(
"certbot", "ssl-dhparams.pem")
"""Path to the nginx ssl_dhparams file found in the Certbot distribution."""
UPDATED_SSL_DHPARAMS_DIGEST = ".updated-ssl-dhparams-pem-digest.txt"
"""Name of the hash of the updated or informed ssl_dhparams as saved in `IConfig.config_dir`."""
ALL_SSL_DHPARAMS_HASHES = [
'9ba6429597aeed2d8617a7705b56e96d044f64b07971659382e426675105654b',
]
"""SHA256 hashes of the contents of all versions of SSL_DHPARAMS_SRC"""
certbot/plugins/common.py
+38 -21
View File
@@ -103,7 +103,7 @@ class Plugin(object):
class Installer(Plugin):
"""An installer base class with reverter methods defined.
"""An installer base class with reverter and ssl_dhparam methods defined.
Installer plugins do not have to inherit from this class.
@@ -197,6 +197,24 @@ class Installer(Plugin):
except errors.ReverterError as err:
raise errors.PluginError(str(err))
@property
def ssl_dhparams(self):
"""Full absolute path to ssl_dhparams file."""
return os.path.join(self.config.config_dir, constants.SSL_DHPARAMS_DEST)
@property
def updated_ssl_dhparams_digest(self):
"""Full absolute path to digest of updated ssl_dhparams file."""
return os.path.join(self.config.config_dir, constants.UPDATED_SSL_DHPARAMS_DIGEST)
def install_ssl_dhparams(self):
"""Copy Certbot's ssl_dhparams file into the system's config dir if required."""
return install_version_controlled_file(
self.ssl_dhparams,
self.updated_ssl_dhparams_digest,
constants.SSL_DHPARAMS_SRC,
constants.ALL_SSL_DHPARAMS_HASHES)
class Addr(object):
r"""Represents an virtual host address.
@@ -368,51 +386,50 @@ class TLSSNI01(object):
return response
def install_ssl_options_conf(options_ssl, options_ssl_digest, mod_ssl_conf_src,
all_ssl_options_hashes):
"""Copy Certbot's SSL options file into the system's config dir if required.
def install_version_controlled_file(dest_path, digest_path, src_path, all_hashes):
"""Copy a file into an active location (likely the system's config dir) if required.
:param str options_ssl: destination path for file containing ssl options
:param str options_ssl_digest: path to save a digest of options_ssl in
:param str mod_ssl_conf_src: path to file containing ssl options found in distribution
:param list all_ssl_options_hashes: hashes of every released version of options_ssl
:param str dest_path: destination path for version controlled file
:param str digest_path: path to save a digest of the file in
:param str src_path: path to version controlled file found in distribution
:param list all_hashes: hashes of every released version of the file
"""
current_ssl_options_hash = crypto_util.sha256sum(mod_ssl_conf_src)
current_hash = crypto_util.sha256sum(src_path)
def _write_current_hash():
with open(options_ssl_digest, "w") as f:
f.write(current_ssl_options_hash)
with open(digest_path, "w") as f:
f.write(current_hash)
def _install_current_file():
shutil.copyfile(mod_ssl_conf_src, options_ssl)
shutil.copyfile(src_path, dest_path)
_write_current_hash()
# Check to make sure options-ssl.conf is installed
if not os.path.isfile(options_ssl):
if not os.path.isfile(dest_path):
_install_current_file()
return
# there's already a file there. if it's up to date, do nothing. if it's not but
# it matches a known file hash, we can update it.
# otherwise, print a warning once per new version.
active_file_digest = crypto_util.sha256sum(options_ssl)
if active_file_digest == current_ssl_options_hash: # already up to date
active_file_digest = crypto_util.sha256sum(dest_path)
if active_file_digest == current_hash: # already up to date
return
elif active_file_digest in all_ssl_options_hashes: # safe to update
elif active_file_digest in all_hashes: # safe to update
_install_current_file()
else: # has been manually modified, not safe to update
# did they modify the current version or an old version?
if os.path.isfile(options_ssl_digest):
with open(options_ssl_digest, "r") as f:
if os.path.isfile(digest_path):
with open(digest_path, "r") as f:
saved_digest = f.read()
# they modified it after we either installed or told them about this version, so return
if saved_digest == current_ssl_options_hash:
if saved_digest == current_hash:
return
# there's a new version but we couldn't update the file, or they deleted the digest.
# save the current digest so we only print this once, and print a warning
_write_current_hash()
logger.warning("%s has been manually modified; updated ssl configuration options "
logger.warning("%s has been manually modified; updated file "
"saved to %s. We recommend updating %s for security purposes.",
options_ssl, mod_ssl_conf_src, options_ssl)
dest_path, src_path, dest_path)
# test utils used by certbot_apache/certbot_nginx (hence
certbot/plugins/common_test.py
+32 -16
View File
@@ -79,14 +79,16 @@ class PluginTest(unittest.TestCase):
"--mock-foo-bar", dest="different_to_foo_bar", x=1, y=None)
class InstallerTest(unittest.TestCase):
class InstallerTest(test_util.ConfigTestCase):
"""Tests for certbot.plugins.common.Installer."""
def setUp(self):
super(InstallerTest, self).setUp()
os.mkdir(self.config.config_dir)
from certbot.plugins.common import Installer
with mock.patch("certbot.plugins.common.reverter.Reverter"):
self.installer = Installer(config=mock.MagicMock(),
self.installer = Installer(config=self.config,
name="Installer")
self.reverter = self.installer.reverter
@@ -163,6 +165,20 @@ class InstallerTest(unittest.TestCase):
self.assertRaises(
errors.PluginError, installer_func, *passed_args, **passed_kwargs)
def test_install_ssl_dhparams(self):
self.installer.install_ssl_dhparams()
self.assertTrue(os.path.isfile(self.installer.ssl_dhparams))
def _current_ssl_dhparams_hash(self):
from certbot.constants import SSL_DHPARAMS_SRC
return crypto_util.sha256sum(SSL_DHPARAMS_SRC)
def test_current_file_hash_in_all_hashes(self):
from certbot.constants import ALL_SSL_DHPARAMS_HASHES
self.assertTrue(self._current_ssl_dhparams_hash() in ALL_SSL_DHPARAMS_HASHES,
"Constants.ALL_SSL_DHPARAMS_HASHES must be appended"
" with the sha256 hash of self.config.ssl_dhparams when it is updated.")
class AddrTest(unittest.TestCase):
"""Tests for certbot.client.plugins.common.Addr."""
@@ -314,11 +330,11 @@ class TLSSNI01Test(unittest.TestCase):
achall.response(achall.account_key).z_domain.decode("utf-8"))
class InstallSslOptionsConfTest(test_util.TempDirTestCase):
"""Tests for certbot.plugins.common.install_ssl_options_conf."""
class InstallVersionControlledFileTest(test_util.TempDirTestCase):
"""Tests for certbot.plugins.common.install_version_controlled_file."""
def setUp(self):
super(InstallSslOptionsConfTest, self).setUp()
super(InstallVersionControlledFileTest, self).setUp()
self.hashes = ["someotherhash"]
self.dest_path = os.path.join(self.tempdir, "options-ssl-dest.conf")
self.hash_path = os.path.join(self.tempdir, ".options-ssl-conf.txt")
@@ -330,19 +346,19 @@ class InstallSslOptionsConfTest(test_util.TempDirTestCase):
self.hashes.append(crypto_util.sha256sum(path))
def _call(self):
from certbot.plugins.common import install_ssl_options_conf
install_ssl_options_conf(self.dest_path,
self.hash_path,
self.source_path,
self.hashes)
from certbot.plugins.common import install_version_controlled_file
install_version_controlled_file(self.dest_path,
self.hash_path,
self.source_path,
self.hashes)
def _current_ssl_options_hash(self):
def _current_file_hash(self):
return crypto_util.sha256sum(self.source_path)
def _assert_current_file(self):
self.assertTrue(os.path.isfile(self.dest_path))
self.assertEqual(crypto_util.sha256sum(self.dest_path),
self._current_ssl_options_hash())
self._current_file_hash())
def test_no_file(self):
self.assertFalse(os.path.isfile(self.dest_path))
@@ -369,9 +385,9 @@ class InstallSslOptionsConfTest(test_util.TempDirTestCase):
self.assertFalse(mock_logger.warning.called)
self.assertTrue(os.path.isfile(self.dest_path))
self.assertEqual(crypto_util.sha256sum(self.source_path),
self._current_ssl_options_hash())
self._current_file_hash())
self.assertNotEqual(crypto_util.sha256sum(self.dest_path),
self._current_ssl_options_hash())
self._current_file_hash())
def test_manually_modified_past_file_warns(self):
with open(self.dest_path, "a") as mod_ssl_conf:
@@ -381,10 +397,10 @@ class InstallSslOptionsConfTest(test_util.TempDirTestCase):
with mock.patch("certbot.plugins.common.logger") as mock_logger:
self._call()
self.assertEqual(mock_logger.warning.call_args[0][0],
"%s has been manually modified; updated ssl configuration options "
"%s has been manually modified; updated file "
"saved to %s. We recommend updating %s for security purposes.")
self.assertEqual(crypto_util.sha256sum(self.source_path),
self._current_ssl_options_hash())
self._current_file_hash())
# only print warning once
with mock.patch("certbot.plugins.common.logger") as mock_logger:
self._call()
certbot/ssl-dhparams.pem
+8
View File
@@ -0,0 +1,8 @@
-----BEGIN DH PARAMETERS-----
MIIBCAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz
+8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a
87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7
YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi
7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD
ssbzSibBsu/6iGtCOGEoXJf//////////wIBAg==
-----END DH PARAMETERS-----