Merge pull request #13700 from feliksik/shred-vault-tmpfile

[security] shred ansible-vault tmp_file. Also when editor is interruped

lib/ansible/parsing/vault/__init__.py

@@ -22,6 +22,7 @@ import shlex
 import shutil
 import sys
 import tempfile
+import random
 from io import BytesIO
 from subprocess import call
 from ansible.errors import AnsibleError
@@ -219,22 +220,90 @@ class VaultEditor:
 
     def __init__(self, password):
         self.vault = VaultLib(password)
+    
+    def _shred_file_custom(self, tmp_path):
+        """"Destroy a file, when shred (core-utils) is not available
+        
+        Unix `shred' destroys files "so that they can be recovered only with great difficulty with 
+        specialised hardware, if at all". It is based on the method from the paper 
+        "Secure Deletion of Data from Magnetic and Solid-State Memory", 
+        Proceedings of the Sixth USENIX Security Symposium (San Jose, California, July 22-25, 1996).
+        
+        We do not go to that length to re-implement shred in Python; instead, overwriting with a block
+        of random data should suffice. 
+        
+        See https://github.com/ansible/ansible/pull/13700 .
+        """
+        
+        file_len = os.path.getsize(tmp_path)
+        max_chunk_len = min(1024*1024*2, file_len)
+        
+        passes = 3
+        with open(tmp_path,  "wb") as fh:
+            for _ in range(passes):
+                fh.seek(0,  0)
+                # get a random chunk of data, each pass with other length
+                chunk_len = random.randint(max_chunk_len/2, max_chunk_len)
+                data = os.urandom(chunk_len)
+                
+                for _ in range(0, file_len // chunk_len):
+                    fh.write(data)
+                fh.write(data[:file_len % chunk_len])
+                
+                assert(fh.tell() == file_len) # FIXME remove this assert once we have unittests to check its accuracy
+                os.fsync(fh)
+        
+            
+    def _shred_file(self, tmp_path):
+        """Securely destroy a decrypted file
 
+        Note standard limitations of GNU shred apply (For flash, overwriting would have no effect 
+        due to wear leveling; for other storage systems, the async kernel->filesystem->disk calls never
+        guarantee data hits the disk; etc). Furthermore, if your tmp dirs is on tmpfs (ramdisks), 
+        it is a non-issue. 
+        
+        Nevertheless, some form of overwriting the data (instead of just removing the fs index entry) is 
+        a good idea. If shred is not available (e.g. on windows, or no core-utils installed), fall back on 
+        a custom shredding method.
+        """
+        
+        if not os.path.isfile(tmp_path):
+            # file is already gone
+            return 
+        
+        try:
+            r = call(['shred', tmp_path])
+        except OSError as e:
+            # shred is not available on this system, or some other error occured. 
+            r = 1
+        
+        if r != 0:
+            # we could not successfully execute unix shred; therefore, do custom shred. 
+            self._shred_file_custom(tmp_path)
+        
+        os.remove(tmp_path)
+        
     def _edit_file_helper(self, filename, existing_data=None, force_save=False):
 
         # Create a tempfile
         _, tmp_path = tempfile.mkstemp()
 
         if existing_data:
-            self.write_data(existing_data, tmp_path)
+            self.write_data(existing_data, tmp_path, shred=False)
 
         # drop the user into an editor on the tmp file
-        call(self._editor_shell_command(tmp_path))
+        try:
+            call(self._editor_shell_command(tmp_path))
+        except: 
+            # whatever happens, destroy the decrypted file
+            self._shred_file(tmp_path)
+            raise 
+        
         tmpdata = self.read_data(tmp_path)
 
         # Do nothing if the content has not changed
         if existing_data == tmpdata and not force_save:
-            os.remove(tmp_path)
+            self._shred_file(tmp_path)
             return
 
         # encrypt new data and write out to tmp
@@ -258,7 +327,7 @@ class VaultEditor:
 
         ciphertext = self.read_data(filename)
         plaintext = self.vault.decrypt(ciphertext)
-        self.write_data(plaintext, output_file or filename)
+        self.write_data(plaintext, output_file or filename, shred=False)
 
     def create_file(self, filename):
         """ create a new encrypted file """
@@ -323,13 +392,21 @@ class VaultEditor:
 
         return data
 
-    def write_data(self, data, filename):
+    def write_data(self, data, filename, shred=True):
+        """write data to given path
+        
+        if shred==True, make sure that the original data is first shredded so 
+        that is cannot be recovered
+        """
         bytes = to_bytes(data, errors='strict')
         if filename == '-':
             sys.stdout.write(bytes)
         else:
             if os.path.isfile(filename):
-                os.remove(filename)
+                if shred:
+                    self._shred_file(filename)
+                else:
+                    os.remove(filename)
             with open(filename, "wb") as fh:
                 fh.write(bytes)
 
@@ -338,6 +415,7 @@ class VaultEditor:
         # overwrite dest with src
         if os.path.isfile(dest):
             prev = os.stat(dest)
+            # old file 'dest' was encrypted, no need to _shred_file
             os.remove(dest)
         shutil.move(src, dest)