From 4a79c2163d3430237ad626a29a627a0568a536ef Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Sat, 30 Dec 2017 15:05:12 +0100 Subject: [PATCH 1/7] Proposal on sandbox support for different implementations Related to GitHub issue #269 and #271. --- Makefile.am | 6 +++++- compat-sandbox.c | 38 ++++++++++++++++++++++++++++++++++++++ compat.h | 6 ++++++ pick.c | 12 +++--------- 4 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 compat-sandbox.c diff --git a/Makefile.am b/Makefile.am index 8c7876aa..5b54deec 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4,7 +4,11 @@ AM_CFLAGS=-Wall -Werror -Wextra AM_CPPFLAGS=-D_GNU_SOURCE bin_PROGRAMS=pick -pick_SOURCES=pick.c compat-reallocarray.c compat-strtonum.c compat.h +pick_SOURCES= pick.c \ + compat-reallocarray.c \ + compat-sandbox.c \ + compat-strtonum.c \ + compat.h pick_CPPFLAGS=$(AM_CPPFLAGS) $(NCURSES_CFLAGS) pick_LDADD=$(NCURSES_LIBS) diff --git a/compat-sandbox.c b/compat-sandbox.c new file mode 100644 index 00000000..aa2aff99 --- /dev/null +++ b/compat-sandbox.c @@ -0,0 +1,38 @@ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#ifdef HAVE_PLEDGE + +#include "compat.h" + +#include +#include + +void +sandbox(int stage) +{ + switch (stage) { + case SANDBOX_ENTER: + if (pledge("stdio tty rpath wpath cpath", NULL) == -1) + err(1, "pledge"); + break; + case SANDBOX_MAIN_LOOP_ENTER: + if (pledge("stdio tty", NULL) == -1) + err(1, "pledge"); + break; + case SANDBOX_MAIN_LOOP_EXIT: + if (pledge("stdio", NULL) == -1) + err(1, "pledge"); + break; + } +} + +#else + +void +sandbox(int stage __attribute__((unused))) +{ +} + +#endif diff --git a/compat.h b/compat.h index f8bbc0ce..af579ea6 100644 --- a/compat.h +++ b/compat.h @@ -31,4 +31,10 @@ long long strtonum(const char *, long long, long long, const char **); #endif /* !HAVE_STRTONUM */ +void sandbox(int); + +#define SANDBOX_ENTER 0 +#define SANDBOX_MAIN_LOOP_ENTER 1 +#define SANDBOX_MAIN_LOOP_EXIT 2 + #endif /* COMPAT_H */ diff --git a/pick.c b/pick.c index 128b47a3..a7fa1a32 100644 --- a/pick.c +++ b/pick.c @@ -124,10 +124,7 @@ main(int argc, char *argv[]) setlocale(LC_CTYPE, ""); -#ifdef HAVE_PLEDGE - if (pledge("stdio tty rpath wpath cpath", NULL) == -1) - err(1, "pledge"); -#endif + sandbox(SANDBOX_ENTER); while ((c = getopt(argc, argv, "dhoq:KSvxX")) != -1) switch (c) { @@ -181,13 +178,10 @@ main(int argc, char *argv[]) input = get_choices(); tty_init(1); -#ifdef HAVE_PLEDGE - if (pledge("stdio tty", NULL) == -1) - err(1, "pledge"); -#endif - + sandbox(SANDBOX_MAIN_LOOP_ENTER); choice = selected_choice(); tty_restore(1); + sandbox(SANDBOX_MAIN_LOOP_EXIT); if (choice != NULL) { printf("%s\n", choice->string); if (output_description) From f317de0f18b9ed3ff8f5e597cf181c74f7bd0cad Mon Sep 17 00:00:00 2001 From: Jenz Guenther Date: Tue, 2 Jan 2018 19:20:43 +0100 Subject: [PATCH 2/7] Add seccomp support for sandbox --- compat-sandbox.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++-- config.h.in | 3 +++ configure.ac | 2 ++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/compat-sandbox.c b/compat-sandbox.c index aa2aff99..a182d222 100644 --- a/compat-sandbox.c +++ b/compat-sandbox.c @@ -4,11 +4,11 @@ #ifdef HAVE_PLEDGE -#include "compat.h" - #include #include +#include "compat.h" + void sandbox(int stage) { @@ -28,6 +28,68 @@ sandbox(int stage) } } +#elif HAVE_SECCOMP + +#include + +#include +#include +#include + +#include "compat.h" + +#define ALLOW(syscall) \ + (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(syscall), 0) < 0) + +#define ALLOW_IOCTL(syscall, x) \ + (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(ioctl), x, \ + SCMP_A1(SCMP_CMP_EQ, syscall, 0)) < 0) + +void +sandbox(int stage) +{ + scmp_filter_ctx ctx; + + switch (stage) { + case SANDBOX_ENTER: + if ((ctx = seccomp_init(SCMP_ACT_TRAP)) == NULL) + err(1, "seccomp_init"); + + if (ALLOW(access) || + ALLOW(close) || + ALLOW(exit_group) || + ALLOW(fstat) || + ALLOW(fstat64) || + ALLOW(mmap) || + ALLOW(mmap2) || + ALLOW(munmap) || + ALLOW(open) || + ALLOW(poll) || + ALLOW(read) || + ALLOW(rt_sigaction) || + ALLOW(sigaction) || + ALLOW(sigreturn) || + ALLOW(stat) || + ALLOW(stat64) || + ALLOW(time) || + ALLOW(write) || + ALLOW_IOCTL(TCGETS, 1) || + ALLOW_IOCTL(TCSETS, 1) || + ALLOW_IOCTL(TIOCGWINSZ, 1)) + err(1, "seccomp_rule_add"); + + if (seccomp_load(ctx) < 0) + err(1, "seccomp_load"); + + seccomp_release(ctx); + break; + case SANDBOX_MAIN_LOOP_ENTER: + break; + case SANDBOX_MAIN_LOOP_EXIT: + break; + } +} + #else void diff --git a/config.h.in b/config.h.in index 66b3dbd6..e952d192 100644 --- a/config.h.in +++ b/config.h.in @@ -9,6 +9,9 @@ /* Define to 1 if you have the `reallocarray' function. */ #undef HAVE_REALLOCARRAY +/* Define if seccomp is available */ +#undef HAVE_SECCOMP + /* Define to 1 if you have the `strtonum' function. */ #undef HAVE_STRTONUM diff --git a/configure.ac b/configure.ac index 08f1ed22..c8dd904e 100644 --- a/configure.ac +++ b/configure.ac @@ -5,6 +5,8 @@ AC_CONFIG_HEADERS([config.h]) AC_PROG_CC AM_PROG_CC_C_O AC_CHECK_FUNCS([pledge reallocarray strtonum]) +AC_SEARCH_LIBS([seccomp_init], [seccomp], + [AC_DEFINE([HAVE_SECCOMP], [1], [Define if seccomp is available])]) AC_SEARCH_LIBS([setupterm], [curses], [], [ AC_SEARCH_LIBS([setupterm], [ncursesw], [AC_DEFINE([HAVE_NCURSESW_H], [1], [Define if ncursesw is available])], From d9ff2de1e3e5b1a506437c32dee58124d476638f Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 3 Jan 2018 18:10:15 +0100 Subject: [PATCH 3/7] Get rid of alignment --- compat-sandbox.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/compat-sandbox.c b/compat-sandbox.c index a182d222..749d4ce6 100644 --- a/compat-sandbox.c +++ b/compat-sandbox.c @@ -55,26 +55,26 @@ sandbox(int stage) if ((ctx = seccomp_init(SCMP_ACT_TRAP)) == NULL) err(1, "seccomp_init"); - if (ALLOW(access) || - ALLOW(close) || - ALLOW(exit_group) || - ALLOW(fstat) || - ALLOW(fstat64) || - ALLOW(mmap) || - ALLOW(mmap2) || - ALLOW(munmap) || - ALLOW(open) || - ALLOW(poll) || - ALLOW(read) || - ALLOW(rt_sigaction) || - ALLOW(sigaction) || - ALLOW(sigreturn) || - ALLOW(stat) || - ALLOW(stat64) || - ALLOW(time) || - ALLOW(write) || - ALLOW_IOCTL(TCGETS, 1) || - ALLOW_IOCTL(TCSETS, 1) || + if (ALLOW(access) || + ALLOW(close) || + ALLOW(exit_group) || + ALLOW(fstat) || + ALLOW(fstat64) || + ALLOW(mmap) || + ALLOW(mmap2) || + ALLOW(munmap) || + ALLOW(open) || + ALLOW(poll) || + ALLOW(read) || + ALLOW(rt_sigaction) || + ALLOW(sigaction) || + ALLOW(sigreturn) || + ALLOW(stat) || + ALLOW(stat64) || + ALLOW(time) || + ALLOW(write) || + ALLOW_IOCTL(TCGETS, 1) || + ALLOW_IOCTL(TCSETS, 1) || ALLOW_IOCTL(TIOCGWINSZ, 1)) err(1, "seccomp_rule_add"); From c01d0ca5519bb851d68e008d6a76008477797235 Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 3 Jan 2018 18:14:43 +0100 Subject: [PATCH 4/7] travis: install libseccomp-dev --- .travis.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.travis.yml b/.travis.yml index 1300ed97..c099792c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,10 @@ matrix: - os: linux compiler: clang env: BUILD_TYPE=default CFLAGS='-fsanitize=address -fsanitize=undefined' + addons: + apt: + packages: + - libseccomp-dev - os: linux compiler: clang-5.0 env: BUILD_TYPE=default CFLAGS='-fsanitize=address -fsanitize=undefined' @@ -18,9 +22,14 @@ matrix: - llvm-toolchain-trusty-5.0 packages: - clang-5.0 + - libseccomp-dev - os: linux compiler: gcc env: BUILD_TYPE=default CFLAGS=-fsanitize=address + addons: + apt: + packages: + - libseccomp-dev - os: osx compiler: clang env: BUILD_TYPE=default CFLAGS=-fsanitize=address From 3262498a6cd6445f305927acfc9dbdd327e8525f Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 3 Jan 2018 18:50:47 +0100 Subject: [PATCH 5/7] Add enable-seccomp-debug option to configure in order to aid debugging --- compat-sandbox.c | 35 +++++++++++++++++++++++++++++++++++ config.h.in | 3 +++ configure.ac | 4 ++++ 3 files changed, 42 insertions(+) diff --git a/compat-sandbox.c b/compat-sandbox.c index 749d4ce6..15431701 100644 --- a/compat-sandbox.c +++ b/compat-sandbox.c @@ -34,7 +34,10 @@ sandbox(int stage) #include #include +#include +#include #include +#include #include "compat.h" @@ -45,6 +48,34 @@ sandbox(int stage) (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(ioctl), x, \ SCMP_A1(SCMP_CMP_EQ, syscall, 0)) < 0) +/* + * Print out the offending syscall and exit. + * Not thread-safe and shall only be used for debugging purposes. + */ +void +handle_sigsys(int signum __attribute__((unused)), siginfo_t *info, + void *ctx __attribute__((unused))) +{ + errx(1, "disallowed syscall #%d", info->si_syscall); +} + +void +sandbox_sighandler(void) +{ + struct sigaction act; + sigset_t mask; + + memset(&act, 0, sizeof(act)); + sigemptyset(&mask); + sigaddset(&mask, SIGSYS); + act.sa_sigaction = &handle_sigsys; + act.sa_flags = SA_SIGINFO; + if (sigaction(SIGSYS, &act, NULL) == -1) + err(1, "sigaction"); + if (sigprocmask(SIG_UNBLOCK, &mask, NULL) == -1) + err(1, "sigprocmask"); +} + void sandbox(int stage) { @@ -52,6 +83,10 @@ sandbox(int stage) switch (stage) { case SANDBOX_ENTER: +#ifdef HAVE_SECCOMP_DEBUG + sandbox_sighandler(); +#endif + if ((ctx = seccomp_init(SCMP_ACT_TRAP)) == NULL) err(1, "seccomp_init"); diff --git a/config.h.in b/config.h.in index e952d192..c7c2560b 100644 --- a/config.h.in +++ b/config.h.in @@ -12,6 +12,9 @@ /* Define if seccomp is available */ #undef HAVE_SECCOMP +/* Define to debug seccomp */ +#undef HAVE_SECCOMP_DEBUG + /* Define to 1 if you have the `strtonum' function. */ #undef HAVE_STRTONUM diff --git a/configure.ac b/configure.ac index c8dd904e..a03bb628 100644 --- a/configure.ac +++ b/configure.ac @@ -4,6 +4,10 @@ AM_INIT_AUTOMAKE([subdir-objects]) AC_CONFIG_HEADERS([config.h]) AC_PROG_CC AM_PROG_CC_C_O +AC_ARG_ENABLE([seccomp-debug], + [AS_HELP_STRING([--enable-seccomp-debug], [enable seccomp debugging])], + [AC_DEFINE([HAVE_SECCOMP_DEBUG], [1], [Define to debug seccomp])], + []) AC_CHECK_FUNCS([pledge reallocarray strtonum]) AC_SEARCH_LIBS([seccomp_init], [seccomp], [AC_DEFINE([HAVE_SECCOMP], [1], [Define if seccomp is available])]) From 7ef624f7d4767ed2bb337e754b25846cb396b2d3 Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 3 Jan 2018 19:04:45 +0100 Subject: [PATCH 6/7] travis: pass enable-seccomp-debug to configure --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index c099792c..8929997b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,14 +8,14 @@ matrix: include: - os: linux compiler: clang - env: BUILD_TYPE=default CFLAGS='-fsanitize=address -fsanitize=undefined' + env: BUILD_TYPE=default CFLAGS='-fsanitize=address -fsanitize=undefined' CONFFLAGS=--enable-seccomp-debug addons: apt: packages: - libseccomp-dev - os: linux compiler: clang-5.0 - env: BUILD_TYPE=default CFLAGS='-fsanitize=address -fsanitize=undefined' + env: BUILD_TYPE=default CFLAGS='-fsanitize=address -fsanitize=undefined' CONFFLAGS=--enable-seccomp-debug addons: apt: sources: @@ -25,7 +25,7 @@ matrix: - libseccomp-dev - os: linux compiler: gcc - env: BUILD_TYPE=default CFLAGS=-fsanitize=address + env: BUILD_TYPE=default CFLAGS=-fsanitize=address CONFFLAGS=--enable-seccomp-debug addons: apt: packages: @@ -70,7 +70,7 @@ script: case $BUILD_TYPE in default) ./autogen.sh - ./configure || (cat config.log; exit 1) + ./configure ${CONFFLAGS} || (cat config.log; exit 1) make make distcheck || (find . -name test-suite.log | xargs -t cat; exit 1) ;; From 321e6e7b4def2c5c0a184b526cb546943aaff286 Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 3 Jan 2018 19:13:43 +0100 Subject: [PATCH 7/7] Debug --- compat-sandbox.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compat-sandbox.c b/compat-sandbox.c index 15431701..69a37cfc 100644 --- a/compat-sandbox.c +++ b/compat-sandbox.c @@ -38,6 +38,7 @@ sandbox(int stage) #include #include #include +#include #include "compat.h" @@ -56,7 +57,9 @@ void handle_sigsys(int signum __attribute__((unused)), siginfo_t *info, void *ctx __attribute__((unused))) { - errx(1, "disallowed syscall #%d", info->si_syscall); + warnx("disallowed syscall #%d", info->si_syscall); + fflush(stderr); + exit(1); } void