Add default security policy (#24)

repomate_junit4/_junit4_runner.py

@@ -1,16 +1,67 @@
 import pathlib
+import tempfile
 import re
 import sys
 import subprocess
 import os
+import contextlib
+from typing import Tuple, Optional
 
-from typing import Tuple
+import daiquiri
 
 from repomate_plug import Status
 
 from repomate_junit4 import _java
 
 
+LOGGER = daiquiri.getLogger(__file__)
+HAMCREST_JAR = "hamcrest-core-1.3.jar"
+JUNIT_JAR = "junit-4.12.jar"
+
+_DEFAULT_SECURITY_POLICY_TEMPLATE = """grant {{
+}};
+grant codeBase "file:{junit4_jar_path}" {{
+    permission java.lang.RuntimePermission "accessDeclaredMembers";
+}};
+"""
+
+
+@contextlib.contextmanager
+def security_policy(classpath: str, active: bool):
+    """Yield the path to the default security policy file if ``active``,
+    else yield None. The policy file is deleted once the context is
+    exited.
+
+    TODO: Make it possible to use a custom security policy here.
+    """
+    if not active:
+        LOGGER.warning(
+            "Security policy disabled, student code running without restrictions"
+        )
+        yield
+        return
+
+    with tempfile.NamedTemporaryFile() as security_policy_file:
+        policy = _generate_default_security_policy(classpath)
+        security_policy_file.write(policy.encode(encoding=sys.getdefaultencoding()))
+        security_policy_file.flush()
+        yield pathlib.Path(security_policy_file.name)
+
+
+def _generate_default_security_policy(classpath: str) -> str:
+    """Generate the default security policy from the classpath. ``junit-4.12.jar``
+    must be on the classpath.
+    """
+    pattern = "{sep}([^{sep}]*{junit_jar}){sep}".format(
+        sep=os.pathsep, junit_jar=JUNIT_JAR
+    )
+    junit_jar_matches = re.search(pattern, classpath)
+    if not junit_jar_matches:
+        raise ValueError("{} not on the classpath".format(JUNIT_JAR))
+    path = junit_jar_matches.group(1)
+    return _DEFAULT_SECURITY_POLICY_TEMPLATE.format(junit4_jar_path=path)
+
+
 def get_num_failed(test_output: bytes) -> int:
     """Get the amount of failed tests from the error output of JUnit4."""
     decoded = test_output.decode(encoding=sys.getdefaultencoding())
@@ -47,6 +98,7 @@ def run_test_class(
     prod_class: pathlib.Path,
     classpath: str,
     verbose: bool = False,
+    security_policy: Optional[pathlib.Path] = None,
 ) -> Tuple[str, str]:
     """Run a single test class on a single production class.
 
@@ -69,7 +121,15 @@ def run_test_class(
     classpath = _java.generate_classpath(
         test_class_dir, prod_class_dir, classpath=classpath
     )
-    command = ["java", "-cp", classpath, "org.junit.runner.JUnitCore", test_class_name]
+
+    command = ["java"]
+    if security_policy:
+        command += [
+            "-Djava.security.manager",
+            "-Djava.security.policy=={!s}".format(security_policy),
+        ]
+    command += ["-cp", classpath, "org.junit.runner.JUnitCore", test_class_name]
+
     proc = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
     return _extract_results(proc, test_class_name, verbose)

repomate_junit4/junit4.py

@@ -22,6 +22,7 @@
 import configparser
 import re
 import pathlib
+import tempfile
 from typing import Union, Iterable, Tuple, List
 
 import daiquiri
@@ -36,9 +37,6 @@
 
 LOGGER = daiquiri.getLogger(__file__)
 
-HAMCREST_JAR = "hamcrest-core-1.3.jar"
-JUNIT_JAR = "junit-4.12.jar"
-
 ResultPair = Tuple[pathlib.Path, pathlib.Path]
 
 
@@ -57,6 +55,7 @@ def __init__(self):
         self._hamcrest_path = ""
         self._junit_path = ""
         self._classpath = ""
+        self._disable_security = False
 
     def act_on_cloned_repo(self, path: Union[str, pathlib.Path]) -> plug.HookResult:
         """Look for production classes in the student repo corresponding to
@@ -117,6 +116,9 @@ def parse_args(self, args: argparse.Namespace) -> None:
         )
         self._junit_path = args.junit_path if args.junit_path else self._junit_path
         self._verbose = args.verbose
