Copy dbm file to work directory before writing

certbot-apache/certbot_apache/_internal/apache_util.py

@@ -1,9 +1,12 @@
 """ Utility functions for certbot-apache plugin """
 import binascii
+import hashlib
 import struct
+import shutil
 import time
 
 from certbot import crypto_util
+from certbot import errors
 from certbot import util
 from certbot.compat import os
 
@@ -49,6 +52,53 @@ def certid_sha1(cert_path):
     return crypto_util.cert_sha1_fingerprint(cert_path)
 
 
+def safe_copy(source, target):
+    """Copies a file, while verifying the target integrity
+    with the source. Retries twice if the initial
+    copy fails.
+
+    :param str source: File path of the source file
+    :param str target: File path of the target file
+
+    :raises: .errors.PluginError: If file cannot be
+        copied or the target file hash does not match
+        with the source file.
+    """
+    for _ in range(3):
+        try:
+            shutil.copy2(source, target)
+        except IOError as e:
+            emsg = "Could not copy {} to {}: {}".format(
+                source, target, e
+            )
+            raise errors.PluginError(emsg)
+        try:
+            source_hash = _file_hash(source)
+            target_hash = _file_hash(target)
+        except IOError:
+            continue
+        if source_hash == target_hash:
+            return
+    raise errors.PluginError(
+        "Safe copy failed. The file integrity does not match"
+    )
+
+
+def _file_hash(filepath):
+    """Helper function for safe_copy that calculates a
+    sha-256 hash of file.
+
+    :param str filepath: Path of file to calculate hash for
+
+    :returns: File sha-256 hash
+    :rtype: str
+    """
+    fhash = hashlib.sha256()
+    with open(filepath, 'rb') as fh:
+        fhash.update(fh.read())
+    return fhash.hexdigest()
+
+
 def get_mod_deps(mod_name):
     """Get known module dependencies.
 

certbot-apache/certbot_apache/_internal/configurator.py

@@ -1752,7 +1752,7 @@ class ApacheConfigurator(common.Installer):
         if prefetch:
             if "socache_dbm_module" not in self.parser.modules:
                 self.enable_mod("socache_dbm")
-            cache_path = os.path.join(self.config.config_dir, "ocsp", "ocsp_cache.db")
+            cache_path = os.path.join(self.config.work_dir, "ocsp", "ocsp_cache.db")
             cache_dir = ["dbm:"+cache_path]
         else:
             if "socache_shmcb_module" not in self.parser.modules:

certbot-apache/certbot_apache/_internal/prefetch_ocsp.py

@@ -30,20 +30,18 @@ class DBMHandler(object):
     def __enter__(self):
         """Open the DBM file and return the filehandle"""
 
-        if not os.path.isfile(self.filename + ".db"):
-            raise errors.PluginError(
-                "The OCSP stapling cache DBM file wasn't created by Apache.")
         try:
             import bsddb
             self.bsddb = True
-            cache_path = self.filename + ".db"
             try:
-                self.database = bsddb.hashopen(cache_path, self.filemode)
+                self.database = bsddb.hashopen(self.filename, self.filemode)
             except Exception:
                 raise errors.PluginError("Unable to open dbm database file.")
         except ImportError:
             # Python3 doesn't have bsddb module, so we use dbm.ndbm instead
             import dbm
+            if self.filename.endswith(".db"):
+                self.filename = self.filename[:-3]
             try:
                 self.database = dbm.ndbm.open(self.filename, self.filemode)  # pylint: disable=no-member
             except Exception:
@@ -63,7 +61,6 @@ class OCSPPrefetchMixin(object):
 
     def __init__(self, *args, **kwargs):
         self._ocsp_prefetch = {}  # type: Dict[str, str]
-        self._ocsp_dbm_bsddb = False
         # This is required because of python super() call chain.
         # Additionally, mypy isn't able to figure the chain out and needs to be
         # disabled for this line. See https://github.com/python/mypy/issues/5887
@@ -124,7 +121,7 @@ class OCSPPrefetchMixin(object):
         handler = ocsp.RevocationChecker()
         if not handler.ocsp_revoked_by_paths(cert_path, chain_path, ocsp_workfile):
             # Guaranteed good response
-            cache_path = os.path.join(self.config.work_dir, "ocsp", "ocsp_cache")
+            cache_path = os.path.join(self.config.work_dir, "ocsp", "ocsp_cache.db")
             cert_sha = apache_util.certid_sha1(cert_path)
             # dbm.open automatically adds the file extension
             self._write_to_dbm(cache_path, cert_sha, self._ocsp_response_dbm(ocsp_workfile))
@@ -139,16 +136,26 @@ class OCSPPrefetchMixin(object):
             return
 
     def _write_to_dbm(self, filename, key, value):
-        """Helper method to write an OCSP response cache value to DBM
+        """Helper method to write an OCSP response cache value to DBM.
 
         :param filename: DBM database filename
         :param bytes key: Database key name
         :param bytes value: Database entry value
         """
+        tmp_file = os.path.join(
+            self.config.work_dir,
+            "ocsp_work",
+            "tmp_" + os.path.basename(filename)
+        )
 
-        with DBMHandler(filename, 'w') as db:
+        apache_util.safe_copy(filename, tmp_file)
+
+        with DBMHandler(tmp_file, 'w') as db:
             db[key] = value
 
