diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 13b325d7f..24a2f3934 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -5,7 +5,6 @@ import fnmatch import logging import os import re -import shutil import socket import time @@ -145,6 +144,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return os.path.join(self.config.config_dir, constants.MOD_SSL_CONF_DEST) + @property + def updated_mod_ssl_conf_digest(self): + """Full absolute path to digest of updated SSL configuration file.""" + return os.path.join(self.config.config_dir, constants.UPDATED_MOD_SSL_CONF_DIGEST) + def prepare(self): """Prepare the authenticator/installer. @@ -195,7 +199,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Get all of the available vhosts self.vhosts = self.get_virtual_hosts() - install_ssl_options_conf(self.mod_ssl_conf) + install_ssl_options_conf(self.mod_ssl_conf, self.updated_mod_ssl_conf_digest) # Prevent two Apache plugins from modifying a config at once try: @@ -1981,19 +1985,11 @@ def _split_aug_path(vhost_path): return file_path, "/".join(reversed(internal_path)) -def install_ssl_options_conf(options_ssl): - """ - Copy Certbot's SSL options file into the system's config dir if - required. - """ +def install_ssl_options_conf(options_ssl, options_ssl_digest): + """Copy Certbot's SSL options file into the system's config dir if required.""" + # 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. - - # XXX if the user is in security-autoupdate mode, we should be willing to - # overwrite the options_ssl file at least if it's unmodified: - # https://github.com/letsencrypt/letsencrypt/issues/1123 - - # Check to make sure options-ssl.conf is installed - if not os.path.isfile(options_ssl): - shutil.copyfile(constants.os_constant("MOD_SSL_CONF_SRC"), options_ssl) + return common.install_ssl_options_conf(options_ssl, options_ssl_digest, + constants.os_constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES, logger) diff --git a/certbot-apache/certbot_apache/constants.py b/certbot-apache/certbot_apache/constants.py index 3cfeb4dd6..0b96d392e 100644 --- a/certbot-apache/certbot_apache/constants.py +++ b/certbot-apache/certbot_apache/constants.py @@ -131,6 +131,16 @@ CLI_DEFAULTS = { MOD_SSL_CONF_DEST = "options-ssl-apache.conf" """Name of the mod_ssl config file as saved in `IConfig.config_dir`.""" + +UPDATED_MOD_SSL_CONF_DIGEST = ".updated-options-ssl-apache-conf-digest.txt" +"""Name of the hash of the updated or informed mod_ssl_conf as saved in `IConfig.config_dir`.""" + +ALL_SSL_OPTIONS_HASHES = [ + '2086bca02db48daf93468332543c60ac6acdb6f0b58c7bfdf578a5d47092f82a', + '4844d36c9a0f587172d9fa10f4f1c9518e3bcfa1947379f155e16a70a728c21a', +] +"""SHA256 hashes of the contents of previous versions of all versions of MOD_SSL_CONF_SRC""" + AUGEAS_LENS_DIR = pkg_resources.resource_filename( "certbot_apache", "augeas_lens") """Path to the Augeas lens directory""" diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 75589dce5..055feeece 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -12,6 +12,7 @@ import six # pylint: disable=unused-import from acme import challenges from certbot import achallenges +from certbot import crypto_util from certbot import errors from certbot.tests import acme_util @@ -826,8 +827,10 @@ class MultipleVhostsTest(util.ApacheTest): def test_install_ssl_options_conf(self): from certbot_apache.configurator import install_ssl_options_conf path = os.path.join(self.work_dir, "test_it") - install_ssl_options_conf(path) + other_path = os.path.join(self.work_dir, "other_test_it") + install_ssl_options_conf(path, other_path) self.assertTrue(os.path.isfile(path)) + self.assertTrue(os.path.isfile(other_path)) # TEST ENHANCEMENTS def test_supported_enhancements(self): @@ -1432,5 +1435,83 @@ class MultiVhostsTest(util.ApacheTest): mock.ANY) +class InstallSslOptionsConfTest(util.ApacheTest): + """Test that the options-ssl-nginx.conf file is installed and updated properly.""" + + def setUp(self): # pylint: disable=arguments-differ + super(InstallSslOptionsConfTest, self).setUp() + + self.config = util.get_apache_configurator( + self.config_path, self.vhost_path, self.config_dir, self.work_dir) + + def _call(self): + from certbot_apache.configurator import install_ssl_options_conf + install_ssl_options_conf(self.config.mod_ssl_conf, self.config.updated_mod_ssl_conf_digest) + + def _current_ssl_options_hash(self): + return crypto_util.sha256sum(constants.os_constant("MOD_SSL_CONF_SRC")) + + def _assert_current_file(self): + self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) + self.assertEqual(crypto_util.sha256sum(self.config.mod_ssl_conf), + self._current_ssl_options_hash()) + + def test_no_file(self): + # prepare should have placed a file there + self._assert_current_file() + os.remove(self.config.mod_ssl_conf) + self.assertFalse(os.path.isfile(self.config.mod_ssl_conf)) + self._call() + self._assert_current_file() + + def test_current_file(self): + self._assert_current_file() + self._call() + self._assert_current_file() + + def test_prev_file_updates_to_current(self): + from certbot_apache.constants import ALL_SSL_OPTIONS_HASHES + ALL_SSL_OPTIONS_HASHES.insert(0, "test_hash_does_not_match") + with mock.patch('certbot.crypto_util.sha256sum') as mock_sha256: + mock_sha256.return_value = ALL_SSL_OPTIONS_HASHES[0] + self._call() + self._assert_current_file() + + def test_manually_modified_current_file_does_not_update(self): + with open(self.config.mod_ssl_conf, "a") as mod_ssl_conf: + mod_ssl_conf.write("a new line for the wrong hash\n") + with mock.patch("certbot_apache.configurator.logger") as mock_logger: + self._call() + self.assertFalse(mock_logger.warning.called) + self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) + self.assertEqual(crypto_util.sha256sum(constants.os_constant("MOD_SSL_CONF_SRC")), + self._current_ssl_options_hash()) + self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf), + self._current_ssl_options_hash()) + + def test_manually_modified_past_file_warns(self): + with open(self.config.mod_ssl_conf, "a") as mod_ssl_conf: + mod_ssl_conf.write("a new line for the wrong hash\n") + with open(self.config.updated_mod_ssl_conf_digest, "w") as f: + f.write("hashofanoldversion") + with mock.patch("certbot_apache.configurator.logger") as mock_logger: + self._call() + self.assertEqual(mock_logger.warning.call_args[0][0], + "%s has been manually modified; updated ssl configuration options " + "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()) + # only print warning once + with mock.patch("certbot_apache.configurator.logger") as mock_logger: + self._call() + self.assertFalse(mock_logger.warning.called) + + def test_current_file_hash_in_all_hashes(self): + from certbot_apache.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.") + + if __name__ == "__main__": unittest.main() # pragma: no cover