Return the exit code, stdout and stderr from run_sandbox()

This required a rewrite of the 'chroot' module.

4 files changed, 79 insertions, 26 deletions

diff --git a/run-sandbox b/run-sandbox
index 356d9f6..fef3e94 100755
--- a/run-sandbox
+++ b/run-sandbox
@@ -93,10 +93,13 @@ def run():
 
             sharing_config = executor.maximum_possible_isolation()
 
-            executor.run_sandbox(
+            exit, out, err = executor.run_sandbox(
                 rootfs_path, command, cwd=cwd, extra_env=extra_env,
                 **sharing_config)
 
+            # We'll take a punt on the output being valid UTF-8.
+            print(out.decode('utf-8'))
+            print(err.decode('utf-8'))
     else:
         # We should at minimum handle filesystem trees as well.
         raise RuntimeError(
diff --git a/sandboxlib/__init__.py b/sandboxlib/__init__.py
index c17a12b..db8f34a 100644
--- a/sandboxlib/__init__.py
+++ b/sandboxlib/__init__.py
@@ -22,6 +22,9 @@ docstrings that describe the different parameters.
 '''
 
 
+import subprocess
+
+
 def maximum_possible_isolation():
     '''Describe the 'tightest' isolation possible with a specific backend.
 
@@ -92,6 +95,28 @@ def environment_vars(extra_env=None):
     return env
 
 
+def _run_command(argv, cwd=None, env=None, preexec_fn=None):
+    '''Wrapper around subprocess.Popen() with common settings.
+
+    This function blocks until the subprocesses has terminated. It then
+    returns a tuple of (exit code, stdout output, stderr output).
+
+    '''
+    process = subprocess.Popen(
+        argv,
+        # The default is to share file descriptors from the parent process
+        # to the subprocess, which is rarely good for sandboxing.
+        close_fds=True,
+        cwd=cwd,
+        env=env,
+        preexec_fn=preexec_fn,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE
+    )
+    process.wait()
+    return process.returncode, process.stdout.read(), process.stderr.read()
+
+
 # Executors
 import sandboxlib.chroot
 import sandboxlib.linux_user_chroot
diff --git a/sandboxlib/chroot.py b/sandboxlib/chroot.py
index 8862b13..986c7c2 100644
--- a/sandboxlib/chroot.py
+++ b/sandboxlib/chroot.py
@@ -32,9 +32,8 @@ that the sandbox contains a shell and we do some hack like running
 '''
 
 
+import multiprocessing
 import os
-import subprocess
-import sys
 
 import sandboxlib
 
@@ -56,6 +55,39 @@ def process_network_config(network):
         "Network sharing cannot be be configured in this backend." % network
 
 
+def _run_command_in_chroot(pipe, rootfs_path, command, cwd, env):
+    # This function should be run in a multiprocessing.Process() subprocess,
+    # because it calls os.chroot(). There's no 'unchroot()' function! After
+    # chrooting, it calls sandboxlib._run_command(), which uses the
+    # 'subprocess' module to exec 'command'. This means there are actually
+    # two subprocesses, which is not ideal, but it seems to be the simplest
+    # implementation.
+    #
+    # An alternative approach would be to use the 'preexec_fn' feature of
+    # subprocess.Popen() to call os.chroot(rootfs_path) and os.chdir(cwd).
+    # The Python 3 '_posixsubprocess' module hints in several places that
+    # deadlocks can occur when using preexec_fn, and it is very difficult to
+    # propagate exceptions from that function, so it seems best to avoid it.
+
+    try:
+        # You have most likely got to be the 'root' user in order for this to
+        # work.
+        try:
+            os.chroot(rootfs_path)
+        except OSError as e:
+            raise RuntimeError("Unable to chroot: %s" % e)
+
+        if cwd is not None:
+            os.chdir(cwd)
+
+        exit, out, err = sandboxlib._run_command(command, env=env)
+        pipe.send([exit, out, err])
+        os._exit(0)
+    except Exception as e:
+        pipe.send(e)
+        os._exit(1)
+
+
 def run_sandbox(rootfs_path, command, cwd=None, extra_env=None,
                 network='undefined'):
     if type(command) == str:
@@ -65,25 +97,19 @@ def run_sandbox(rootfs_path, command, cwd=None, extra_env=None,
 
     process_network_config(network)
 
-    pid = os.fork()
-    if pid == 0:
-        # Child process. It's a bit messy that we create a child process and
-        # then a second child process, but it saves duplicating stuff from the
-        # 'subprocess' module.
+    pipe_parent, pipe_child = multiprocessing.Pipe()
 
-        # FIXME: you gotta be root for this one.
-        try:
-            try:
-                os.chroot(rootfs_path)
-            except OSError as e:
-                raise RuntimeError("Unable to chroot: %s" % e)
-
-            result = subprocess.call(command, cwd=cwd, env=env)
-        except Exception as e:
-            print("ERROR: %s" % e)
-            result = 255
-        finally:
-            os._exit(result)
+    process = multiprocessing.Process(
+        target=_run_command_in_chroot,
+        args=(pipe_child, rootfs_path, command, cwd, env))
+    process.start()
+    process.join()
+
+    if process.exitcode == 0:
+        exit, out, err = pipe_parent.recv()
+        return exit, out, err
     else:
-        # Parent process. Wait for child to exit.
-        os.waitpid(pid, 0)
+        # Note that no effort is made to pass on the original traceback, which
+        # will be within the _run_command_in_chroot() function somewhere.
+        exception = pipe_parent.recv()
+        raise exception
diff --git a/sandboxlib/linux_user_chroot.py b/sandboxlib/linux_user_chroot.py
index 0a65f5d..0dfbe4d 100644
--- a/sandboxlib/linux_user_chroot.py
+++ b/sandboxlib/linux_user_chroot.py
@@ -26,8 +26,6 @@ Supported network settings: 'undefined', 'isolated'.
 '''
 
 
-import subprocess
-
 import sandboxlib
 
 
@@ -79,4 +77,5 @@ def run_sandbox(rootfs_path, command, cwd=None, extra_env=None,
 
     argv = (
         [linux_user_chroot] + linux_user_chroot_args + [rootfs_path] + command)
-    subprocess.call(argv, env=env)
+    exit, out, err = sandboxlib._run_command(argv, env=env)
+    return exit, out, err