+        shutil.copy2(tmp_file, filename)
+        os.remove(tmp_file)
+
     def _read_dbm(self, filename):
         """Helper method for reading the dbm using context manager.
         Used for tests.

certbot-apache/tests/ocsp_prefetch_test.py

@@ -3,8 +3,8 @@ import base64
 from datetime import datetime
 from datetime import timedelta
 import json
-import unittest
 import sys
+import unittest
 
 import mock
 # six is used in mock.patch()
@@ -98,18 +98,15 @@ class OCSPPrefetchTest(util.ApacheTest):
         self.vh_truth = util.get_vh_truth(
             self.temp_dir, "debian_apache_2_4/multiple_vhosts")
         self.config._ensure_ocsp_dirs()
-        self.db_path = os.path.join(self.work_dir, "ocsp", "ocsp_cache")
-        self.db_fullpath = self.db_path + ".db"
+        self.db_path = os.path.join(self.work_dir, "ocsp", "ocsp_cache") + ".db"
 
     def _call_mocked(self, func, *args, **kwargs):
         """Helper method to call functins with mock stack"""
 
-        db_fullpath = self.db_path + ".db"
-
         def mock_restart():
             """Mock ApacheConfigurator.restart that creates the dbm file"""
             # Mock the Apache dbm file creation
-            open(db_fullpath, 'a').close()
+            open(self.db_path, 'a').close()
 
         ver_path = "certbot_apache._internal.configurator.ApacheConfigurator.get_version"
         res_path = "certbot_apache._internal.prefetch_ocsp.OCSPPrefetchMixin.restart"
@@ -234,11 +231,11 @@ class OCSPPrefetchTest(util.ApacheTest):
         def ocsp_del_db():
             """Side effect of _reload() that deletes the DBM file, like Apache
             does when restarting"""
-            os.remove(self.db_fullpath)
-            self.assertFalse(os.path.isfile(self.db_fullpath))
+            os.remove(self.db_path)
+            self.assertFalse(os.path.isfile(self.db_path))
 
         # Make sure that the db file exists
-        open(self.db_fullpath, 'a').close()
+        open(self.db_path, 'a').close()
         self.call_mocked_py2(self.config._write_to_dbm, self.db_path, b'mock_key', b'mock_value')
 
         # Mock OCSP prefetch dict to signify that there should be a db
@@ -300,7 +297,7 @@ class OCSPPrefetchTest(util.ApacheTest):
                               self.config._ensure_ocsp_prefetch_compatibility)
 
     def test_ocsp_prefetch_open_dbm_no_file(self):
-        open(self.db_fullpath, 'a').close()
+        open(self.db_path, 'a').close()
         db_not_exists = self.db_path+"nonsense"
         self.call_mocked_py2(self.config._write_to_dbm, self.db_path, b'k', b'v')
         self.assertRaises(errors.PluginError,
@@ -310,7 +307,7 @@ class OCSPPrefetchTest(util.ApacheTest):
                           b'k', b'v')
 
     def test_ocsp_prefetch_py2_open_file_error(self):
-        open(self.db_fullpath, 'a').close()
+        open(self.db_path, 'a').close()
         mock_db = mock.MagicMock()
         mock_db.hashopen.side_effect = Exception("error")
         sys.modules["bsddb"] = mock_db
@@ -320,7 +317,7 @@ class OCSPPrefetchTest(util.ApacheTest):
                           b'k', b'v')
 
     def test_ocsp_prefetch_py3_open_file_error(self):
-        open(self.db_fullpath, 'a').close()
+        open(self.db_path, 'a').close()
         mock_db = mock.MagicMock()
         mock_db.ndbm.open.side_effect = Exception("error")
         sys.modules["dbm"] = mock_db
@@ -332,7 +329,7 @@ class OCSPPrefetchTest(util.ApacheTest):
 
     def test_ocsp_prefetch_open_close_py2_noerror(self):
         expected_val = b'whatever_value'
-        open(self.db_fullpath, 'a').close()
+        open(self.db_path, 'a').close()
         self.call_mocked_py2(
             self.config._write_to_dbm, self.db_path,
             b'key', expected_val
@@ -342,7 +339,7 @@ class OCSPPrefetchTest(util.ApacheTest):
 
     def test_ocsp_prefetch_open_close_py3_noerror(self):
         expected_val = b'whatever_value'
-        open(self.db_fullpath, 'a').close()
+        open(self.db_path, 'a').close()
         self.call_mocked_py3(
             self.config._write_to_dbm, self.db_path,
             b'key', expected_val
@@ -350,6 +347,31 @@ class OCSPPrefetchTest(util.ApacheTest):
         db2 = self.call_mocked_py3(self.config._read_dbm, self.db_path)
         self.assertEqual(db2[b'key'], expected_val)
 
+    def test_ocsp_prefetch_safe_open_hash_mismatch(self):
+        import random
+        def mock_hash(_filepath):
+            # shound not be the same value twice
+            return str(random.getrandbits(1024))
+
+        open(self.db_path, 'a').close()
+        with mock.patch("certbot_apache._internal.apache_util._file_hash", side_effect=mock_hash):
+            self.assertRaises(
+                errors.PluginError,
+                self.call_mocked_py3,
+                self.config._write_to_dbm, self.db_path,
+                b'anything', b'irrelevant'
+            )
+
+    def test_ocsp_prefetch_safe_open_hash_fail_open(self):
+        open(self.db_path, 'a').close()
+        with mock.patch("certbot_apache._internal.apache_util._file_hash", side_effect=IOError):
+            self.assertRaises(
+                errors.PluginError,
+                self.call_mocked_py3,
+                self.config._write_to_dbm, self.db_path,
+                b'anything', b'irrelevant'
+            )
+
     @mock.patch("certbot_apache._internal.constants.OCSP_APACHE_TTL", 1234)
     def test_ttl(self):
         self.assertEqual(self.config._ocsp_ttl(None), 1234)