diff --git a/Documentation/en/changes.tely b/Documentation/en/changes.tely index 3593da6cbbaceb0e660b55f109273304f6d778e3..9e42be6f9669d4df2ac00ac982eee6b7fc209111 100644 --- a/Documentation/en/changes.tely +++ b/Documentation/en/changes.tely @@ -65,6 +65,9 @@ which scares away people. @end ignore +@item +A new @code{-dsafer} mode is available for robust sandboxing +against malicious @file{.ly} files @item A new grob @code{DurationLine} is now available. diff --git a/Documentation/en/usage/running.itely b/Documentation/en/usage/running.itely index 1c310cff588d88e99b25f772b7a4eb4dc36ae038..1af21fd3244f1976c3959800e52a949e74f15d12 100644 --- a/Documentation/en/usage/running.itely +++ b/Documentation/en/usage/running.itely @@ -853,6 +853,29 @@ Default: @code{#t}. Set resolution for generating @code{PNG} pixmaps to @var{num} dpi. Default: @code{101}. +@item @code{safer} @var{bool} +Runs lilypond in a seccomp/ptrace based sandbox. This is a very +robust security mechanism, but is only available on Linux. If a +file tries to do anything naughty, its execution will be halted. + +@example +$ cat naughty.ly +#(system "id") +$ lilypond -dsafer naughty.ly +GNU LilyPond 2.21.5 +Processing `naughty.ly' +Parsing...killing child due to prohibited syscall 13 +@end example + +This option is strongly recommended over @code{-dsafe} and +@code{-j}. + +When using this option, it is recommended to link LilyPond against +Ghostscript using the @code{--enable-gs-api} configure option, and +produce PDF, as this also protects against malicious postscript +code. + + @item @code{safe} @var{bool} If @var{bool} is @code{#t}, do not trust the @file{.ly} input. Default: @code{#f}. diff --git a/config.hh.in b/config.hh.in index c0d25fb319d07fb632786ada9905efbb4dc0924d..6179fe35c32a6c98009098536faf0e569ae396bd 100644 --- a/config.hh.in +++ b/config.hh.in @@ -42,6 +42,12 @@ /* define if you have sys/stat.h */ #define STAT_MACROS_BROKEN 0 +/* define if you have ptrace_syscall_info */ +#define HAVE_STRUCT_PTRACE_SYSCALL_INFO 0 + +/* define if you have libseccomp */ +#define HAVE_SECCOMP 0 + /* define if you have fontconfig */ #define HAVE_FONTCONFIG 0 diff --git a/configure.ac b/configure.ac index e4b80573af841972a83e9564b198e2383a0ffd80..d205573af7c227ce49b2722fdf100600a7da806b 100644 --- a/configure.ac +++ b/configure.ac @@ -269,6 +269,7 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])], [AC_MSG_RESULT([no]) CXXFLAGS="$save_CXXFLAGS"]) + ## Check for usable cxxabi save_LIBS="$LIBS" LIBS="$LIBS $CXXABI_LIBS" @@ -287,6 +288,12 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include ], LIBS="$save_LIBS" AC_SUBST(CXXABI_LIBS) +AC_CHECK_TYPES([struct ptrace_syscall_info], [], [], [[#include ]]) +PKG_CHECK_MODULES(SECCOMP, libseccomp, AC_DEFINE(HAVE_SECCOMP), + STEPMAKE_ADD_ENTRY(OPTIONAL, libseccomp)); +CPPFLAGS="${CPPFLAGS} ${SECCOMP_CFLAGS}" +LIBS="${LIBS} ${SECCOMP_LIBS}" + STEPMAKE_PATH_PROG(FONTFORGE, fontforge, REQUIRED, 20110222) STEPMAKE_PATH_PROG(T1ASM, t1asm, REQUIRED) diff --git a/lily/sandbox.cc b/lily/sandbox.cc new file mode 100644 index 0000000000000000000000000000000000000000..757a6508643cc7e10245214ff5f72c511d6a3203 --- /dev/null +++ b/lily/sandbox.cc @@ -0,0 +1,386 @@ +#include "config.hh" +#include "lily-guile.hh" +#include "warn.hh" + +#if HAVE_SECCOMP +#include + +#include + +#include +#include +#include +#include +#include + +#include // has to be after sys/ptrace.h + +using std::string; + +static void process_signal_loop (pid_t child); +static bool handle_syscall (pid_t child); + +#define IS_SECCOMP_EVENT(status) ((status >> 16) == PTRACE_EVENT_SECCOMP) + +// The sandbox works by forking the process in two. The child sets up +// a list of allowed syscalls and then continues processing. The +// parent starts tracing the child, and is notified of any other +// syscall. The parent uses this to selectively allow file writes; +// all other syscalls are rejected +// +// See +// https://www.alfonsobeato.net/c/filter-and-modify-system-calls-with-seccomp-and-ptrace/ +// for more background + +bool +setup_bpf (void) +{ + // We want to be traced. + if (ptrace ((__ptrace_request) PTRACE_TRACEME, 0, 0, 0)) + { + perror ("PTRACE_TRACEME"); + // continue - in case we run under strace already + } + + scmp_filter_ctx ctx = seccomp_init (SCMP_ACT_TRACE (0)); + if (ctx == NULL) + { + perror ("seccomp_init"); + return false; + } + + // Memory allocation. + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (mmap), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (munmap), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (mremap), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (brk), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (brk), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (mmap), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (mprotect), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (mremap), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (munmap), 0); + + // process mgmt + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (futex), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (getpid), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (fcntl), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (times), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (clock_gettime), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (kill), + 0); // need to setup tracing + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (sysinfo), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (getcwd), 0); + + // I/O + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (read), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (write), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (readlink), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (getdents), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (getdents64), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (select), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (fstat), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (stat), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (lseek), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (read), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (write), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (close), 0); + + // Clean exit + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (exit), 0); + seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS (exit_group), 0); + + int err = seccomp_load (ctx); + if (err < 0) + { + perror ("Failed to install syscall filter"); + return false; + } + + seccomp_release (ctx); + + return true; +} + +LY_DEFINE (ly_start_sandbox, "ly:start-sandbox", 0, 0, 0, (), + "Start seccomp sandbox") +{ + pid_t pid = fork (); + if (pid == 0) + { + if (!setup_bpf ()) + { + printf ("setup_bpf failed\n"); + exit (2); + } + + // Stop to let parent install tracing. + kill (getpid (), SIGSTOP); + } + else + { + int status; + + // Make sure child is in STOP state. + waitpid (pid, &status, 0); + long err = ptrace ((__ptrace_request) PTRACE_SETOPTIONS, pid, 0, + PTRACE_O_TRACESECCOMP); + if (err) + { + perror ("PTRACE_O_TRACESECCOMP"); + } + + process_signal_loop (pid); + } + return SCM_UNSPECIFIED; +} + +static void +process_signal_loop (pid_t child) +{ + while (1) + { + ptrace ((__ptrace_request) PTRACE_CONT, child, 0, 0); + int status; + waitpid (child, &status, 0); + + if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP + && IS_SECCOMP_EVENT (status)) + { + if (!handle_syscall (child)) + { + kill (child, SIGKILL); + exit (2); + } + } + else if (WIFEXITED (status)) + { + exit (WEXITSTATUS (status)); + } + else if (WIFSIGNALED (status)) + { + raise (WTERMSIG (status)); + } + else + { + printf ("unknown ptrace event %08x\n", status); + } + } +} + +bool +read_string0 (pid_t child, uint64_t arg, string *dest) +{ + char file[PATH_MAX] = {}; + char *f = file; + char *end = file + PATH_MAX - 1; + while (1) + { + long val = ptrace ((__ptrace_request) PTRACE_PEEKTEXT, child, arg, NULL); + if (val == -1) + { + fprintf (stderr, "PTRACE_PEEKTEXT error: %s\n", strerror (errno)); + return false; + } + + arg += sizeof (long); + char *p = (char *) &val; + size_t i = 0; + for (; i < sizeof (long) && f < end; ++i, ++f) + { + *f = *p++; + if (*f == '\0') + break; + } + if (i < sizeof (long)) + break; + } + *dest = string (file); + return true; +} + +bool +has_prefix (const std::string &str, const std::string &prefix) +{ + return str.find (prefix, 0) == 0; +} + +bool +has_suffix (const string &s, const string &suffix) +{ + + return s.size () >= suffix.size () + && s.rfind (suffix) == s.size () - suffix.size (); +} + +static const char * +syscall_name (uint64_t no) +{ + switch (no) + { + case __NR_openat: + return "openat"; + case __NR_open: + return "open"; + case __NR_rename: + return "rename"; + case __NR_unlink: + return "unlink"; + default: + return "unknown"; + } +} + +bool +validate_file_write (uint64_t scno, const string &fn) +{ + if (has_prefix (fn, "/tmp")) + { + // Should really check suffix too + return true; + } + + const char *suffixes[] = { + ".pdf", + ".ps", + ".png", + ".jpg" + ".jpeg", + ".texidoc", + ".texi", + NULL, + }; + for (const char **p = suffixes; *p; p++) + { + // TODO: only allow in cwd and $TMP + if (has_suffix (fn, *p)) + return true; + } + printf ("prohibited file access: syscall %s, file %s\n", syscall_name (scno), + fn.c_str ()); + return false; +} + +struct syscall_info +{ + uint64_t scno; + uint64_t args[6]; +}; + +bool +get_syscall_info (pid_t child, syscall_info *info) +{ + long err = -1; + errno = ENOSYS; + +#if HAVE_STRUCT_PTRACE_SYSCALL_INFO + struct ptrace_syscall_info psi = {}; + err = ptrace ((__ptrace_request) PTRACE_GET_SYSCALL_INFO, child, sizeof (psi), + &psi); + if (err >= 0) + { + info->scno = psi.seccomp.nr; + for (int i = 0; i < 6; i++) + info->args[i] = psi.seccomp.args[i]; + + return true; + } +#endif + +#ifdef __x86_64__ + if (err < 0 && errno == ENOSYS) + { + info->scno = ptrace (PTRACE_PEEKUSER, child, sizeof (long) * ORIG_RAX, 0); + int regs[] = {REG_RDI, RSI, RDX, R10, R8, R9}; + for (int i = 0; i < 6; i++) + { + long val + = ptrace (PTRACE_PEEKUSER, child, sizeof (long) * regs[i], 0); + if (val == -1) + { + perror ("reading register"); + return false; + } + + info->args[i] = (uint64_t) val; + } + + return true; + } +#endif + + return false; +} + +static bool +handle_syscall (pid_t child) +{ + syscall_info info = {}; + if (!get_syscall_info (child, &info)) + { + return false; + } + + int file_arg = -1; + uint64_t open_flags = 0; + switch (info.scno) + { + case __NR_openat: + open_flags = info.args[2]; + file_arg = 1; + break; + case __NR_open: + open_flags = info.args[1]; + file_arg = 0; + break; + case __NR_unlink: + file_arg = 0; + break; + case __NR_rename: + file_arg = 1; + break; + + default: + printf ("killing process due to prohibited syscall %ld\n", info.scno); + return false; + } + + if (!(open_flags & (O_CREAT | O_APPEND | O_WRONLY | O_RDWR))) + { + // allow all reads. + return true; + } + + string filename; + if (!read_string0 (child, info.args[file_arg], &filename)) + { + return false; + } + + if (!validate_file_write (info.scno, filename)) + { + return false; + } + + if (info.scno == __NR_rename) + { + string src; + if (!read_string0 (child, info.args[0], &src)) + return false; + if (!validate_file_write (info.scno, src)) + { + return false; + } + } + + return true; +} +#else + +LY_DEFINE (ly_start_sandbox, "ly:start-sandbox", 0, 0, 0, (), + "Start seccomp sandbox") +{ + error ("ly:start-sandbox: seccomp sandboxing needs linux"); + exit (2); +} + +#endif // __linux__ diff --git a/scm/backend-library.scm b/scm/backend-library.scm index 663b0f8412c643f2c5b7ce7a00a17842880e0e62..75fd418a35bde78a1fbdfcce4b60b82385046c9f 100644 --- a/scm/backend-library.scm +++ b/scm/backend-library.scm @@ -95,7 +95,7 @@ (define-public (postscript->pdf paper-width paper-height base-name tmp-name is-eps) (let* ((pdf-name (ly:format "~a.~a.pdf" tmp-name (random 1000000))) - (flush-name (string-append pdf-name ".flush")) + (flush-name (string-append pdf-name ".flush.pdf")) (dest (string-append base-name ".pdf")) (output-file (string-join (string-split pdf-name #\%) "%%")) (run-strings @@ -250,13 +250,21 @@ ;; TODO: this should write destination atomically. (copy-file from-name to-name))) -(define-public (make-tmpfile basename) +(define-public (make-tmpfile tmp-basename) "Returns a temp file as port. If basename is #f, a file under $TMPDIR is created." (define max-try 10) - (define (inner basename tries) + (define (ext fn) + (let* + ((idx (string-rindex fn #\.))) + (if idx + (substring fn idx) + ""))) + + (define (inner tmp-basename tries) (if (> tries 0) (let* - ((name (ly:format "~a-tmp-~a" basename (random 10000000))) + ((extension (ext (basename tmp-basename))) + (name (ly:format "~a-tmp-~a~a" tmp-basename (random 10000000) extension)) (port (create-file-exclusive name #o666)) (bport #f)) @@ -266,13 +274,13 @@ (close-port port) bport) - (make-tmpfile basename (1- tries)))) + (make-tmpfile tmp-basename (1- tries)))) - (ly:error "can't create temp file for ~a after ~a times" basename max-try) + (ly:error "can't create temp file for ~a after ~a times" tmp-basename max-try) )) - (if (not basename) - (set! basename (cond + (if (not tmp-basename) + (set! tmp-basename (cond ;; MINGW hack: TMP / TEMP may include ;; unusable characters (Unicode etc.). ((eq? PLATFORM 'windows) "./tmp-") @@ -289,7 +297,7 @@ "/tmp") "/lilypond"))))) - (inner basename max-try)) + (inner tmp-basename max-try)) (define-public (postprocess-output paper-book module formats diff --git a/scm/framework-ps.scm b/scm/framework-ps.scm index ef30eacc9e06a0181e5784281a5abe4ee7995528..5e6b3d7fcd6012cb56c2628925197a681adf467e 100644 --- a/scm/framework-ps.scm +++ b/scm/framework-ps.scm @@ -735,7 +735,7 @@ mark {ly~a_stream} /CLOSE pdfmark sorted-page-numbers))) (define-public (output-framework basename book scopes fields) - (let* ((port (make-tmpfile basename)) + (let* ((port (make-tmpfile (string-append basename ".ps"))) (tmp-name (port-filename port)) (outputter (ly:make-paper-outputter port 'ps)) (paper (ly:paper-book-paper book)) diff --git a/scm/lily.scm b/scm/lily.scm index 53224d54808f422ea43003ead2fc6455c82ea78c..b9fc9d10a78445fe7492b60541016d98fe8d9b0c 100644 --- a/scm/lily.scm +++ b/scm/lily.scm @@ -371,8 +371,9 @@ the included file relative to the current file\ (resolution 101 "Set resolution for generating PNG pixmaps to given value (in dpi).") - (safe #f - "Run in safer mode.") + (safer #f + "Run in sandboxed mode; only supported on Linux.") + (safe #f "Deprecated - do not use.") (separate-log-files #f "For input files `FILE1.ly', `FILE2.ly', ... output log data to files `FILE1.log', @@ -994,6 +995,9 @@ PIDs or the number of the process." (if (string-or-symbol? (ly:get-option 'log-file)) (ly:stderr-redirect (format #f "~a.log" (ly:get-option 'log-file)) "w")) + + (if (ly:get-option 'safer) + (ly:start-sandbox)) (let ((failed (lilypond-all files))) (if (pair? failed) (begin (ly:error (_ "failed files: ~S") (string-join failed))