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

certbot/plugins/common.py

@@ -1,4 +1,5 @@
 """Plugin common functions."""
+import logging
 import os
 import re
 import shutil
@@ -11,9 +12,12 @@ import zope.interface
 from acme.jose import util as jose_util
 
 from certbot import constants
+from certbot import crypto_util
 from certbot import interfaces
 from certbot import util
 
+logger = logging.getLogger(__name__)
+
 
 def option_namespace(name):
     """ArgumentParser options namespace (prefix of all options)."""
@@ -262,17 +266,57 @@ 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.
+
+       :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
+    """
+    current_ssl_options_hash = crypto_util.sha256sum(mod_ssl_conf_src)
+
+    def _write_current_hash():
+        with open(options_ssl_digest, "w") as f:
+            f.write(current_ssl_options_hash)
+
+    def _install_current_file():
+        shutil.copyfile(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'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
+        return
+    elif active_file_digest in all_ssl_options_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:
+                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:
+                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, mod_ssl_conf_src, options_ssl)
+
+
 # test utils used by certbot_apache/certbot_nginx (hence
 # "pragma: no cover") TODO: this might quickly lead to dead code (also
 # c.f. #383)
 
-def setup_ssl_options(config_dir, src, dest):  # pragma: no cover
-    """Move the ssl_options into position and return the path."""
-    option_path = os.path.join(config_dir, dest)
-    shutil.copyfile(src, option_path)
-    return option_path
-
-
 def dir_setup(test_dir, pkg):  # pragma: no cover
     """Setup the directories necessary for the configurator."""
     def expanded_tempdir(prefix):

certbot/plugins/common_test.py

@@ -11,6 +11,7 @@ from acme import challenges
 from acme import jose
 
 from certbot import achallenges
+from certbot import crypto_util
 
 from certbot.tests import acme_util
 from certbot.tests import util as test_util
@@ -221,5 +222,79 @@ class TLSSNI01Test(unittest.TestCase):
             OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key))
 
 
+class InstallSslOptionsConfTest(test_util.TempDirTestCase):
+    """Tests for certbot.plugins.common.install_ssl_options_conf."""
+
+    def setUp(self):
+        super(InstallSslOptionsConfTest, self).setUp()
+        self.source_path = os.path.join(self.tempdir, "options-ssl-source.conf")
+        self.dest_path = os.path.join(self.tempdir, "options-ssl-dest.conf")
+        self.hash_path = os.path.join(self.tempdir, ".options-ssl-conf.txt")
+        with open(self.source_path, "w") as f:
+            f.write("test directive\n")
+        self.hashes = ["someotherhash",
+                       "anotheroldhash",
+                       crypto_util.sha256sum(self.source_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)
+
+    def _current_ssl_options_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())
+
+    def test_no_file(self):
+        self.assertFalse(os.path.isfile(self.dest_path))
+        self._call()
+        self._assert_current_file()
+
+    def test_current_file(self):
+        self._call()
+        self._assert_current_file()
+
+    def test_prev_file_updates_to_current(self):
+        with mock.patch('certbot.crypto_util.sha256sum') as mock_sha256:
+            mock_sha256.return_value = self.hashes[0]
+            self._call()
+        self._assert_current_file()
+
+    def test_manually_modified_current_file_does_not_update(self):
+        self._call()
+        with open(self.dest_path, "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.dest_path))
+        self.assertEqual(crypto_util.sha256sum(self.source_path),
+            self._current_ssl_options_hash())
+        self.assertNotEqual(crypto_util.sha256sum(self.dest_path),
+            self._current_ssl_options_hash())
+
+    def test_manually_modified_past_file_warns(self):
+        with open(self.dest_path, "a") as mod_ssl_conf:
+            mod_ssl_conf.write("a new line for the wrong hash\n")
+        with open(self.hash_path, "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(self.source_path),
+            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)
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover