Merge pull request #12 from edx/ned/allow-tempfiles-in-sandbox

Allow programs in the sandbox to write temp files.

README.rst

@@ -64,7 +64,7 @@ Other details here that depend on your configuration:
 
     $ visudo -f /etc/sudoers.d/01-sandbox
 
-    <WWWUSER> ALL=(sandbox) NOPASSWD:<SANDENV>/bin/python
+    <WWWUSER> ALL=(sandbox) SETENV:NOPASSWD:<SANDENV>/bin/python
     <WWWUSER> ALL=(ALL) NOPASSWD:/usr/bin/pkill
 
 5. Edit an AppArmor profile.  This is a text file specifying the limits on the

codejail/jail_code.py

@@ -32,14 +32,7 @@ def configure(command, bin_path, user=None):
     the user name to run the command under.
 
     """
-    cmd_argv = []
-
-    if user:
-        # Run as the specified user
-        cmd_argv.extend(['sudo', '-u', user])
-
-    # Run the command!
-    cmd_argv.append(bin_path)
+    cmd_argv = [bin_path]
 
     # Command-specific arguments
     if command == "python":
@@ -47,7 +40,12 @@ def configure(command, bin_path, user=None):
         # -B means don't try to write .pyc files.
         cmd_argv.extend(['-E', '-B'])
 
-    COMMANDS[command] = cmd_argv
+    COMMANDS[command] = {
+        # The start of the command line for this program.
+        'cmdline_start': cmd_argv,
+        # The user to run this as, perhaps None.
+        'user': user,
+    }
 
 
 def is_configured(command):
@@ -76,6 +74,8 @@ LIMITS = {
     "REALTIME": 1,
     # Total process virutal memory, in bytes, defaulting to unlimited.
     "VMEM": 0,
+    # Size of files creatable, in bytes, defaulting to nothing can be written.
+    "FSIZE": 0,
 }
 
 
@@ -98,6 +98,9 @@ def set_limit(limit_name, value):
         * `"VMEM"`: the total virtual memory available to the jailed code, in
             bytes.  The default is 0 (no memory limit).
 
+        * `"FSIZE"`: the maximum size of files creatable by the jailed code,
+            in bytes.  The default is 0 (no files may be created).
+
     Limits are process-wide, and will affect all future calls to jail_code.
     Providing a limit of 0 will disable that limit.
 
@@ -149,34 +152,61 @@ def jail_code(command, code=None, files=None, argv=None, stdin=None,
     if not is_configured(command):
         raise Exception("jail_code needs to be configured for %r" % command)
 
-    with temp_directory() as tmpdir:
+    # We make a temp directory to serve as the home of the sandboxed code.
+    # It has a writable "tmp" directory within it for temp files.
+
+    with temp_directory() as homedir:
+
+        # Make directory readable by other users ('sandbox' user needs to be
+        # able to read it).
+        os.chmod(homedir, 0775)
+
+        # Make a subdir to use for temp files, world-writable so that the
+        # sandbox user can write to it.
+        tmptmp = os.path.join(homedir, "tmp")
+        os.mkdir(tmptmp)
+        os.chmod(tmptmp, 0777)
 
         if slug:
-            log.debug("Executing jailed code %s in %s", slug, tmpdir)
+            log.debug("Executing jailed code %s in %s", slug, homedir)
 
         argv = argv or []
 
         # All the supporting files are copied into our directory.
         for filename in files or ():
-            dest = os.path.join(tmpdir, os.path.basename(filename))
+            dest = os.path.join(homedir, os.path.basename(filename))
             if os.path.islink(filename):
                 os.symlink(os.readlink(filename), dest)
             elif os.path.isfile(filename):
-                shutil.copy(filename, tmpdir)
+                shutil.copy(filename, homedir)
             else:
                 shutil.copytree(filename, dest, symlinks=True)
 
         # Create the main file.
         if code:
-            with open(os.path.join(tmpdir, "jailed_code"), "w") as jailed:
+            with open(os.path.join(homedir, "jailed_code"), "w") as jailed:
                 jailed.write(code)
 
             argv = ["jailed_code"] + argv
 
-        cmd = COMMANDS[command] + argv
+        cmd = []
+
+        # Build the command to run.
+        user = COMMANDS[command]['user']
+        if user:
+            # Run as the specified user
+            cmd.extend(['sudo', '-u', user])
 
+        # Point TMPDIR at our temp directory.
+        cmd.extend(['TMPDIR=tmp'])
+        # Start with the command line dictated by "python" or whatever.
+        cmd.extend(COMMANDS[command]['cmdline_start'])
+        # Add the code-specific command line pieces.
+        cmd.extend(argv)
+
+        # Run the subprocess.
         subproc = subprocess.Popen(
-            cmd, preexec_fn=set_process_limits, cwd=tmpdir, env={},
+            cmd, preexec_fn=set_process_limits, cwd=homedir, env={},
             stdin=subprocess.PIPE,
             stdout=subprocess.PIPE, stderr=subprocess.PIPE,
         )
@@ -202,9 +232,8 @@ def set_process_limits():       # pragma: no cover
     # in a new process group, so we can kill them all later if we need to.
     os.setsid()
 
-    # No subprocesses or files.
+    # No subprocesses.
     resource.setrlimit(resource.RLIMIT_NPROC, (0, 0))
-    resource.setrlimit(resource.RLIMIT_FSIZE, (0, 0))
 
     # CPU seconds, not wall clock time.
     cpu = LIMITS["CPU"]
@@ -216,6 +245,10 @@ def set_process_limits():       # pragma: no cover
     if vmem:
         resource.setrlimit(resource.RLIMIT_AS, (vmem, vmem))
 
+    # Size of written files.  Can be zero (nothing can be written).
+    fsize = LIMITS["FSIZE"]
+    resource.setrlimit(resource.RLIMIT_FSIZE, (fsize, fsize))
+
 
 class ProcessKillerThread(threading.Thread):
     """

