Refactor nginx file update mechanism in preparation for working with apache plugin (#4720)

* move install_ssl_options_conf functionality to common

* add no cover

* compute current hash instead of saving

* make current hash be computed; switch to list of all canonical hashes

* put message directly into assertion

* don't pass logger

* add docstring

* Add unit tests for certbot.plugins.common.install_ssl_options_conf

3 files changed, 25 insertions, 55 deletions

diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py
index 752ccc133..63f659453 100644
--- a/certbot-nginx/certbot_nginx/configurator.py
+++ b/certbot-nginx/certbot_nginx/configurator.py
@@ -2,7 +2,6 @@
 import logging
 import os
 import re
-import shutil
 import socket
 import subprocess
 import tempfile
@@ -869,36 +868,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."""
-    def _write_current_hash():
-        with open(options_ssl_digest, "w") as f:
-            f.write(constants.CURRENT_SSL_OPTIONS_HASH)
-
-    def _install_current_file():
-        shutil.copyfile(constants.MOD_SSL_CONF_SRC, options_ssl)
-        _write_current_hash()
-
-    # Check to make sure options-ssl.conf is installed
-    if not os.path.isfile(options_ssl):
-        _install_current_file()
-        return
-    # there's already a file there. if it exactly matches a previous 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 in constants.PREVIOUS_SSL_OPTIONS_HASHES: # safe to update
-        _install_current_file()
-    elif active_file_digest == constants.CURRENT_SSL_OPTIONS_HASH: # already up to date
-        return
-    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:
-                saved_digest = f.read()
-            # they modified it after we either installed or told them about this version, so return
-            if saved_digest == constants.CURRENT_SSL_OPTIONS_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 "
-            "saved to %s. We recommend updating %s for security purposes.",
-            options_ssl, constants.MOD_SSL_CONF_SRC, options_ssl)
+    return common.install_ssl_options_conf(options_ssl, options_ssl_digest,
+        constants.MOD_SSL_CONF_SRC, constants.ALL_SSL_OPTIONS_HASHES)
diff --git a/certbot-nginx/certbot_nginx/constants.py b/certbot-nginx/certbot_nginx/constants.py
index 765bdd7a8..a74f97662 100644
--- a/certbot-nginx/certbot_nginx/constants.py
+++ b/certbot-nginx/certbot_nginx/constants.py
@@ -21,16 +21,14 @@ 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`."""
 
 
-PREVIOUS_SSL_OPTIONS_HASHES = [
+ALL_SSL_OPTIONS_HASHES = [
     '0f81093a1465e3d4eaa8b0c14e77b2a2e93568b0fc1351c2b87893a95f0de87c',
     '9a7b32c49001fed4cff8ad24353329472a50e86ade1ef9b2b9e43566a619612e',
     'a6d9f1c7d6b36749b52ba061fff1421f9a0a3d2cfdafbd63c05d06f65b990937',
     '7f95624dd95cf5afc708b9f967ee83a24b8025dc7c8d9df2b556bbc64256b3ff',
+    '394732f2bbe3e5e637c3fb5c6e980a1f1b90b01e2e8d6b7cff41dde16e2a756d',
 ]
-"""SHA256 hashes of the contents of previous versions of MOD_SSL_CONF_SRC"""
-
-CURRENT_SSL_OPTIONS_HASH = '394732f2bbe3e5e637c3fb5c6e980a1f1b90b01e2e8d6b7cff41dde16e2a756d'
-"""SHA256 hash of the current contents of MOD_SSL_CONF_SRC"""
+"""SHA256 hashes of the contents of all versions of MOD_SSL_CONF_SRC"""
 
 def os_constant(key):
     # XXX TODO: In the future, this could return different constants
diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py
index 215fe3165..1f9d3e253 100644
--- a/certbot-nginx/certbot_nginx/tests/configurator_test.py
+++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py
@@ -552,14 +552,14 @@ class InstallSslOptionsConfTest(util.NginxTest):
         from certbot_nginx.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):
+        from certbot_nginx.constants import MOD_SSL_CONF_SRC
+        return crypto_util.sha256sum(MOD_SSL_CONF_SRC)
+
     def _assert_current_file(self):
-        """If this is failing, remember that constants.PREVIOUS_SSL_OPTIONS_HASHES and
-           constants.CURRENT_SSL_OPTIONS_HASH must be updated when self.config.mod_ssl_conf
-           is updated. Add CURRENT_SSL_OPTIONS_HASH to PREVIOUS_SSL_OPTIONS_HASHES and set
-           CURRENT_SSL_OPTIONS_HASH to the hash of the updated self.config.mod_ssl_conf."""
         self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
-        from certbot_nginx.constants import CURRENT_SSL_OPTIONS_HASH
-        self.assertEqual(crypto_util.sha256sum(self.config.mod_ssl_conf), CURRENT_SSL_OPTIONS_HASH)
+        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
@@ -575,43 +575,47 @@ class InstallSslOptionsConfTest(util.NginxTest):
         self._assert_current_file()
 
     def test_prev_file_updates_to_current(self):
-        from certbot_nginx.constants import PREVIOUS_SSL_OPTIONS_HASHES
+        from certbot_nginx.constants import ALL_SSL_OPTIONS_HASHES
         with mock.patch('certbot.crypto_util.sha256sum') as mock_sha256:
-            mock_sha256.return_value = PREVIOUS_SSL_OPTIONS_HASHES[0]
+            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_nginx.configurator.logger") as mock_logger:
+        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))
-        from certbot_nginx.constants import CURRENT_SSL_OPTIONS_HASH
         self.assertEqual(crypto_util.sha256sum(constants.MOD_SSL_CONF_SRC),
-            CURRENT_SSL_OPTIONS_HASH)
+            self._current_ssl_options_hash())
         self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf),
-            CURRENT_SSL_OPTIONS_HASH)
+            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_nginx.configurator.logger") as mock_logger:
+        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.")
-        from certbot_nginx.constants import CURRENT_SSL_OPTIONS_HASH
         self.assertEqual(crypto_util.sha256sum(constants.MOD_SSL_CONF_SRC),
-            CURRENT_SSL_OPTIONS_HASH)
+            self._current_ssl_options_hash())
         # only print warning once
-        with mock.patch("certbot_nginx.configurator.logger") as mock_logger:
+        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_nginx.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