From 99c831ab826b56e29339d68fdc4dc11b432ec37f Mon Sep 17 00:00:00 2001 From: valoq Date: Fri, 8 Apr 2022 22:59:47 +0200 Subject: [PATCH 1/3] improve seccomp filter --- zathura/seccomp-filters.c | 124 +++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 61 deletions(-) diff --git a/zathura/seccomp-filters.c b/zathura/seccomp-filters.c index 7c44995..2dc4b54 100644 --- a/zathura/seccomp-filters.c +++ b/zathura/seccomp-filters.c @@ -11,6 +11,9 @@ #include #include #include +#include /* for clone filter */ + + #define ADD_RULE(str_action, action, call, ...) \ do { \ @@ -100,8 +103,12 @@ seccomp_enable_basic_filter(void) DENY_RULE(uselib); DENY_RULE(vmsplice); - /* TODO: check for additional syscalls to blacklist */ - /* DENY_RULE (execve); */ + /*TODO + * + * In case this basic filter is actually triggered, print a clear error message to report this + * The syscalls here should never be executed by an unprivileged process + * + * */ /* applying filter... */ if (seccomp_load(ctx) >= 0) { @@ -142,13 +149,11 @@ seccomp_enable_strict_filter(void) } ALLOW_RULE(access); - /* ALLOW_RULE (arch_prctl); */ ALLOW_RULE(bind); ALLOW_RULE(brk); ALLOW_RULE(clock_getres); - ALLOW_RULE(clone); /* TODO: investigate */ + /* ALLOW_RULE(clone); specified below */ ALLOW_RULE(close); - /* ALLOW_RULE (connect); */ ALLOW_RULE(eventfd2); ALLOW_RULE(exit); ALLOW_RULE(exit_group); @@ -170,23 +175,19 @@ seccomp_enable_strict_filter(void) ALLOW_RULE(getpid); ALLOW_RULE(getppid); ALLOW_RULE(gettid); - /* ALLOW_RULE (getpeername); */ ALLOW_RULE(getrandom); ALLOW_RULE(getresgid); ALLOW_RULE(getresuid); ALLOW_RULE(getrlimit); ALLOW_RULE(getpeername); - /* ALLOW_RULE (getsockname); */ - /* ALLOW_RULE (getsockopt); needed for access to x11 socket in network namespace (without abstract sockets) */ ALLOW_RULE(inotify_add_watch); ALLOW_RULE(inotify_init1); ALLOW_RULE(inotify_rm_watch); - /* ALLOW_RULE (ioctl); specified below */ + /* ALLOW_RULE (ioctl); specified below */ ALLOW_RULE(lseek); ALLOW_RULE(lstat); ALLOW_RULE(madvise); ALLOW_RULE(memfd_create); - ALLOW_RULE(mkdir); /* needed for first run only */ ALLOW_RULE(mmap); ALLOW_RULE(mprotect); ALLOW_RULE(mremap); @@ -197,9 +198,8 @@ seccomp_enable_strict_filter(void) ALLOW_RULE(pipe); ALLOW_RULE(pipe2); ALLOW_RULE(poll); - ALLOW_RULE(pwrite64); /* TODO: build detailed filter */ + ALLOW_RULE(pwrite64); ALLOW_RULE(pread64); - /* ALLOW_RULE (prlimit64); */ /* ALLOW_RULE (prctl); specified below */ ALLOW_RULE(read); ALLOW_RULE(readlink); @@ -209,12 +209,12 @@ seccomp_enable_strict_filter(void) ALLOW_RULE(rseq); ALLOW_RULE(rt_sigaction); ALLOW_RULE(rt_sigprocmask); + ALLOW_RULE(sched_setattr); + ALLOW_RULE(sched_getattr); ALLOW_RULE(sendmsg); ALLOW_RULE(sendto); ALLOW_RULE(select); ALLOW_RULE(set_robust_list); - /* ALLOW_RULE (set_tid_address); */ - /* ALLOW_RULE (setsockopt); */ ALLOW_RULE(shmat); ALLOW_RULE(shmctl); ALLOW_RULE(shmdt); @@ -223,30 +223,51 @@ seccomp_enable_strict_filter(void) ALLOW_RULE(stat); ALLOW_RULE(statx); ALLOW_RULE(statfs); - /* ALLOW_RULE (socket); */ ALLOW_RULE(sysinfo); ALLOW_RULE(uname); ALLOW_RULE(unlink); - ALLOW_RULE(write); /* specified below (zathura needs to write files)*/ + ALLOW_RULE(write); ALLOW_RULE(writev); - ALLOW_RULE(wait4); /* trying to open links should not crash the app */ + ALLOW_RULE(wait4); - /* ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), sched_setattr, 0); */ - /* ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), sched_getattr, 0); */ - - /* required by glib */ - ALLOW_RULE(sched_setattr); - ALLOW_RULE(sched_getattr); /* required by some X11 setups */ - ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), umask, 0); - ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), socket, 0); + /* X11 no longer supported in strict sandbox mode */ + /* ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), umask, 0); */ + /* ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), socket, 0); */ /* required for testing only */ ALLOW_RULE(timer_create); ALLOW_RULE(timer_delete); + + /* filter clone arguments */ + ADD_RULE("allow", SCMP_ACT_ALLOW, clone, 1, SCMP_CMP(0, SCMP_CMP_EQ, \ + CLONE_VM | \ + CLONE_FS | \ + CLONE_FILES | \ + CLONE_SIGHAND | \ + CLONE_THREAD | \ + CLONE_SYSVSEM | \ + CLONE_SETTLS | \ + CLONE_PARENT_SETTID | \ + CLONE_CHILD_CLEARTID)); + + + + /* fcntl filter - not yet working */ + /*ADD_RULE("allow", SCMP_ACT_ALLOW, fcntl, 1, SCMP_CMP(0, SCMP_CMP_EQ, \ + F_GETFL | \ + F_SETFL | \ + F_ADD_SEALS | \ + F_SEAL_SEAL | \ + F_SEAL_SHRINK | \ + F_DUPFD_CLOEXEC | \ + F_SETFD | \ + FD_CLOEXEC )); */ + + /* Special requirements for ioctl, allowed on stdout/stderr */ ADD_RULE("allow", SCMP_ACT_ALLOW, ioctl, 1, SCMP_CMP(0, SCMP_CMP_EQ, 1)); ADD_RULE("allow", SCMP_ACT_ALLOW, ioctl, 1, SCMP_CMP(0, SCMP_CMP_EQ, 2)); @@ -256,53 +277,34 @@ seccomp_enable_strict_filter(void) ADD_RULE("allow", SCMP_ACT_ALLOW, prctl, 1, SCMP_CMP(0, SCMP_CMP_EQ, PR_SET_PDEATHSIG)); /* special restrictions for open, prevent opening files for writing */ - ADD_RULE("allow", SCMP_ACT_ALLOW, open, 1, SCMP_CMP(1, SCMP_CMP_MASKED_EQ, O_WRONLY | O_RDWR, 0)); + ADD_RULE("allow", SCMP_ACT_ALLOW, open, 1, SCMP_CMP(1, SCMP_CMP_MASKED_EQ, O_WRONLY | O_RDWR, 0)); ADD_RULE("errno", SCMP_ACT_ERRNO(EACCES), open, 1, SCMP_CMP(1, SCMP_CMP_MASKED_EQ, O_WRONLY, O_WRONLY)); ADD_RULE("errno", SCMP_ACT_ERRNO(EACCES), open, 1, SCMP_CMP(1, SCMP_CMP_MASKED_EQ, O_RDWR, O_RDWR)); /* special restrictions for openat, prevent opening files for writing */ - ADD_RULE("allow", SCMP_ACT_ALLOW, openat, 1, SCMP_CMP(2, SCMP_CMP_MASKED_EQ, O_WRONLY | O_RDWR, 0)); + ADD_RULE("allow", SCMP_ACT_ALLOW, openat, 1, SCMP_CMP(2, SCMP_CMP_MASKED_EQ, O_WRONLY | O_RDWR, 0)); ADD_RULE("errno", SCMP_ACT_ERRNO(EACCES), openat, 1, SCMP_CMP(2, SCMP_CMP_MASKED_EQ, O_WRONLY, O_WRONLY)); ADD_RULE("errno", SCMP_ACT_ERRNO(EACCES), openat, 1, SCMP_CMP(2, SCMP_CMP_MASKED_EQ, O_RDWR, O_RDWR)); - /* allowed for debugging: */ - - /* ALLOW_RULE (prctl); */ - /* ALLOW_RULE (ioctl); */ - - /* TODO: test fcntl rules */ - /* if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl), 1, */ - /* SCMP_CMP(0, SCMP_CMP_EQ, F_GETFL)) < 0) */ - /* goto out; */ - - /* if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl), 1, */ - /* SCMP_CMP(0, SCMP_CMP_EQ, F_SETFL)) < 0) */ - /* goto out; */ - - /* if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl), 1, */ - /* SCMP_CMP(0, SCMP_CMP_EQ, F_SETFD)) < 0) */ - /* goto out; */ - - /* if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl), 1, */ - /* SCMP_CMP(0, SCMP_CMP_EQ, F_GETFD)) < 0) */ - /* goto out; */ - - /* if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(fcntl), 1, */ - /* SCMP_CMP(0, SCMP_CMP_EQ, F_SETLK)) < 0) */ - /* goto out; */ - /* TODO: build detailed filter for prctl */ - /* needed by gtk??? (does not load content without) */ - /* /\* special restrictions for prctl, only allow PR_SET_NAME/PR_SET_PDEATHSIG *\/ */ - /* if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(prctl), 1, */ - /* SCMP_CMP(0, SCMP_CMP_EQ, PR_SET_NAME)) < 0) */ - /* goto out; */ + /* Sandbox Status Notes: + * + * write: no actual files on the filesystem are opened with write permissions + * exception is /run/user/UID/dconf/user (file descriptor not available during runtime) + * + * + * mkdir: needed for first run only to create /run/user/UID/dconf (before seccomp init) + * wait4: required to attempt opening links (which is then blocked) + * + * X11 environments require umask and socket syscalls after sandbox setup + * no longer supported since X11 cannot be easily secured anyway + * + * TODO: prevent dbus socket connection before sandbox init - by checking the sandbox settings in zathurarc + * + */ - /* if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(prctl), 1, */ - /* SCMP_CMP(0, SCMP_CMP_EQ, PR_SET_PDEATHSIG)) < 0) */ - /* goto out; */ /* when zathura is run on wayland, with X11 server available but blocked, unset the DISPLAY variable */ /* otherwise it will try to connect to X11 using inet socket protocol */ From b25637a8be5cc90f7bb93aa3cbeb4f229c3826ff Mon Sep 17 00:00:00 2001 From: valoq Date: Wed, 13 Apr 2022 10:22:59 +0200 Subject: [PATCH 2/3] Allow restricted socket syscall for X11 support --- zathura/seccomp-filters.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/zathura/seccomp-filters.c b/zathura/seccomp-filters.c index 2dc4b54..02e29bd 100644 --- a/zathura/seccomp-filters.c +++ b/zathura/seccomp-filters.c @@ -224,23 +224,21 @@ seccomp_enable_strict_filter(void) ALLOW_RULE(statx); ALLOW_RULE(statfs); ALLOW_RULE(sysinfo); + ALLOW_RULE(umask); /* required by X11 */ ALLOW_RULE(uname); ALLOW_RULE(unlink); ALLOW_RULE(write); ALLOW_RULE(writev); ALLOW_RULE(wait4); - - /* required by some X11 setups */ - /* X11 no longer supported in strict sandbox mode */ - /* ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), umask, 0); */ - /* ADD_RULE("errno", SCMP_ACT_ERRNO(EPERM), socket, 0); */ - - /* required for testing only */ ALLOW_RULE(timer_create); ALLOW_RULE(timer_delete); + + /* permit the socket syscall for local UNIX domain sockets (required by X11) */ + ADD_RULE("allow", SCMP_ACT_ALLOW, socket, 1, SCMP_CMP(0, SCMP_CMP_EQ, AF_UNIX)); + /* filter clone arguments */ ADD_RULE("allow", SCMP_ACT_ALLOW, clone, 1, SCMP_CMP(0, SCMP_CMP_EQ, \ @@ -255,7 +253,6 @@ seccomp_enable_strict_filter(void) CLONE_CHILD_CLEARTID)); - /* fcntl filter - not yet working */ /*ADD_RULE("allow", SCMP_ACT_ALLOW, fcntl, 1, SCMP_CMP(0, SCMP_CMP_EQ, \ F_GETFL | \ @@ -303,6 +300,7 @@ seccomp_enable_strict_filter(void) * * TODO: prevent dbus socket connection before sandbox init - by checking the sandbox settings in zathurarc * + * TODO: check requirement of pipe/pipe2 syscalls when dbus is disabled */ From de0d881f9c536c8afa18f5ab93f000812c23acef Mon Sep 17 00:00:00 2001 From: valoq Date: Sun, 17 Apr 2022 18:28:34 +0200 Subject: [PATCH 3/3] Permit some syscalls on X11 only --- zathura/seccomp-filters.c | 26 +++++++++++++++++++++++--- zathura/seccomp-filters.h | 4 +++- zathura/zathura.c | 2 +- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/zathura/seccomp-filters.c b/zathura/seccomp-filters.c index 02e29bd..6ac2a0e 100644 --- a/zathura/seccomp-filters.c +++ b/zathura/seccomp-filters.c @@ -13,6 +13,9 @@ #include #include /* for clone filter */ +#ifdef GDK_WINDOWING_X11 +#include +#endif #define ADD_RULE(str_action, action, call, ...) \ @@ -124,7 +127,7 @@ out: } int -seccomp_enable_strict_filter(void) +seccomp_enable_strict_filter(zathura_t* zathura) { /* prevent child processes from getting more priv e.g. via setuid, capabilities, ... */ if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) { @@ -235,9 +238,26 @@ seccomp_enable_strict_filter(void) ALLOW_RULE(timer_create); ALLOW_RULE(timer_delete); + +/* Permit X11 specific syscalls */ +#ifdef GDK_WINDOWING_X11 + GdkDisplay* display = gtk_widget_get_display(zathura->ui.session->gtk.view); + + if (GDK_IS_X11_DISPLAY (display)) { - /* permit the socket syscall for local UNIX domain sockets (required by X11) */ - ADD_RULE("allow", SCMP_ACT_ALLOW, socket, 1, SCMP_CMP(0, SCMP_CMP_EQ, AF_UNIX)); + girara_debug("On X11, supporting X11 syscalls"); + + /* permit the socket syscall for local UNIX domain sockets (required by X11) */ + ADD_RULE("allow", SCMP_ACT_ALLOW, socket, 1, SCMP_CMP(0, SCMP_CMP_EQ, AF_UNIX)); + + ALLOW_RULE(mkdir); + ALLOW_RULE(setsockopt); + ALLOW_RULE(connect); + } + else { + girara_debug("On Wayland, blocking X11 syscalls"); + } +#endif /* filter clone arguments */ diff --git a/zathura/seccomp-filters.h b/zathura/seccomp-filters.h index 57bfbb1..b934da5 100644 --- a/zathura/seccomp-filters.h +++ b/zathura/seccomp-filters.h @@ -3,6 +3,8 @@ #ifndef ZATHURA_SECCOMP_FILTERS_H #define ZATHURA_SECCOMP_FILTERS_H +#include "zathura.h" + /* basic filter */ /* this mode allows normal use */ /* only dangerous syscalls are blacklisted */ @@ -10,6 +12,6 @@ int seccomp_enable_basic_filter(void); /* strict filter before document parsing */ /* this filter is to be enabled after most of the initialisation of zathura has finished */ -int seccomp_enable_strict_filter(void); +int seccomp_enable_strict_filter(zathura_t* zathura); #endif diff --git a/zathura/zathura.c b/zathura/zathura.c index b3222ce..b2ccc4b 100644 --- a/zathura/zathura.c +++ b/zathura/zathura.c @@ -451,7 +451,7 @@ zathura_init(zathura_t* zathura) break; case ZATHURA_SANDBOX_STRICT: girara_debug("Strict sandbox preventing write and network access."); - if (seccomp_enable_strict_filter() != 0) { + if (seccomp_enable_strict_filter(zathura) != 0) { girara_error("Failed to initialize strict seccomp filter."); goto error_free; }