codejail/tests/test_jail_code.py

@@ -84,15 +84,19 @@ class TestFeatures(JailCodeHelpers, unittest.TestCase):
         res = jailpy(
             code="""\
                 import os
+                res = []
                 for path, dirs, files in os.walk("."):
-                    print (path, sorted(dirs), sorted(files))
+                    res.append((path, sorted(dirs), sorted(files)))
+                for row in sorted(res):
+                    print row
                 """,
             files=[file_here("hello.txt"), file_here("pylib")]
         )
         self.assertResultOk(res)
         self.assertEqual(res.stdout, textwrap.dedent("""\
-            ('.', ['pylib'], ['hello.txt', 'jailed_code'])
+            ('.', ['pylib', 'tmp'], ['hello.txt', 'jailed_code'])
             ('./pylib', [], ['module.py', 'module.pyc'])
+            ('./tmp', [], [])
             """))
 
     def test_executing_a_copied_file(self):
@@ -177,6 +181,58 @@ class TestLimits(JailCodeHelpers, unittest.TestCase):
         self.assertEqual(res.stdout, "Trying\n")
         self.assertIn("ermission denied", res.stderr)
 
+    def test_can_write_temp_files(self):
+        set_limit('FSIZE', 1000)
+        res = jailpy(code="""\
+                import os, tempfile
+                print "Trying mkstemp"
+                f, path = tempfile.mkstemp()
+                os.close(f)
+                with open(path, "w") as f1:
+                    f1.write("hello")
+                with open(path) as f2:
+                    print "Got this:", f2.read()
+                """)
+        self.assertResultOk(res)
+        self.assertEqual(res.stdout, "Trying mkstemp\nGot this: hello\n")
+
+    def test_cant_write_large_temp_files(self):
+        set_limit('FSIZE', 1000)
+        res = jailpy(code="""\
+                import os, tempfile
+                print "Trying mkstemp"
+                f, path = tempfile.mkstemp()
+                os.close(f)
+                with open(path, "w") as f1:
+                    f1.write("hello"*250)
+                with open(path) as f2:
+                    print "Got this:", f2.read()
+                """)
+        self.assertNotEqual(res.status, 0)
+        self.assertEqual(res.stdout, "Trying mkstemp\n")
+        self.assertIn("IOError", res.stderr)
+
+    def test_cant_write_many_small_temp_files(self):
+        # We would like this to fail, but there's nothing that checks total
+        # file size written, so the sandbox does not prevent it yet.
+        raise SkipTest("There's nothing checking total file size yet.")
+        set_limit('FSIZE', 1000)
+        res = jailpy(code="""\
+                import os, tempfile
+                print "Trying mkstemp 250"
+                for i in range(250):
+                    f, path = tempfile.mkstemp()
+                    os.close(f)
+                    with open(path, "w") as f1:
+                        f1.write("hello")
+                    with open(path) as f2:
+                        assert f2.read() == "hello"
+                print "Finished 250"
+                """)
+        self.assertNotEqual(res.status, 0)
+        self.assertEqual(res.stdout, "Trying mkstemp 250\n")
+        self.assertIn("IOError", res.stderr)
+
     def test_cant_use_network(self):
         res = jailpy(code="""\
                 import urllib

codejail/util.py

@@ -13,9 +13,6 @@ def temp_directory():
     The directory will be removed when done.
     """
     temp_dir = tempfile.mkdtemp(prefix="codejail-")
-    # Make directory readable by other users ('sandbox' user needs to be
-    # able to read it).
-    os.chmod(temp_dir, 0775)
     try:
         yield temp_dir
     finally: