From 563e973944ba84547e1942e1b2f68094a5d165af Mon Sep 17 00:00:00 2001 From: Keenan Gugeler Date: Mon, 30 Aug 2021 15:29:32 -0400 Subject: [PATCH] change regex to filesystem policy matcher, first step of #871 --- .freebsd.test.py | 8 +- dmoj/cptbox/isolate.py | 13 +-- dmoj/executors/COFFEE.py | 3 +- dmoj/executors/DART.py | 1 - dmoj/executors/FORTH.py | 3 +- dmoj/executors/PERL.py | 3 +- dmoj/executors/PHP.py | 2 - dmoj/executors/RKT.py | 6 +- dmoj/executors/RUBY2.py | 3 +- dmoj/executors/SWIFT.py | 1 - dmoj/executors/TUR.py | 3 +- dmoj/executors/filesystem_policies.py | 114 ++++++++++++++++++++++++++ dmoj/executors/java_executor.py | 7 +- dmoj/executors/mixins.py | 95 +++++++++++++++------ dmoj/executors/mono_executor.py | 3 +- dmoj/executors/script_executor.py | 6 +- dmoj/judgeenv.py | 6 +- dmoj/tests/test_filesystem_policy.py | 53 ++++++++++++ dmoj/utils/helper_files.py | 3 +- 19 files changed, 276 insertions(+), 57 deletions(-) create mode 100644 dmoj/executors/filesystem_policies.py create mode 100644 dmoj/tests/test_filesystem_policy.py diff --git a/.freebsd.test.py b/.freebsd.test.py index 9a3f352da..abcac979c 100644 --- a/.freebsd.test.py +++ b/.freebsd.test.py @@ -31,13 +31,17 @@ def main(): judgeenv.env['runtime'] = {} - judgeenv.env['extra_fs'] = {'PERL': ['/dev/dtrace/helper$'], 'RUBY2': ['/dev/dtrace/helper$']} + judgeenv.env['extra_fs'] = { + 'PERL': {"exact_file": ["/dev/dtrace/helper"]}, + 'RUBY2': {"exact_file": ["/dev/dtrace/helper"]} + } logging.basicConfig(level=logging.INFO) print('Using extra allowed filesystems:') for lang, fs in judgeenv.env['extra_fs'].iteritems(): - print('%-6s: %s' % (lang, '|'.join(fs))) + for access_type, files in fs.iteritems(): + print('%-6s: %s: %s' % (lang, access_type, '|'.join([x for x in files]))) print() print('Testing executors...') diff --git a/dmoj/cptbox/isolate.py b/dmoj/cptbox/isolate.py index 9c6c5f12f..438d87fee 100644 --- a/dmoj/cptbox/isolate.py +++ b/dmoj/cptbox/isolate.py @@ -1,6 +1,5 @@ import logging import os -import re import sys from dmoj.cptbox._cptbox import AT_FDCWD, bsd_get_proc_cwd, bsd_get_proc_fdno @@ -9,6 +8,7 @@ from dmoj.cptbox.syscalls import * from dmoj.utils.unicode import utf8text +from dmoj.executors.filesystem_policies import FilesystemPolicy log = logging.getLogger('dmoj.security') open_write_flags = [os.O_WRONLY, os.O_RDWR, os.O_TRUNC, os.O_CREAT, os.O_EXCL] @@ -184,12 +184,7 @@ def __init__(self, read_fs, write_fs=None, writable=(1, 2)): ) def _compile_fs_jail(self, fs): - if fs: - fs_re = '|'.join(fs) - else: - fs_re = '(?!)' # Disallow accessing everything by default. - - return re.compile(fs_re) + return FilesystemPolicy(fs or []) def is_write_flags(self, open_flags): for flag in open_write_flags: @@ -258,9 +253,7 @@ def _file_access_check(self, rel_file, debugger, is_open, flag_reg=1, dirfd=AT_F is_write = is_open and self.is_write_flags(getattr(debugger, 'uarg%d' % flag_reg)) fs_jail = self.write_fs_jail if is_write else self.read_fs_jail - if fs_jail.match(file) is None: - return file, False - return file, True + return file, fs_jail.check(file) def get_full_path(self, debugger, file, dirfd=AT_FDCWD): dirfd = (dirfd & 0x7FFFFFFF) - (dirfd & 0x80000000) diff --git a/dmoj/executors/COFFEE.py b/dmoj/executors/COFFEE.py index 090da03a9..9846f8efe 100644 --- a/dmoj/executors/COFFEE.py +++ b/dmoj/executors/COFFEE.py @@ -1,6 +1,7 @@ import os from dmoj.executors.script_executor import ScriptExecutor +from dmoj.executors.filesystem_policies import ExactFile class Executor(ScriptExecutor): @@ -38,7 +39,7 @@ def get_cmdline(self, **kwargs): return [self.get_command(), self.runtime_dict['coffee'], self._code] def get_fs(self): - return super().get_fs() + [self.runtime_dict['coffee'], self._code] + return super().get_fs() + [ExactFile(self.runtime_dict['coffee']), ExactFile(self._code)] @classmethod def get_versionable_commands(cls): diff --git a/dmoj/executors/DART.py b/dmoj/executors/DART.py index 3310626a6..9889b0b7c 100644 --- a/dmoj/executors/DART.py +++ b/dmoj/executors/DART.py @@ -16,7 +16,6 @@ class Executor(CompiledExecutor): address_grace = 128 * 1024 syscalls = ['epoll_create', 'epoll_ctl', 'epoll_wait', 'timerfd_settime', 'memfd_create', 'ftruncate'] - fs = ['.*/vm-service$'] def get_compile_args(self): return [self.get_command(), '--snapshot=%s' % self.get_compiled_file(), self._code] diff --git a/dmoj/executors/FORTH.py b/dmoj/executors/FORTH.py index e1df148d5..52b4c3aaa 100644 --- a/dmoj/executors/FORTH.py +++ b/dmoj/executors/FORTH.py @@ -1,4 +1,5 @@ from dmoj.executors.script_executor import ScriptExecutor +from dmoj.executors.filesystem_policies import ExactFile class Executor(ScriptExecutor): @@ -10,7 +11,7 @@ class Executor(ScriptExecutor): HELLO ''' - fs = [r'/\.gforth-history$'] + fs = [ExactFile('/.gforth-history')] def get_cmdline(self, **kwargs): return [self.get_command(), self._code, '-e', 'bye'] diff --git a/dmoj/executors/PERL.py b/dmoj/executors/PERL.py index f18e30b7c..57aeeadf4 100644 --- a/dmoj/executors/PERL.py +++ b/dmoj/executors/PERL.py @@ -1,11 +1,12 @@ from dmoj.executors.script_executor import ScriptExecutor +from dmoj.executors.filesystem_policies import RecursiveDir class Executor(ScriptExecutor): ext = 'pl' name = 'PERL' command = 'perl' - fs = ['/etc/perl/.*?'] + fs = [RecursiveDir('/etc/perl')] test_program = 'print<>' syscalls = ['umtx_op'] diff --git a/dmoj/executors/PHP.py b/dmoj/executors/PHP.py index fdc4f9db5..da9e23fc4 100644 --- a/dmoj/executors/PHP.py +++ b/dmoj/executors/PHP.py @@ -7,8 +7,6 @@ class Executor(ScriptExecutor): command = 'php' command_paths = ['php7', 'php5', 'php'] - fs = [r'.*/php[\w-]*\.ini$', r'.*/conf.d/.*\.ini$'] - test_program = ' bool: + if path == "/": + return self._check_final_node(self.root) + + assert os.path.isabs(path), "Must pass an absolute path to check" + + node = self.root + for component in path.split("/")[1:]: + assert component != "", "Must not have empty components in path to check" + if isinstance(node, File): + return False + elif node.access_mode == ACCESS_MODE.RECURSIVE: + return True + else: + node = node.map.get(component) + if node is None: + return False + + return self._check_final_node(node) + + def _check_final_node(self, node: Union[Dir, File]) -> bool: + return isinstance(node, File) or node.access_mode != ACCESS_MODE.NONE diff --git a/dmoj/executors/java_executor.py b/dmoj/executors/java_executor.py index 5d0b829ac..0cca6ec89 100644 --- a/dmoj/executors/java_executor.py +++ b/dmoj/executors/java_executor.py @@ -12,6 +12,7 @@ from dmoj.executors.mixins import SingleDigitVersionMixin from dmoj.judgeenv import skip_self_test from dmoj.utils.unicode import utf8bytes, utf8text +from dmoj.executors.filesystem_policies import ExactFile, ExactDir recomment = re.compile(r'/\*.*?\*/', re.DOTALL | re.U) restring = re.compile(r''''(?:\\.|[^'\\])'|"(?:\\.|[^"\\])*"''', re.DOTALL | re.U) @@ -82,11 +83,11 @@ def get_executable(self): return self.get_vm() def get_fs(self): - return super().get_fs() + [f'{re.escape(self._agent_file)}$'] + \ - [f'{re.escape(str(parent))}$' for parent in PurePath(self._agent_file).parents] + return super().get_fs() + [ExactFile(self._agent_file)] + \ + [ExactDir(str(parent)) for parent in PurePath(self._agent_file).parents] def get_write_fs(self): - return super().get_write_fs() + [os.path.join(self._dir, 'submission_jvm_crash.log')] + return super().get_write_fs() + [ExactFile(os.path.join(self._dir, 'submission_jvm_crash.log'))] def get_agent_flag(self): agent_flag = '-javaagent:%s=policy:%s' % (self._agent_file, self._policy_file) diff --git a/dmoj/executors/mixins.py b/dmoj/executors/mixins.py index 05483c2a3..7cd122bed 100644 --- a/dmoj/executors/mixins.py +++ b/dmoj/executors/mixins.py @@ -11,39 +11,79 @@ from dmoj.judgeenv import env from dmoj.utils import setbufsize_path from dmoj.utils.unicode import utf8bytes +from dmoj.executors.filesystem_policies import Rule, ExactDir, RecursiveDir, ExactFile BASE_FILESYSTEM = [ - '/dev/(?:null|tty|zero|u?random)$', - '/usr/(?!home)', - '/lib(?:32|64)?/', - '/opt/', - '/etc$', - '/etc/(?:localtime|timezone)$', - '/usr$', - '/tmp$', - '/$', + ExactFile("/dev/null"), + ExactFile("/dev/tty"), + ExactFile("/dev/zero"), + ExactFile("/dev/urandom"), + ExactFile("/dev/random"), + *[RecursiveDir(f"/usr/{d}") for d in os.listdir("/usr") if d != "home" and os.path.isdir(f"/usr/{d}")], + + RecursiveDir("/lib"), + RecursiveDir("/lib32"), + RecursiveDir("/lib64"), + RecursiveDir("/opt"), + + ExactDir("/etc"), + ExactFile("/etc/localtime"), + ExactFile("/etc/timezone"), + + ExactDir("/usr"), + ExactDir("/tmp"), + ExactDir("/") ] -BASE_WRITE_FILESYSTEM = ['/dev/stdout$', '/dev/stderr$', '/dev/null$'] + +BASE_WRITE_FILESYSTEM = [ + ExactFile("/dev/stdout"), + ExactFile("/dev/stderr"), + ExactFile("/dev/null") +] + if 'freebsd' in sys.platform: - BASE_FILESYSTEM += [r'/etc/s?pwd\.db$', '/dev/hv_tsc$'] + BASE_FILESYSTEM += [ + ExactFile("/etc/spwd.db"), + ExactFile("/etc/pwd.db"), + ExactFile("/dev/hv_tsc") + ] + else: - BASE_FILESYSTEM += ['/sys/devices/system/cpu(?:$|/online)', '/etc/selinux/config$'] + BASE_FILESYSTEM += [ + ExactDir("/sys/devices/system/cpu"), + ExactFile("/sys/devices/system/cpu/online"), + ExactFile("/etc/selinux/config") + ] if sys.platform.startswith('freebsd'): - BASE_FILESYSTEM += [r'/etc/libmap\.conf$', r'/var/run/ld-elf\.so\.hints$'] + BASE_FILESYSTEM += [ + ExactFile("/etc/libmap.conf"), + ExactFile("/var/run/ld-elf.so.hints") + ] else: # Linux and kFreeBSD mounts linux-style procfs. BASE_FILESYSTEM += [ - '/proc$', - '/proc/self/(?:maps|exe|auxv)$', - '/proc/self$', - '/proc/(?:meminfo|stat|cpuinfo|filesystems|xen|uptime)$', - '/proc/sys/vm/overcommit_memory$', + ExactDir("/proc"), + ExactDir("/proc/self"), + ExactFile("/proc/self/maps"), + ExactFile("/proc/self/exe"), + ExactFile("/proc/auxv"), + ExactFile("/proc/meminfo"), + ExactFile("/proc/stat"), + ExactFile("/proc/cpuinfo"), + ExactFile("/proc/filesystems"), + ExactFile("/proc/xen"), + ExactFile("/proc/uptime"), + ExactFile("/proc/sys/vm/overcommit_memory") ] # Linux-style ld. - BASE_FILESYSTEM += [r'/etc/ld\.so\.(?:nohwcap|preload|cache)$'] + BASE_FILESYSTEM += [ + ExactFile("/etc/ld.so.nohwcap"), + ExactFile("/etc/ld.so.preload"), + ExactFile("/etc/ld.so.cache") + ] UTF8_LOCALE = 'C.UTF-8' @@ -56,8 +96,8 @@ class PlatformExecutorMixin(metaclass=abc.ABCMeta): data_grace = 0 fsize = 0 personality = 0x0040000 # ADDR_NO_RANDOMIZE - fs: List[str] = [] - write_fs: List[str] = [] + fs: List[Rule] = [] + write_fs: List[Rule] = [] syscalls: List[Union[str, Tuple[str, Any]]] = [] def _add_syscalls(self, sec): @@ -74,10 +114,19 @@ def get_security(self, launch_kwargs=None): return self._add_syscalls(sec) def get_fs(self): - name = self.get_executor_name() - fs = BASE_FILESYSTEM + self.fs + env.get('extra_fs', {}).get(name, []) + [re.escape(self._dir)] + extra_fs = self._load_extra_fs() + + fs = BASE_FILESYSTEM + self.fs + extra_fs + [RecursiveDir(self._dir)] return fs + def _load_extra_fs(self): + name = self.get_executor_name() + extra_fs_config = env.get('extra_fs', {}).get(name, {}) + exact_files = [ExactFile(x) for x in extra_fs_config.get("exact_file", [])] + exact_dirs = [ExactDir(x) for x in extra_fs_config.get("exact_dir", [])] + recursive_dirs = [RecursiveDir(x) for x in extra_fs_config.get("recursive_dir", [])] + return exact_files + exact_dirs + recursive_dirs + def get_write_fs(self): return BASE_WRITE_FILESYSTEM + self.write_fs diff --git a/dmoj/executors/mono_executor.py b/dmoj/executors/mono_executor.py index 6fd4e6dc9..401c946d4 100644 --- a/dmoj/executors/mono_executor.py +++ b/dmoj/executors/mono_executor.py @@ -7,6 +7,7 @@ from dmoj.executors.compiled_executor import CompiledExecutor from dmoj.result import Result from dmoj.utils.unicode import utf8text +from dmoj.executors.filesystem_policies import RecursiveDir reexception = re.compile(r'\bFATAL UNHANDLED EXCEPTION: (.*?):', re.U) @@ -32,7 +33,7 @@ class MonoExecutor(CompiledExecutor): # get flagged as MLE. data_grace = 65536 cptbox_popen_class = MonoTracedPopen - fs = ['/etc/mono/'] + fs = [RecursiveDir('/etc/mono')] # Mono sometimes forks during its crashdump procedure, but continues even if # the call to fork fails. syscalls = [ diff --git a/dmoj/executors/script_executor.py b/dmoj/executors/script_executor.py index 18e53bf99..e28715d2c 100644 --- a/dmoj/executors/script_executor.py +++ b/dmoj/executors/script_executor.py @@ -1,9 +1,9 @@ import os -import re from typing import List, Optional from dmoj.executors.base_executor import BaseExecutor from dmoj.utils.unicode import utf8bytes +from dmoj.executors.filesystem_policies import ExactFile, RecursiveDir class ScriptExecutor(BaseExecutor): @@ -24,9 +24,9 @@ def get_command(cls) -> Optional[str]: def get_fs(self) -> list: home = self.runtime_dict.get('%s_home' % self.get_executor_name().lower()) - fs = super().get_fs() + [self._code] + fs = super().get_fs() + [ExactFile(self._code)] if home is not None: - fs.append(re.escape(home)) + fs += [RecursiveDir(home)] return fs def create_files(self, problem_id: str, source_code: bytes) -> None: diff --git a/dmoj/judgeenv.py b/dmoj/judgeenv.py index c3eb966cb..fe3b3459f 100644 --- a/dmoj/judgeenv.py +++ b/dmoj/judgeenv.py @@ -27,9 +27,13 @@ 'compiled_binary_cache_dir': None, # Location to store cached binaries, defaults to tempdir 'compiled_binary_cache_size': 100, # Maximum number of executables to cache (LRU order) 'runtime': {}, - # Map of executor: [list of extra allowed file regexes], used to configure + # Map of executor: fs_config, used to configure # the filesystem sandbox on a per-machine basis, without having to hack # executor source. + # fs_config is a dictionary. Three keys are possible: + # `exact_file`, to allow a specific file + # `exact_dir`, to allow listing files in a directory + # `recursive_dir`, to allow everything under and including a directory 'extra_fs': {}, # List of judge URLs to ping on problem data updates (the URLs are expected # to host judges running with --api-host and --api-port) diff --git a/dmoj/tests/test_filesystem_policy.py b/dmoj/tests/test_filesystem_policy.py new file mode 100644 index 000000000..3a36c859b --- /dev/null +++ b/dmoj/tests/test_filesystem_policy.py @@ -0,0 +1,53 @@ +import unittest +from dmoj.executors.filesystem_policies import FilesystemPolicy, ExactFile, ExactDir, RecursiveDir + + +class CheckerTest(unittest.TestCase): + def test_exact_dir(self): + self.fs = FilesystemPolicy([ExactDir("/etc")]) + + self.checkTrue("/etc") + + self.checkFalse("/") + self.checkFalse("/etc/passwd") + self.checkFalse("/etc/shadow") + + def test_recursive_dir(self): + self.fs = FilesystemPolicy([RecursiveDir("/usr")]) + + self.checkTrue("/usr") + self.checkTrue("/usr/lib") + self.checkTrue("/usr/lib/a/b/c/d/e") + + self.checkFalse("/") + self.checkFalse("/etc") + self.checkFalse("/us") + self.checkFalse("/usr2") + + def test_exact_file(self): + self.fs = FilesystemPolicy([ExactFile("/etc/passwd")]) + + self.checkTrue("/etc/passwd") + + self.checkFalse("/") + self.checkFalse("/etc") + self.checkFalse("/etc/p") + self.checkFalse("/etc/passwd2") + + def test_rule_check(self): + self.assertRaises(AssertionError, FilesystemPolicy, [ExactFile("/usr/lib")]) + self.assertRaises(AssertionError, FilesystemPolicy, [ExactDir("/etc/passwd")]) + self.assertRaises(AssertionError, FilesystemPolicy, [RecursiveDir("/etc/passwd")]) + + def test_absolute_check_on_build(self): + self.assertRaises(AssertionError, FilesystemPolicy, [ExactFile("usr/lib")]) + + def test_absolute_check_on_check(self): + self.fs = FilesystemPolicy([]) + self.assertRaises(AssertionError, self.fs.check, "usr/lib") + + def checkTrue(self, path): + self.assertTrue(self.fs.check(path)) + + def checkFalse(self, path): + self.assertFalse(self.fs.check(path)) diff --git a/dmoj/utils/helper_files.py b/dmoj/utils/helper_files.py index c2888f9bd..44ad64551 100644 --- a/dmoj/utils/helper_files.py +++ b/dmoj/utils/helper_files.py @@ -4,6 +4,7 @@ from dmoj.error import InternalError from dmoj.result import Result from dmoj.utils.os_ext import strsignal +from dmoj.executors.filesystem_policies import RecursiveDir def mktemp(data): @@ -41,7 +42,7 @@ def find_runtime(languages): executor = executor.Executor - kwargs = {'fs': executor.fs + [tempfile.gettempdir()]} + kwargs = {'fs': executor.fs + [RecursiveDir(tempfile.gettempdir())]} if issubclass(executor, CompiledExecutor): kwargs['compiler_time_limit'] = compiler_time_limit