Write secrets to tmpfs

So that we may avoid writing the decrypted contents of secrets to
disk, write them to a file in a tmpfs.

Change-Id: I7c029b67d0fc2fa3827dc811137dd4f3a90706d8

tests/unit/test_bubblewrap.py

@@ -39,8 +39,8 @@ class TestBubblewrap(testtools.TestCase):
         ssh_agent.start()
         po = context.getPopen(work_dir=work_dir,
                               ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
-        self.assertTrue(po.passwd_r > 2)
+        self.assertTrue(po.fds[0] > 2)
-        self.assertTrue(po.group_r > 2)
+        self.assertTrue(po.fds[1] > 2)
         self.assertTrue(work_dir in po.command)
         # Now run /usr/bin/id to verify passwd/group entries made it in
         true_proc = po(['/usr/bin/id'], stdout=subprocess.PIPE,
@@ -51,8 +51,7 @@ class TestBubblewrap(testtools.TestCase):
         # And that it did not print things on stderr
         self.assertEqual(0, len(errs.strip()))
         # Make sure the _r's are closed
-        self.assertIsNone(po.passwd_r)
+        self.assertEqual([], po.fds)
-        self.assertIsNone(po.group_r)
 
     def test_bubblewrap_leak(self):
         bwrap = bubblewrap.BubblewrapDriver()

tests/unit/test_v3.py

@@ -1283,8 +1283,7 @@ class TestSecretLeaks(AnsibleZuulTestCase):
         ], ordered=False)
         matches = self.searchForContent(self.history[0].jobdir.root,
                                         b'test-password')
-        self.assertEqual(set(['/ansible/playbook_0/secrets.yaml',
+        self.assertEqual(set(['/work/secret-file.txt']),
-                              '/work/secret-file.txt']),
                          set(matches))
 
     def test_secret_file(self):
@@ -1319,8 +1318,7 @@ class TestSecretLeaks(AnsibleZuulTestCase):
         ], ordered=False)
         matches = self.searchForContent(self.history[0].jobdir.root,
                                         b'test-password')
