Update in response to changes in #4720.

* add file update mechanism + tests to apache

* update with actual hashes, and update apache test to match since there aren't previous versions

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):
+def install_ssl_options_conf(options_ssl, options_ssl_digest):
-    """
+    """Copy Certbot's SSL options file into the system's config dir if required."""
-    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.
-
+    return common.install_ssl_options_conf(options_ssl, options_ssl_digest,
-    # XXX if the user is in security-autoupdate mode, we should be willing to
+        constants.os_constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
-    # 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)

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

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.plugins.common.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.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 "
+                "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.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.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