From dbff5520e049bd5a18bc5e26e3a41be6bf637a4a Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 21:35:48 -0500 Subject: Profile builder helper should use correct firejail binary path. --- src/common.mk.in | 6 +++++- src/fbuilder/build_profile.c | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/common.mk.in b/src/common.mk.in index ff66c6748..1464ab9b2 100644 --- a/src/common.mk.in +++ b/src/common.mk.in @@ -3,6 +3,7 @@ CC=@CC@ prefix=@prefix@ exec_prefix=@exec_prefix@ +bindir=@bindir@ libdir=@libdir@ sysconfdir=@sysconfdir@ @@ -29,7 +30,10 @@ C_FILE_LIST = $(sort $(wildcard *.c)) OBJS = $(C_FILE_LIST:.c=.o) BINOBJS = $(foreach file, $(OBJS), $file) -CFLAGS += -ggdb $(HAVE_FATAL_WARNINGS) -O2 -DVERSION='"$(VERSION)"' $(HAVE_GCOV) -DPREFIX='"$(prefix)"' -DSYSCONFDIR='"$(sysconfdir)/firejail"' -DLIBDIR='"$(libdir)"' $(HAVE_X11) $(HAVE_PRIVATE_HOME) $(HAVE_APPARMOR) $(HAVE_OVERLAYFS) $(HAVE_FIRETUNNEL) $(HAVE_SECCOMP) $(HAVE_GLOBALCFG) $(HAVE_SECCOMP_H) $(HAVE_CHROOT) $(HAVE_NETWORK) $(HAVE_USERNS) $(HAVE_FILE_TRANSFER) $(HAVE_WHITELIST) -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -pie -Wformat -Wformat-security +CFLAGS += -ggdb $(HAVE_FATAL_WARNINGS) -O2 -DVERSION='"$(VERSION)"' $(HAVE_GCOV) +CFLAGS += -DPREFIX='"$(prefix)"' -DSYSCONFDIR='"$(sysconfdir)/firejail"' -DLIBDIR='"$(libdir)"' -DBINDIR='"$(bindir)"' +CFLAGS += $(HAVE_X11) $(HAVE_PRIVATE_HOME) $(HAVE_APPARMOR) $(HAVE_OVERLAYFS) $(HAVE_FIRETUNNEL) $(HAVE_SECCOMP) $(HAVE_GLOBALCFG) $(HAVE_SECCOMP_H) $(HAVE_CHROOT) $(HAVE_NETWORK) $(HAVE_USERNS) $(HAVE_FILE_TRANSFER) $(HAVE_WHITELIST) +CFLAGS += -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -pie -Wformat -Wformat-security LDFLAGS += -pie -Wl,-z,relro -Wl,-z,now -lpthread EXTRA_LDFLAGS +=@EXTRA_LDFLAGS@ diff --git a/src/fbuilder/build_profile.c b/src/fbuilder/build_profile.c index f11e37057..83fe4b188 100644 --- a/src/fbuilder/build_profile.c +++ b/src/fbuilder/build_profile.c @@ -67,7 +67,7 @@ void build_profile(int argc, char **argv, int index, FILE *fp) { errExit("asprintf"); char *cmdlist[] = { - "/usr/bin/firejail", + BINDIR "/firejail", "--quiet", output, "--noprofile", -- cgit v1.2.3-70-g09d2 From 9af2c14723da9a2e0559d36d37b3d480f77eeb6e Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 21:37:46 -0500 Subject: Better debug handling. --- src/fbuilder/build_profile.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/fbuilder/build_profile.c b/src/fbuilder/build_profile.c index 83fe4b188..16425dd48 100644 --- a/src/fbuilder/build_profile.c +++ b/src/fbuilder/build_profile.c @@ -110,7 +110,7 @@ void build_profile(int argc, char **argv, int index, FILE *fp) { if (arg_debug) { for (i = 0; i < len; i++) - printf("\t%s\n", cmd[i]); + printf("%s%s\n", (i)?"\t":"", cmd[i]); } // fork and execute @@ -130,7 +130,8 @@ void build_profile(int argc, char **argv, int index, FILE *fp) { errExit("waitpid"); if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { - printf("\n\n\n"); + if (fp == stdout) + printf("--- Built profile beings after this line ---\n"); fprintf(fp, "############################################\n"); fprintf(fp, "# %s profile\n", argv[index]); fprintf(fp, "############################################\n"); @@ -177,9 +178,10 @@ void build_profile(int argc, char **argv, int index, FILE *fp) { fprintf(fp, "### environment\n"); fprintf(fp, "shell none\n"); - unlink(trace_output); - unlink(strace_output); - + if (!arg_debug) { + unlink(trace_output); + unlink(strace_output); + } } else { fprintf(stderr, "Error: cannot run the sandbox\n"); -- cgit v1.2.3-70-g09d2 From 6620aac849bb6d95eece058bfca53d025721c862 Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 21:40:16 -0500 Subject: Fix issue where strace output file path has leading space making it an invalid path. --- src/fbuilder/build_profile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/fbuilder/build_profile.c b/src/fbuilder/build_profile.c index 16425dd48..5199829d7 100644 --- a/src/fbuilder/build_profile.c +++ b/src/fbuilder/build_profile.c @@ -63,7 +63,7 @@ void build_profile(int argc, char **argv, int index, FILE *fp) { char *stroutput; if(asprintf(&output,"--output=%s",trace_output) == -1) errExit("asprintf"); - if(asprintf(&stroutput,"-o %s",strace_output) == -1) + if(asprintf(&stroutput,"-o%s",strace_output) == -1) errExit("asprintf"); char *cmdlist[] = { -- cgit v1.2.3-70-g09d2 From 1b02467adfef544c17d1e1606c0167777100c328 Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 21:53:46 -0500 Subject: Allow libtrace preload library to use for trace output a logfile specified by the environment variable FIREJAIL_TRACEFILE or as the RUN_TRACE_FILE if it exists ortherwise use the console as before. --- src/include/rundefs.h | 1 + src/libtrace/libtrace.c | 27 +++++++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/include/rundefs.h b/src/include/rundefs.h index 6cc931faf..df135b9ca 100644 --- a/src/include/rundefs.h +++ b/src/include/rundefs.h @@ -95,6 +95,7 @@ #define RUN_PASSWD_FILE RUN_MNT_DIR "/passwd" #define RUN_GROUP_FILE RUN_MNT_DIR "/group" #define RUN_FSLOGGER_FILE RUN_MNT_DIR "/fslogger" +#define RUN_TRACE_FILE RUN_MNT_DIR "/trace" #define RUN_UMASK_FILE RUN_MNT_DIR "/umask" #define RUN_OVERLAY_ROOT RUN_MNT_DIR "/oroot" #define RUN_READY_FOR_JOIN RUN_MNT_DIR "/ready-for-join" diff --git a/src/libtrace/libtrace.c b/src/libtrace/libtrace.c index 745dd2260..b3f040e8f 100644 --- a/src/libtrace/libtrace.c +++ b/src/libtrace/libtrace.c @@ -32,7 +32,7 @@ #include #include #include -#include +#include "../include/rundefs.h" #define tprintf(fp, args...) \ do { \ @@ -46,6 +46,8 @@ typedef FILE *(*orig_fopen_t)(const char *pathname, const char *mode); static orig_fopen_t orig_fopen = NULL; typedef FILE *(*orig_fopen64_t)(const char *pathname, const char *mode); static orig_fopen64_t orig_fopen64 = NULL; +typedef int (*orig_access_t)(const char *pathname, int mode); +static orig_access_t orig_access = NULL; // // library constructor/destructor @@ -65,14 +67,24 @@ void init(void) { return; orig_fopen = (orig_fopen_t)dlsym(RTLD_NEXT, "fopen"); - - // tty + orig_access = (orig_access_t)dlsym(RTLD_NEXT, "access"); + + // allow environment variable to override defaults + char *logfile = getenv("FIREJAIL_TRACEFILE"); + if (!logfile) { + // if exists, log to trace file + logfile = RUN_TRACE_FILE; + if (orig_access(logfile, F_OK)) #ifdef PRINTF_DEVTTY - ftty = orig_fopen("/dev/tty", "w"); + // else log to associated tty + logfile = "/dev/tty"; #else - ftty = stderr; + logfile = "/proc/self/fd/2"; #endif - tprintf(ftty, "=== tracelib init() === \n"); + } + + // logfile + ftty = orig_fopen(logfile, "a"); // pid mypid = getpid(); @@ -92,6 +104,7 @@ void init(void) { if (ptr) *ptr = '\0'; + tprintf(ftty, "=== tracelib init() [%d:%s] === \n", mypid, myname); fclose(fp); free(fname); } @@ -483,8 +496,6 @@ DIR *opendir(const char *pathname) { } // access -typedef int (*orig_access_t)(const char *pathname, int mode); -static orig_access_t orig_access = NULL; int access(const char *pathname, int mode) { if (!orig_access) orig_access = (orig_access_t)dlsym(RTLD_NEXT, "access"); -- cgit v1.2.3-70-g09d2 From f6584eaf3b5bfa166fed56b900dbda6629c4fbd3 Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 21:57:13 -0500 Subject: Allow firejail --trace option to take an optional parameter which is the trace log file path. The trace log file will be created if it does not exist and then bind mounted to RUN_TRACE_FILE so that the sandboxed program can access it. --- src/firejail/firejail.h | 1 + src/firejail/fs_trace.c | 19 +++++++++++++++++++ src/firejail/main.c | 5 +++++ 3 files changed, 25 insertions(+) (limited to 'src') diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 14cad4190..4a59522bf 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -260,6 +260,7 @@ extern int arg_caps_keep; // keep list extern char *arg_caps_list; // optional caps list extern int arg_trace; // syscall tracing support +extern char *arg_tracefile; // syscall tracing file extern int arg_tracelog; // blacklist tracing support extern int arg_rlimit_cpu; // rlimit cpu extern int arg_rlimit_nofile; // rlimit nofile diff --git a/src/firejail/fs_trace.c b/src/firejail/fs_trace.c index 26dd5cb27..eac73a074 100644 --- a/src/firejail/fs_trace.c +++ b/src/firejail/fs_trace.c @@ -41,6 +41,25 @@ void fs_trace_preload(void) { fclose(fp); fs_logger("touch /etc/ld.so.preload"); } + if (arg_tracefile) { + if (arg_debug) + printf("Creating an empty trace log file: %s\n", arg_tracefile); + // create a bind mounted trace logfile that the sandbox can see + FILE *fp = fopen(arg_tracefile, "w"); + if (!fp) + errExit("fopen"); + SET_PERMS_STREAM(fp, firejail_uid, firejail_gid, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH); + fclose(fp); + fp = fopen(RUN_TRACE_FILE, "w"); + if (!fp) + errExit("fopen"); + fclose(fp); + fs_logger2("touch ", arg_tracefile); + if (mount(arg_tracefile, RUN_TRACE_FILE, NULL, MS_BIND|MS_REC, NULL) < 0) + errExit("mount bind " RUN_TRACE_FILE); + if (arg_debug) + printf("Bind mount %s to %s\n", arg_tracefile, RUN_TRACE_FILE); + } } void fs_trace(void) { diff --git a/src/firejail/main.c b/src/firejail/main.c index 9f44c6281..4c6d20626 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -80,6 +80,7 @@ int arg_caps_keep = 0; // keep list char *arg_caps_list = NULL; // optional caps list int arg_trace = 0; // syscall tracing support +char *arg_tracefile = NULL; // syscall tracing file int arg_tracelog = 0; // blacklist tracing support int arg_rlimit_cpu = 0; // rlimit max cpu time int arg_rlimit_nofile = 0; // rlimit nofile @@ -1296,6 +1297,10 @@ int main(int argc, char **argv) { } else if (strcmp(argv[i], "--trace") == 0) arg_trace = 1; + else if (strncmp(argv[i], "--trace=", 8) == 0) { + arg_trace = 1; + arg_tracefile = argv[i] + 8; + } else if (strcmp(argv[i], "--tracelog") == 0) arg_tracelog = 1; else if (strncmp(argv[i], "--rlimit-cpu=", 13) == 0) { -- cgit v1.2.3-70-g09d2 From 96505fd6765a124016cc7e64ea8191f38efb09a5 Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 22:02:08 -0500 Subject: Update man page to note that --trace can now take an optional parameter. --- src/man/firejail.txt | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/man/firejail.txt b/src/man/firejail.txt index 500850413..9f9d8e6ec 100644 --- a/src/man/firejail.txt +++ b/src/man/firejail.txt @@ -71,10 +71,10 @@ If an appropriate profile is not found, Firejail will use a default profile. The default profile is quite restrictive. In case the application doesn't work, use --noprofile option to disable it. For more information, please see \fBSECURITY PROFILES\fR section below. .PP -If a program argument is not specified, Firejail starts the default shell from the current user. +If a program argument is not specified, Firejail starts /bin/bash shell. Examples: .PP -$ firejail [OPTIONS] # starting the user default shell (normally /bin/bash) +$ firejail [OPTIONS] # starting a /bin/bash shell .PP $ firejail [OPTIONS] firefox # starting Mozilla Firefox .PP @@ -1776,14 +1776,11 @@ vm86, vm86old, vmsplice and vserver. .br To help creating useful seccomp filters more easily, the following -system call groups are defined: @aio, @basic-io, @chown, @clock, -@cpu-emulation, @debug, @default, @default-nodebuggers, @default-keep, -@file-system, @io-event, @ipc, @keyring, @memlock, @module, @mount, -@network-io, @obsolete, @privileged, @process, @raw-io, @reboot, -@resources, @setuid, @swap, @sync, @system-service and @timer. In addition, a +system call groups are defined: @clock, @cpu-emulation, @debug, +@default, @default-nodebuggers, @default-keep, @module, @obsolete, +@privileged, @raw-io, @reboot, @resources and @swap. In addition, a system call can be specified by its number instead of name with prefix -$, so for example $165 would be equal to mount on i386. Exceptions -can be allowed with prefix !. +$, so for example $165 would be equal to mount on i386. .br System architecture is strictly imposed only if flag @@ -1801,10 +1798,8 @@ Example: .br $ firejail \-\-seccomp .TP -\fB\-\-seccomp=syscall,@group,!syscall2 -Enable seccomp filter, whitelist "syscall2", but blacklist the default -list (@default) and the syscalls or syscall groups specified by the -command. +\fB\-\-seccomp=syscall,@group +Enable seccomp filter, blacklist the default list (@default) and the syscalls or syscall groups specified by the command. .br .br @@ -1868,9 +1863,8 @@ domain with personality(2) system call. .br .TP -\fB\-\-seccomp.drop=syscall,@group,!syscall2 -Enable seccomp filter, whitelist "syscall2" but blacklist the -syscalls or the syscall groups specified by the command. +\fB\-\-seccomp.drop=syscall,@group +Enable seccomp filter, and blacklist the syscalls or the syscall groups specified by the command. .br .br @@ -1905,11 +1899,10 @@ rm: cannot remove `testfile': Operation not permitted .TP -\fB\-\-seccomp.keep=syscall,@group,!syscall2 -Enable seccomp filter, blacklist "syscall2" but whitelist the -syscalls or the syscall groups specified by the command. The system -calls needed by Firejail (group @default-keep: prctl, execve) are -handled with the preload library. +\fB\-\-seccomp.keep=syscall,syscall,syscall +Enable seccomp filter, and whitelist the syscalls specified by the +command. The system calls needed by Firejail (group @default-keep: +prctl, execve) are handled with the preload library. .br .br @@ -2149,8 +2142,9 @@ Example: .br $ firejail \-\-top .TP -\fB\-\-trace -Trace open, access and connect system calls. +\fB\-\-trace[=filename] +Trace open, access and connect system calls. If filename is specified, log +trace output to filename, otherwise log to console. .br .br -- cgit v1.2.3-70-g09d2 From 02580c890f28766ffef0b9aa8d4f17fd7b8ab905 Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 22:08:25 -0500 Subject: When running builder trace output should go to separate file because (1) trace output is logged to console, which is a pain to capture, and (2) it should not be mingled with program output anyway, which it was when sending to stdout. --- src/fbuilder/build_profile.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/fbuilder/build_profile.c b/src/fbuilder/build_profile.c index 5199829d7..a0f71ae03 100644 --- a/src/fbuilder/build_profile.c +++ b/src/fbuilder/build_profile.c @@ -61,7 +61,7 @@ void build_profile(int argc, char **argv, int index, FILE *fp) { char *output; char *stroutput; - if(asprintf(&output,"--output=%s",trace_output) == -1) + if(asprintf(&output,"--trace=%s",trace_output) == -1) errExit("asprintf"); if(asprintf(&stroutput,"-o%s",strace_output) == -1) errExit("asprintf"); @@ -69,11 +69,10 @@ void build_profile(int argc, char **argv, int index, FILE *fp) { char *cmdlist[] = { BINDIR "/firejail", "--quiet", - output, "--noprofile", "--caps.drop=all", "--nonewprivs", - "--trace", + output, "--shell=none", "/usr/bin/strace", // also used as a marker in build_profile() "-c", -- cgit v1.2.3-70-g09d2 From 742d2a26ca5281b9d1b161011d92164a4f3dc90e Mon Sep 17 00:00:00 2001 From: Glenn Washburn Date: Thu, 29 Aug 2019 22:42:05 -0500 Subject: Make sure that we are unprivileged before creating the trace log file. --- src/firejail/fs_trace.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/firejail/fs_trace.c b/src/firejail/fs_trace.c index eac73a074..2a7c83049 100644 --- a/src/firejail/fs_trace.c +++ b/src/firejail/fs_trace.c @@ -45,14 +45,16 @@ void fs_trace_preload(void) { if (arg_debug) printf("Creating an empty trace log file: %s\n", arg_tracefile); // create a bind mounted trace logfile that the sandbox can see + EUID_USER(); FILE *fp = fopen(arg_tracefile, "w"); if (!fp) errExit("fopen"); SET_PERMS_STREAM(fp, firejail_uid, firejail_gid, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH); fclose(fp); + EUID_ROOT(); fp = fopen(RUN_TRACE_FILE, "w"); if (!fp) - errExit("fopen"); + errExit("fopen " RUN_TRACE_FILE); fclose(fp); fs_logger2("touch ", arg_tracefile); if (mount(arg_tracefile, RUN_TRACE_FILE, NULL, MS_BIND|MS_REC, NULL) < 0) -- cgit v1.2.3-70-g09d2