From d0e64328dfe44b5e780fade3348f2a28dd0088ba Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Tue, 11 Feb 2020 18:24:14 -0800 Subject: [PATCH] Add mechanism for selecting apache config file, based on work done in #7191. --- certbot-apache/MANIFEST.in | 1 + .../certbot_apache/_internal/apache_util.py | 13 ++++++++ .../certbot_apache/_internal/configurator.py | 21 +++++++++---- .../certbot_apache/_internal/constants.py | 1 + .../certbot_apache/_internal/override_arch.py | 3 -- .../_internal/override_centos.py | 3 -- .../_internal/override_darwin.py | 3 -- .../_internal/override_debian.py | 3 -- .../_internal/override_fedora.py | 4 --- .../_internal/override_gentoo.py | 3 -- .../certbot_apache/_internal/override_suse.py | 3 -- .../current-options-ssl-apache.conf | 19 ++++++++++++ .../old-options-ssl-apache.conf} | 0 certbot-apache/tests/configurator_test.py | 30 ++++++++++++++----- certbot/CHANGELOG.md | 5 ++-- 15 files changed, 75 insertions(+), 37 deletions(-) create mode 100644 certbot-apache/certbot_apache/_internal/tls_configs/current-options-ssl-apache.conf rename certbot-apache/certbot_apache/_internal/{options-ssl-apache.conf => tls_configs/old-options-ssl-apache.conf} (100%) diff --git a/certbot-apache/MANIFEST.in b/certbot-apache/MANIFEST.in index 2316983bb..8fd9b6814 100644 --- a/certbot-apache/MANIFEST.in +++ b/certbot-apache/MANIFEST.in @@ -3,5 +3,6 @@ include README.rst recursive-include tests * include certbot_apache/_internal/options-ssl-apache.conf recursive-include certbot_apache/_internal/augeas_lens *.aug +recursive-include certbot_apache/_internal/tls_configs *.conf global-exclude __pycache__ global-exclude *.py[cod] diff --git a/certbot-apache/certbot_apache/_internal/apache_util.py b/certbot-apache/certbot_apache/_internal/apache_util.py index 085ccddc8..fed90f2f7 100644 --- a/certbot-apache/certbot_apache/_internal/apache_util.py +++ b/certbot-apache/certbot_apache/_internal/apache_util.py @@ -5,6 +5,8 @@ import logging import re import subprocess +import pkg_resources + from certbot import errors from certbot import util @@ -241,3 +243,14 @@ def _get_runtime_cfg(command): "loaded because Apache is misconfigured.") return stdout + +def find_ssl_apache_conf(prefix): + """ + Find a TLS Apache config file in the dedicated storage. + :param str prefix: prefix of the TLS Apache config file to find + :return: the path the TLS Apache config file + :rtype: str + """ + return pkg_resources.resource_filename( + "certbot_apache", + os.path.join("tls_configs", "{0}-options-ssl-apache.conf".format(prefix))) diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index e9ed1f8ab..d5c07f6e3 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -8,7 +8,6 @@ import re import socket import time -import pkg_resources import six import zope.component import zope.interface @@ -110,14 +109,24 @@ class ApacheConfigurator(common.Installer): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) def option(self, key): """Get a value from options""" return self.options.get(key) + def pick_apache_config(self): + """ + Pick the appropriate TLS Apache configuration file for current version of Apache and OS. + :return: the path to the TLS Apache configuration file to use + :rtype: str + """ + # Disabling TLS session tickets is supported by Apache 2.4.11+. + # So for old versions of Apache we pick a configuration without this option. + if self.version < (2, 4, 11): + return apache_util.find_ssl_apache_conf("old") + return apache_util.find_ssl_apache_conf("current") + def _prepare_options(self): """ Set the values possibly changed by command line parameters to @@ -2460,8 +2469,10 @@ class ApacheConfigurator(common.Installer): # 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_version_controlled_file(options_ssl, options_ssl_digest, - self.option("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES) + apache_config_path = self.pick_apache_config() + + return common.install_version_controlled_file( + options_ssl, options_ssl_digest, apache_config_path, constants.ALL_SSL_OPTIONS_HASHES) def enable_autohsts(self, _unused_lineage, domains): """ diff --git a/certbot-apache/certbot_apache/_internal/constants.py b/certbot-apache/certbot_apache/_internal/constants.py index a37bebac5..68d29406e 100644 --- a/certbot-apache/certbot_apache/_internal/constants.py +++ b/certbot-apache/certbot_apache/_internal/constants.py @@ -26,6 +26,7 @@ ALL_SSL_OPTIONS_HASHES = [ '06675349e457eae856120cdebb564efe546f0b87399f2264baeb41e442c724c7', '5cc003edd93fb9cd03d40c7686495f8f058f485f75b5e764b789245a386e6daf', '007cd497a56a3bb8b6a2c1aeb4997789e7e38992f74e44cc5d13a625a738ac73', + '34783b9e2210f5c4a23bced2dfd7ec289834716673354ed7c7abf69fe30192a3', ] """SHA256 hashes of the contents of previous versions of all versions of MOD_SSL_CONF_SRC""" diff --git a/certbot-apache/certbot_apache/_internal/override_arch.py b/certbot-apache/certbot_apache/_internal/override_arch.py index 2765bd238..2c4f813a1 100644 --- a/certbot-apache/certbot_apache/_internal/override_arch.py +++ b/certbot-apache/certbot_apache/_internal/override_arch.py @@ -1,5 +1,4 @@ """ Distribution specific override class for Arch Linux """ -import pkg_resources import zope.interface from certbot import interfaces @@ -26,6 +25,4 @@ class ArchConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/httpd/conf", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) diff --git a/certbot-apache/certbot_apache/_internal/override_centos.py b/certbot-apache/certbot_apache/_internal/override_centos.py index a3ef2d760..6a031021b 100644 --- a/certbot-apache/certbot_apache/_internal/override_centos.py +++ b/certbot-apache/certbot_apache/_internal/override_centos.py @@ -1,7 +1,6 @@ """ Distribution specific override class for CentOS family (RHEL, Fedora) """ import logging -import pkg_resources import zope.interface from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module @@ -37,8 +36,6 @@ class CentOSConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/httpd/conf.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) def config_test(self): diff --git a/certbot-apache/certbot_apache/_internal/override_darwin.py b/certbot-apache/certbot_apache/_internal/override_darwin.py index 00faff623..244fc07eb 100644 --- a/certbot-apache/certbot_apache/_internal/override_darwin.py +++ b/certbot-apache/certbot_apache/_internal/override_darwin.py @@ -1,5 +1,4 @@ """ Distribution specific override class for macOS """ -import pkg_resources import zope.interface from certbot import interfaces @@ -26,6 +25,4 @@ class DarwinConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2/other", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) diff --git a/certbot-apache/certbot_apache/_internal/override_debian.py b/certbot-apache/certbot_apache/_internal/override_debian.py index 77ced6a3f..c977fb43e 100644 --- a/certbot-apache/certbot_apache/_internal/override_debian.py +++ b/certbot-apache/certbot_apache/_internal/override_debian.py @@ -1,7 +1,6 @@ """ Distribution specific override class for Debian family (Ubuntu/Debian) """ import logging -import pkg_resources import zope.interface from certbot import errors @@ -34,8 +33,6 @@ class DebianConfigurator(configurator.ApacheConfigurator): handle_modules=True, handle_sites=True, challenge_location="/etc/apache2", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) def enable_site(self, vhost): diff --git a/certbot-apache/certbot_apache/_internal/override_fedora.py b/certbot-apache/certbot_apache/_internal/override_fedora.py index 8197b0dcd..b4eeea127 100644 --- a/certbot-apache/certbot_apache/_internal/override_fedora.py +++ b/certbot-apache/certbot_apache/_internal/override_fedora.py @@ -1,5 +1,4 @@ """ Distribution specific override class for Fedora 29+ """ -import pkg_resources import zope.interface from certbot import errors @@ -31,9 +30,6 @@ class FedoraConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/httpd/conf.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - # TODO: eventually newest version of Fedora will need their own config - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) def config_test(self): diff --git a/certbot-apache/certbot_apache/_internal/override_gentoo.py b/certbot-apache/certbot_apache/_internal/override_gentoo.py index c215771e6..3db946292 100644 --- a/certbot-apache/certbot_apache/_internal/override_gentoo.py +++ b/certbot-apache/certbot_apache/_internal/override_gentoo.py @@ -1,5 +1,4 @@ """ Distribution specific override class for Gentoo Linux """ -import pkg_resources import zope.interface from certbot import interfaces @@ -29,8 +28,6 @@ class GentooConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2/vhosts.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) def _prepare_options(self): diff --git a/certbot-apache/certbot_apache/_internal/override_suse.py b/certbot-apache/certbot_apache/_internal/override_suse.py index 0c9219e6d..60a3acabc 100644 --- a/certbot-apache/certbot_apache/_internal/override_suse.py +++ b/certbot-apache/certbot_apache/_internal/override_suse.py @@ -1,5 +1,4 @@ """ Distribution specific override class for OpenSUSE """ -import pkg_resources import zope.interface from certbot import interfaces @@ -26,6 +25,4 @@ class OpenSUSEConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2/vhosts.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf")) ) diff --git a/certbot-apache/certbot_apache/_internal/tls_configs/current-options-ssl-apache.conf b/certbot-apache/certbot_apache/_internal/tls_configs/current-options-ssl-apache.conf new file mode 100644 index 000000000..32a2c3335 --- /dev/null +++ b/certbot-apache/certbot_apache/_internal/tls_configs/current-options-ssl-apache.conf @@ -0,0 +1,19 @@ +# 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. + +SSLEngine on + +# Intermediate configuration, tweak to your needs +SSLProtocol all -SSLv2 -SSLv3 -TLSv1 -TLSv1.1 +SSLCipherSuite 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 +SSLHonorCipherOrder off +SSLSessionTickets off + +SSLOptions +StrictRequire + +# Add vhost name to log entries: +LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined +LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common diff --git a/certbot-apache/certbot_apache/_internal/options-ssl-apache.conf b/certbot-apache/certbot_apache/_internal/tls_configs/old-options-ssl-apache.conf similarity index 100% rename from certbot-apache/certbot_apache/_internal/options-ssl-apache.conf rename to certbot-apache/certbot_apache/_internal/tls_configs/old-options-ssl-apache.conf diff --git a/certbot-apache/tests/configurator_test.py b/certbot-apache/tests/configurator_test.py index cbb052155..17addd79f 100644 --- a/certbot-apache/tests/configurator_test.py +++ b/certbot-apache/tests/configurator_test.py @@ -1700,7 +1700,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): self.config.updated_mod_ssl_conf_digest) def _current_ssl_options_hash(self): - return crypto_util.sha256sum(self.config.option("MOD_SSL_CONF_SRC")) + return crypto_util.sha256sum(self.config.pick_apache_config()) def _assert_current_file(self): self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) @@ -1736,7 +1736,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): self.assertFalse(mock_logger.warning.called) self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) self.assertEqual(crypto_util.sha256sum( - self.config.option("MOD_SSL_CONF_SRC")), + self.config.pick_apache_config()), self._current_ssl_options_hash()) self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf), self._current_ssl_options_hash()) @@ -1752,18 +1752,32 @@ class InstallSslOptionsConfTest(util.ApacheTest): "%s has been manually modified; updated file " "saved to %s. We recommend updating %s for security purposes.") self.assertEqual(crypto_util.sha256sum( - self.config.option("MOD_SSL_CONF_SRC")), + self.config.pick_apache_config()), self._current_ssl_options_hash()) # only print warning once with mock.patch("certbot.plugins.common.logger") as mock_logger: self._call() self.assertFalse(mock_logger.warning.called) - def test_current_file_hash_in_all_hashes(self): - from certbot_apache._internal.constants import ALL_SSL_OPTIONS_HASHES - self.assertTrue(self._current_ssl_options_hash() in ALL_SSL_OPTIONS_HASHES, - "Constants.ALL_SSL_OPTIONS_HASHES must be appended" - " with the sha256 hash of self.config.mod_ssl_conf when it is updated.") + def test_ssl_config_files_hash_in_all_hashes(self): + """ + It is really critical that all TLS Apache config files have their SHA256 hash registered in + constants.ALL_SSL_OPTIONS_HASHES. Otherwise Certbot will mistakenly assume that the config + file has been manually edited by the user, and will refuse to update it. + This test ensures that all necessary hashes are present. + """ + from certbot_apache.constants import ALL_SSL_OPTIONS_HASHES + import pkg_resources + + tls_configs_dir = pkg_resources.resource_filename("certbot_apache", "tls_configs") + all_files = [os.path.join(tls_configs_dir, name) for name in os.listdir(tls_configs_dir) + if name.endswith('options-ssl-apache.conf')] + self.assertTrue(all_files) + for one_file in all_files: + file_hash = crypto_util.sha256sum(one_file) + self.assertTrue(file_hash in ALL_SSL_OPTIONS_HASHES, + "Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 " + "hash of {0} when it is updated.".format(one_file)) if __name__ == "__main__": diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index ff3061e01..748c2a554 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -6,10 +6,11 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added -* Added certbot.ocsp Certbot's API. The certbot.ocsp module can be used to +* Added certbot.ocsp Certbot's API. The certbot.ocsp module can be used to determine the OCSP status of certificates. -* Don't verify the existing certificate in HTTP01Response.simple_verify, for +* Don't verify the existing certificate in HTTP01Response.simple_verify, for compatibility with the real-world ACME challenge checks. +* Turn off session tickets for apache plugin by default when appropriate. ### Changed