+        self._disable_security = (
+            args.disable_security if args.disable_security else self._disable_security
+        )
 
     def clone_parser_hook(self, clone_parser: configparser.ConfigParser) -> None:
         """Add reference_tests_dir argument to parser.
@@ -143,19 +145,29 @@ def clone_parser_hook(self, clone_parser: configparser.ConfigParser) -> None:
         clone_parser.add_argument(
             "-ham",
             "--hamcrest-path",
-            help="Absolute path to the `{}` library.".format(HAMCREST_JAR),
+            help="Absolute path to the `{}` library.".format(
+                _junit4_runner.HAMCREST_JAR
+            ),
             type=str,
             # required if not picked up in config_hook nor on classpath
-            required=not self._hamcrest_path and not HAMCREST_JAR in self._classpath,
+            required=not self._hamcrest_path
+            and not _junit4_runner.HAMCREST_JAR in self._classpath,
         )
 
         clone_parser.add_argument(
             "-junit",
             "--junit-path",
-            help="Absolute path to the `{}` library.".format(JUNIT_JAR),
+            help="Absolute path to the `{}` library.".format(_junit4_runner.JUNIT_JAR),
             type=str,
             # required if not picked up in config_hook nor on classpath
-            required=not self._junit_path and not JUNIT_JAR in self._classpath,
+            required=not self._junit_path
+            and not _junit4_runner.JUNIT_JAR in self._classpath,
+        )
+
+        clone_parser.add_argument(
+            "--disable-security",
+            help="Disable the default security policy (student code can do whatever).",
+            action="store_true",
         )
 
         clone_parser.add_argument(
@@ -295,16 +307,23 @@ def _run_tests(
         """
         succeeded = []
         failed = []
-        for test_class, prod_class in test_prod_class_pairs:
-            classpath = self._generate_classpath()
-            status, msg = _junit4_runner.run_test_class(
-                test_class, prod_class, classpath=classpath, verbose=self._verbose
-            )
-            if status != Status.SUCCESS:
-                failed.append(plug.HookResult(SECTION, status, msg))
-            else:
-                succeeded.append(plug.HookResult(SECTION, status, msg))
-        return succeeded, failed
+        classpath = self._generate_classpath()
+        with _junit4_runner.security_policy(
+            classpath, active=not self._disable_security
+        ) as security_policy:
+            for test_class, prod_class in test_prod_class_pairs:
+                status, msg = _junit4_runner.run_test_class(
+                    test_class,
+                    prod_class,
+                    classpath=classpath,
+                    verbose=self._verbose,
+                    security_policy=security_policy,
+                )
+                if status != Status.SUCCESS:
+                    failed.append(plug.HookResult(SECTION, status, msg))
+                else:
+                    succeeded.append(plug.HookResult(SECTION, status, msg))
+            return succeeded, failed
 
     def _generate_classpath(self, *paths: pathlib.Path) -> str:
         """
@@ -317,10 +336,10 @@ def _generate_classpath(self, *paths: pathlib.Path) -> str:
             "`{}` is not configured and not on the CLASSPATH variable."
             "This will probably crash."
         )
-        if not (self._hamcrest_path or HAMCREST_JAR in self._classpath):
-            LOGGER.warning(warn.format(HAMCREST_JAR))
-        if not (self._junit_path or JUNIT_JAR in self._classpath):
-            LOGGER.warning(warn.format(JUNIT_JAR))
+        if not (self._hamcrest_path or _junit4_runner.HAMCREST_JAR in self._classpath):
+            LOGGER.warning(warn.format(_junit4_runner.HAMCREST_JAR))
+        if not (self._junit_path or _junit4_runner.JUNIT_JAR in self._classpath):
+            LOGGER.warning(warn.format(_junit4_runner.JUNIT_JAR))
 
         paths = list(paths)
         if self._hamcrest_path:

tests/conftest.py

@@ -1,17 +1,15 @@
-from repomate_junit4 import junit4
+from repomate_junit4._junit4_runner import HAMCREST_JAR, JUNIT_JAR
 
 from envvars import JUNIT_PATH, HAMCREST_PATH
 
-if JUNIT_PATH.name != junit4.JUNIT_JAR:
+if JUNIT_PATH.name != JUNIT_JAR:
     raise RuntimeError(
         "test suite requires the env variable "
-        "REPOMATE_JUNIT4_JUNIT to contain the path to `{}`".format(junit4.JUNIT_JAR)
+        "REPOMATE_JUNIT4_JUNIT to contain the path to `{}`".format(JUNIT_JAR)
     )
 
-if HAMCREST_PATH.name != junit4.HAMCREST_JAR:
+if HAMCREST_PATH.name != HAMCREST_JAR:
     raise RuntimeError(
         "test suite requires the env variable "
-        "REPOMATE_JUNIT4_HAMCREST to contain the path to `{}`".format(
-            junit4.HAMCREST_JAR
-        )
+        "REPOMATE_JUNIT4_HAMCREST to contain the path to `{}`".format(HAMCREST_JAR)
     )

tests/repos/unauthorized-network-access-week-10/README.md

@@ -0,0 +1,2 @@
+This repo has code that tries to access the network with a GET request. The
+default security policy should block the attempt.

tests/repos/unauthorized-network-access-week-10/src/Fibo.java

@@ -0,0 +1,40 @@
+/**
+ * Class for calculating Fibonacci numbers.
+ */
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.BufferedReader;
+
+public class Fibo {
+    private long prev;
+    private long current;
+
+    public Fibo() {
+        prev = 0;
+        current = 1;
+
+        try {
+            URL url = new URL("https://google.se");
+            HttpURLConnection con = (HttpURLConnection) url.openConnection();
+            BufferedReader reader = new BufferedReader(new InputStreamReader(con.getInputStream()));
+            String line;
+            while ((line = reader.readLine()) != null) {
+                System.out.println(line);
+            }
+        } catch (IOException e) {
+            // pass
+        }
+    }
+
+    /**
+     * Generate the next Fibonacci number.
+     */
+    public long next() {
+        long ret = prev;
+        prev = current;
+        current = ret + current;
+        return ret;
+    }
+}

tests/repos/unauthorized-read-file-week-10/README.md

@@ -0,0 +1,2 @@
+This repo has code that tries to read a file from the host's filesystem. The
+default security policy should block the attempt.

tests/repos/unauthorized-read-file-week-10/src/Fibo.java

@@ -0,0 +1,32 @@
+/**
+ * Class for calculating Fibonacci numbers.
+ */
+import java.io.FileReader;
+import java.io.IOException;
+
+public class Fibo {
+    private long prev;
+    private long current;
+
+    public Fibo() {
+        prev = 0;
+        current = 1;
+
+        try {
+            FileReader reader = new FileReader("../../secrets/token.txt");
+            reader.read();
+        } catch (IOException e) {
+            // pass
+        }
+    }
+
+    /**
+     * Generate the next Fibonacci number.
+     */
+    public long next() {
+        long ret = prev;
+        prev = current;
+        current = ret + current;
+        return ret;
+    }
+}

tests/secrets/README.md

@@ -0,0 +1,2 @@
+This directory contains "secrets" with highly public permissions used to test
+that student code can't reach such vulnurable targets.