-        self.assertEqual(set(['/ansible/playbook_0/secrets.yaml',
+        self.assertEqual(set(['/work/failure-file.txt']),
-                              '/work/failure-file.txt']),
                          set(matches))
 
     def test_secret_file_fail(self):

zuul/driver/__init__.py

@@ -258,7 +258,7 @@ class WrapperInterface(object, metaclass=abc.ABCMeta):
     """
 
     @abc.abstractmethod
-    def getExecutionContext(self, ro_paths=None, rw_paths=None):
+    def getExecutionContext(self, ro_paths=None, rw_paths=None, secrets=None):
         """Create and return an execution context.
 
         The execution context is meant to be used for a single
@@ -268,6 +268,9 @@ class WrapperInterface(object, metaclass=abc.ABCMeta):
 
         :arg list ro_paths: read only files or directories to bind mount
         :arg list rw_paths: read write files or directories to bind mount
+        :arg dict secrets: a dictionary where the key is a file path,
+             and the value is the content which should be written to
+             that path in a secure manner.
 
         :returns: a new ExecutionContext object.
         :rtype: BaseExecutionContext

zuul/driver/bubblewrap/__init__.py

@@ -22,6 +22,7 @@ import pwd
 import shlex
 import subprocess
 import sys
+import threading
 import re
 
 from typing import Dict, List  # flake8: noqa
@@ -31,10 +32,9 @@ from zuul.execution_context import BaseExecutionContext
 
 
 class WrappedPopen(object):
-    def __init__(self, command, passwd_r, group_r):
+    def __init__(self, command, fds):
         self.command = command
-        self.passwd_r = passwd_r
+        self.fds = fds
-        self.group_r = group_r
 
     def __call__(self, args, *sub_args, **kwargs):
         try:
@@ -46,7 +46,7 @@ class WrappedPopen(object):
                 # versions. So until we are py3 only we can only bwrap
                 # things that are close_fds=False
                 pass_fds = list(kwargs.get('pass_fds', []))
-                for fd in (self.passwd_r, self.group_r):
+                for fd in self.fds:
                     if fd not in pass_fds:
                         pass_fds.append(fd)
                 kwargs['pass_fds'] = pass_fds
@@ -56,26 +56,32 @@ class WrappedPopen(object):
         return proc
 
     def __del__(self):
-        if self.passwd_r:
+        for fd in self.fds:
             try:
-                os.close(self.passwd_r)
+                os.close(fd)
             except OSError:
                 pass
-            self.passwd_r = None
+        self.fds = []
-        if self.group_r:
-            try:
-                os.close(self.group_r)
-            except OSError:
-                pass
-            self.group_r = None
 
 
 class BubblewrapExecutionContext(BaseExecutionContext):
     log = logging.getLogger("zuul.BubblewrapExecutionContext")
 
-    def __init__(self, bwrap_command, ro_paths, rw_paths):
+    def __init__(self, bwrap_command, ro_paths, rw_paths, secrets):
         self.bwrap_command = bwrap_command
         self.mounts_map = {'ro': ro_paths, 'rw': rw_paths}
+        self.secrets = secrets
+
+    def startPipeWriter(self, pipe, data):
+        # In case we have a large amount of data to write through a
+        # pipe, spawn a thread to handle the writes.
+        t = threading.Thread(target=self._writer, args=(pipe, data))
+        t.daemon = True
+        t.start()
+
+    def _writer(self, pipe, data):
+        os.write(pipe, data)
+        os.close(pipe)
 
     def getPopen(self, **kwargs):
         # Set zuul_dir if it was not passed in
@@ -95,6 +101,9 @@ class BubblewrapExecutionContext(BaseExecutionContext):
             for bind in self.mounts_map[mount_type]:
                 bwrap_command.extend([bind_arg, bind, bind])
 
+        # A list of file descriptors which must be held open so that
+        # bwrap may read from them.
+        read_fds = []
         # Need users and groups
         uid = os.getuid()
         passwd = list(pwd.getpwuid(uid))
@@ -106,6 +115,7 @@ class BubblewrapExecutionContext(BaseExecutionContext):
         os.write(passwd_w, passwd_bytes)
         os.write(passwd_w, b'\n')
         os.close(passwd_w)
+        read_fds.append(passwd_r)
 
         gid = os.getgid()
         group = grp.getgrgid(gid)
@@ -115,6 +125,20 @@ class BubblewrapExecutionContext(BaseExecutionContext):
         os.write(group_w, group_bytes)
         os.write(group_w, b'\n')
         os.close(group_w)
+        read_fds.append(group_r)
+
+        # Create a tmpfs for each directory which holds secrets, and
+        # tell bubblewrap to write the contents to a file therein.
+        secret_dirs = set()
+        for fn, content in self.secrets.items():
+            secret_dir = os.path.dirname(fn)
+            if secret_dir not in secret_dirs:
+                bwrap_command.extend(['--tmpfs', secret_dir])
+                secret_dirs.add(secret_dir)
+            secret_r, secret_w = os.pipe()
+            self.startPipeWriter(secret_w, content.encode('utf8'))
+            bwrap_command.extend(['--file', str(secret_r), fn])
+            read_fds.append(secret_r)
 
         kwargs = dict(kwargs)  # Don't update passed in dict
         kwargs['uid'] = uid
@@ -126,7 +150,7 @@ class BubblewrapExecutionContext(BaseExecutionContext):
         self.log.debug("Bubblewrap command: %s",
                        " ".join(shlex.quote(c) for c in command))
 
-        wrapped_popen = WrappedPopen(command, passwd_r, group_r)
+        wrapped_popen = WrappedPopen(command, read_fds)
 
         return wrapped_popen
 
@@ -186,14 +210,17 @@ class BubblewrapDriver(Driver, WrapperInterface):
 
         return bwrap_command
 
-    def getExecutionContext(self, ro_paths=None, rw_paths=None):
+    def getExecutionContext(self, ro_paths=None, rw_paths=None, secrets=None):
         if not ro_paths:
             ro_paths = []
         if not rw_paths:
             rw_paths = []
+        if not secrets:
+            secrets = {}
         return BubblewrapExecutionContext(
             self.bwrap_command,
-            ro_paths, rw_paths)
+            ro_paths, rw_paths,
+            secrets)
 
 
 def main(args=None):
@@ -204,14 +231,22 @@ def main(args=None):
     parser = argparse.ArgumentParser()
     parser.add_argument('--ro-paths', nargs='+')
     parser.add_argument('--rw-paths', nargs='+')
+    parser.add_argument('--secret', nargs='+')
     parser.add_argument('work_dir')
     parser.add_argument('run_args', nargs='+')
     cli_args = parser.parse_args()
 
     ssh_auth_sock = os.environ.get('SSH_AUTH_SOCK')
 
+    secrets = {}
+    if cli_args.secret:
+        for secret in cli_args.secret:
+            fn, content = secret.split('=', 1)
+            secrets[fn]=content
+
     context = driver.getExecutionContext(
-        cli_args.ro_paths, cli_args.rw_paths)
+        cli_args.ro_paths, cli_args.rw_paths,
+        secrets)
 
     popen = context.getPopen(work_dir=cli_args.work_dir,
                              ssh_auth_sock=ssh_auth_sock)

zuul/driver/nullwrap/__init__.py

@@ -32,5 +32,16 @@ class NullwrapDriver(Driver, WrapperInterface):
     name = 'nullwrap'
     log = logging.getLogger("zuul.NullwrapDriver")
 
-    def getExecutionContext(self, ro_paths=None, rw_paths=None):
+    def getExecutionContext(self, ro_paths=None, rw_paths=None, secrets=None):
+        # The bubblewrap driver writes secrets to a tmpfs so that they
+        # don't hit the disk (unless the kernel swaps the memory to
+        # disk, which can be mitigated with encrypted swap).  We
+        # haven't implemented similar functionality in nullwrap, so
+        # for safety, raise an exception in that case.  If you are
+        # interested in implementing this functionality, please
+        # contact us on the mailing list.
+        if secrets:
+            raise NotImplementedError(
+                "The nullwrap driver does not support the use of secrets. "
+                "Consider using the bubblewrap driver instead.")
         return NullExecutionContext()

zuul/executor/server.py

@@ -255,8 +255,9 @@ class JobDirPlaybook(object):
         self.roles_path = []
         self.ansible_config = os.path.join(self.root, 'ansible.cfg')
         self.project_link = os.path.join(self.root, 'project')
-        self.secrets = os.path.join(self.root, 'secrets.yaml')
+        self.secrets_root = os.path.join(self.root, 'secrets')
-        self.has_secrets = False
+        self.secrets = os.path.join(self.secrets_root, 'secrets.yaml')
+        self.secrets_content = None
 
     def addRole(self):
         count = len(self.roles)
@@ -1266,6 +1267,13 @@ class AnsibleJob(object):
         for role in playbook['roles']:
             self.prepareRole(jobdir_playbook, role, args)
 
+        secrets = playbook['secrets']
+        if secrets:
+            if 'zuul' in secrets:
+                raise Exception("Defining secrets named 'zuul' is not allowed")
+            jobdir_playbook.secrets_content = yaml.safe_dump(
+                secrets, default_flow_style=False)
+
         self.writeAnsibleConfig(jobdir_playbook, playbook)
 
     def checkoutTrustedProject(self, project, branch):
@@ -1390,15 +1398,6 @@ class AnsibleJob(object):
     def writeAnsibleConfig(self, jobdir_playbook, playbook):
         trusted = jobdir_playbook.trusted
 
-        secrets = playbook['secrets'].copy()
-        if secrets:
-            if 'zuul' in secrets:
-                raise Exception("Defining secrets named 'zuul' is not allowed")
-            with open(jobdir_playbook.secrets, 'w') as secrets_yaml:
-                secrets_yaml.write(
-                    yaml.safe_dump(secrets, default_flow_style=False))
-            jobdir_playbook.has_secrets = True
-
         # TODO(mordred) This should likely be extracted into a more generalized
         #               mechanism for deployers being able to add callback
         #               plugins.
@@ -1441,7 +1440,7 @@ class AnsibleJob(object):
             # role. Otherwise, printing the args could be useful for
             # debugging.
             config.write('display_args_to_stdout = %s\n' %
-                         str(not secrets))
+                         str(not playbook['secrets']))
 
             config.write('[ssh_connection]\n')
             # NB: when setting pipelining = True, keep_remote_files
@@ -1509,8 +1508,12 @@ class AnsibleJob(object):
         if self.executor_variables_file:
             ro_paths.append(self.executor_variables_file)
 
+        secrets = {}
+        if playbook.secrets_content:
+            secrets[playbook.secrets] = playbook.secrets_content
+
         context = self.executor_server.execution_wrapper.getExecutionContext(
-            ro_paths, rw_paths)
+            ro_paths, rw_paths, secrets)
 
         popen = context.getPopen(
             work_dir=self.jobdir.work_root,
@@ -1593,7 +1596,7 @@ class AnsibleJob(object):
             verbose = '-v'
 
         cmd = ['ansible-playbook', verbose, playbook.path]
-        if playbook.has_secrets:
+        if playbook.secrets_content:
             cmd.extend(['-e', '@' + playbook.secrets])
 
         if success is not None: