non-dumpable plugins

(hopefully) fixes the issues that led to reverting commits 6abb65d and 98e42dc
netblue30 · Jan 4, 2021 · 18e806c · 18e806c
1 parent 6ca31ec
commit 18e806c
Show file tree

Hide file tree

Showing 23 changed files with 63 additions and 80 deletions.
diff --git a/Makefile.in b/Makefile.in
@@ -110,9 +110,9 @@ endif
  install -m 0755 -d $(DESTDIR)$(libdir)/firejail
  install -m 0644 -t $(DESTDIR)$(libdir)/firejail $(MYLIBS) $(SECCOMP_FILTERS) src/firecfg/firecfg.config
  install -m 0755 -t $(DESTDIR)$(libdir)/firejail $(SBOX_APPS)
- # non-dumpable plugins
- install -m 0755 -t $(DESTDIR)$(libdir)/firejail $(SBOX_APPS_NON_DUMPABLE)
- install -m 0755 -t $(DESTDIR)$(libdir)/firejail src/fshaper/fshaper.sh
+ # plugins w/o read permission (non-dumpable)
+ install -m 0711 -t $(DESTDIR)$(libdir)/firejail $(SBOX_APPS_NON_DUMPABLE)
+ install -m 0711 -t $(DESTDIR)$(libdir)/firejail src/fshaper/fshaper.sh
 ifeq ($(HAVE_CONTRIB_INSTALL),yes)
  # contrib scripts
  install -m 0755 -t $(DESTDIR)$(libdir)/firejail contrib/*.py contrib/*.sh

diff --git a/src/fcopy/Makefile.in b/src/fcopy/Makefile.in
@@ -5,8 +5,8 @@ include ../common.mk
 %.o : %.c $(H_FILE_LIST) ../include/common.h ../include/syscall.h
  $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(INCLUDE) -c $< -o $@
 
-fcopy: $(OBJS)
- $(CC) $(LDFLAGS) -o $@ $(OBJS) $(LIBS) $(EXTRA_LDFLAGS)
+fcopy: $(OBJS) ../lib/common.o
+ $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/common.o $(LIBS) $(EXTRA_LDFLAGS)
 
 clean:; rm -fr *.o fcopy *.gcov *.gcda *.gcno *.plist
 

diff --git a/src/fcopy/main.c b/src/fcopy/main.c
@@ -23,7 +23,6 @@
 #include <ftw.h>
 #include <errno.h>
 #include <pwd.h>
-#include <sys/prctl.h>
 
 #if HAVE_SELINUX
 #include <sys/stat.h>
@@ -412,10 +411,7 @@ int main(int argc, char **argv) {
  exit(1);
  }
 
-#ifdef WARN_DUMPABLE
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid())
- fprintf(stderr, "Error fcopy: I am dumpable\n");
-#endif
+ warn_dumpable();
 
  // trim trailing chars
  if (src[strlen(src) - 1] == '/')

diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
@@ -513,7 +513,6 @@ void check_private_dir(void);
 void update_map(char *mapping, char *map_file);
 void wait_for_other(int fd);
 void notify_other(int fd);
-const char *gnu_basename(const char *path);
 uid_t pid_get_uid(pid_t pid);
 uid_t get_group_id(const char *group);
 int remove_overlay_directory(void);

diff --git a/src/firejail/main.c b/src/firejail/main.c
@@ -1231,11 +1231,6 @@ int main(int argc, char **argv, char **envp) {
  }
  EUID_ASSERT();
 
-#ifdef WARN_DUMPABLE
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid())
- fprintf(stderr, "Error: Firejail is dumpable\n");
-#endif
-
  // check for force-nonewprivs in /etc/firejail/firejail.config file
  if (checkcfg(CFG_FORCE_NONEWPRIVS))
  arg_nonewprivs = 1;

diff --git a/src/firejail/util.c b/src/firejail/util.c
@@ -820,20 +820,6 @@ void notify_other(int fd) {
  fclose(stream);
 }
 
-
-
-
-// Equivalent to the GNU version of basename, which is incompatible with
-// the POSIX basename. A few lines of code saves any portability pain.
-// https://www.gnu.org/software/libc/manual/html_node/Finding-Tokens-in-a-String.html#index-basename
-const char *gnu_basename(const char *path) {
- const char *last_slash = strrchr(path, '/');
- if (!last_slash)
- return path;
- return last_slash+1;
-}
-
-
 uid_t pid_get_uid(pid_t pid) {
  EUID_ASSERT();
  uid_t rv = 0;

diff --git a/src/fldd/Makefile.in b/src/fldd/Makefile.in
@@ -5,8 +5,8 @@ include ../common.mk
 %.o : %.c $(H_FILE_LIST) ../include/common.h ../include/syscall.h ../include/ldd_utils.h
  $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(INCLUDE) -c $< -o $@
 
-fldd: $(OBJS) ../lib/ldd_utils.o
- $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/ldd_utils.o $(LIBS) $(EXTRA_LDFLAGS)
+fldd: $(OBJS) ../lib/common.o ../lib/ldd_utils.o
+ $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/common.o ../lib/ldd_utils.o $(LIBS) $(EXTRA_LDFLAGS)
 
 clean:; rm -fr *.o fldd *.gcov *.gcda *.gcno *.plist
 

diff --git a/src/fldd/main.c b/src/fldd/main.c
@@ -24,7 +24,6 @@
 #include <fcntl.h>
 #include <sys/mman.h>
 #include <sys/mount.h>
-#include <sys/prctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -303,10 +302,7 @@ printf("\n");
  return 0;
  }
 
-#ifdef WARN_DUMPABLE
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid())
- fprintf(stderr, "Error fldd: I am dumpable\n");
-#endif
+ warn_dumpable();
 
  // check program access
  if (access(argv[1], R_OK)) {

diff --git a/src/fnet/Makefile.in b/src/fnet/Makefile.in
@@ -5,8 +5,8 @@ include ../common.mk
 %.o : %.c $(H_FILE_LIST) ../include/common.h ../include/libnetlink.h
  $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(INCLUDE) -c $< -o $@
 
-fnet: $(OBJS) ../lib/libnetlink.o
- $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/libnetlink.o $(LIBS) $(EXTRA_LDFLAGS)
+fnet: $(OBJS) ../lib/common.o ../lib/libnetlink.o
+ $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/common.o ../lib/libnetlink.o $(LIBS) $(EXTRA_LDFLAGS)
 
 clean:; rm -fr *.o fnet *.gcov *.gcda *.gcno *.plist
 

diff --git a/src/fnet/main.c b/src/fnet/main.c
@@ -21,7 +21,6 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/utsname.h>
-#include <sys/prctl.h>
 
 int arg_quiet = 0;
 
@@ -69,10 +68,9 @@ printf("\n");
  usage();
  return 0;
  }
-#ifdef WARN_DUMPABLE
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid())
- fprintf(stderr, "Error fnet: I am dumpable\n");
-#endif
+
+ warn_dumpable();
+
  char *quiet = getenv("FIREJAIL_QUIET");
  if (quiet && strcmp(quiet, "yes") == 0)
  arg_quiet = 1;

diff --git a/src/fnetfilter/Makefile.in b/src/fnetfilter/Makefile.in
@@ -5,8 +5,8 @@ include ../common.mk
 %.o : %.c $(H_FILE_LIST) ../include/common.h ../include/syscall.h
  $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(INCLUDE) -c $< -o $@
 
-fnetfilter: $(OBJS)
- $(CC) $(LDFLAGS) -o $@ $(OBJS) $(LIBS) $(EXTRA_LDFLAGS)
+fnetfilter: $(OBJS) ../lib/common.o
+ $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/common.o $(LIBS) $(EXTRA_LDFLAGS)
 
 clean:; rm -fr *.o fnetfilter *.gcov *.gcda *.gcno *.plist
 

diff --git a/src/fnetfilter/main.c b/src/fnetfilter/main.c
@@ -18,7 +18,6 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 #include "../include/common.h"
-#include <sys/prctl.h>
 
 #define MAXBUF 4098
 #define MAXARGS 16
@@ -181,10 +180,9 @@ printf("\n");
  usage();
  return 1;
  }
-#ifdef WARN_DUMPABLE
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid())
- fprintf(stderr, "Error fnetfilter: I am dumpable\n");
-#endif
+
+ warn_dumpable();
+
  char *destfile = (argc == 3)? argv[2]: argv[1];
  char *command = (argc == 3)? argv[1]: NULL;
 //printf("command %s\n", command);

diff --git a/src/fsec-optimize/Makefile.in b/src/fsec-optimize/Makefile.in
@@ -5,8 +5,8 @@ include ../common.mk
 %.o : %.c $(H_FILE_LIST) ../include/common.h ../include/seccomp.h ../include/syscall.h
  $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(INCLUDE) -c $< -o $@
 
-fsec-optimize: $(OBJS) ../lib/libnetlink.o
- $(CC) $(LDFLAGS) -o $@ $(OBJS) $(LIBS) $(EXTRA_LDFLAGS)
+fsec-optimize: $(OBJS) ../lib/common.o ../lib/libnetlink.o
+ $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/common.o $(LIBS) $(EXTRA_LDFLAGS)
 
 clean:; rm -fr *.o fsec-optimize *.gcov *.gcda *.gcno *.plist
 

diff --git a/src/fsec-optimize/fsec_optimize.h b/src/fsec-optimize/fsec_optimize.h
@@ -22,7 +22,6 @@
 #include "../include/common.h"
 #include "../include/seccomp.h"
 #include <sys/mman.h>
-#include <sys/prctl.h>
 
 // optimize.c
 struct sock_filter *duplicate(struct sock_filter *filter, int entries);

diff --git a/src/fsec-optimize/main.c b/src/fsec-optimize/main.c
@@ -44,11 +44,7 @@ printf("\n");
  return 0;
  }
 
-#ifdef WARN_DUMPABLE
- // check FIREJAIL_PLUGIN in order to not print a warning during make
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid() && getenv("FIREJAIL_PLUGIN"))
- fprintf(stderr, "Error fsec-optimize: I am dumpable\n");
-#endif
+ warn_dumpable();
 
  char *fname = argv[1];
 

diff --git a/src/fsec-print/Makefile.in b/src/fsec-print/Makefile.in
@@ -5,8 +5,8 @@ include ../common.mk
 %.o : %.c $(H_FILE_LIST) ../include/common.h ../include/seccomp.h ../include/syscall.h
  $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(INCLUDE) -c $< -o $@
 
-fsec-print: $(OBJS) ../lib/libnetlink.o ../lib/errno.o ../lib/syscall.o
- $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/errno.o ../lib/syscall.o $(LIBS) $(EXTRA_LDFLAGS)
+fsec-print: $(OBJS) ../lib/common.o ../lib/libnetlink.o ../lib/errno.o ../lib/syscall.o
+ $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/common.o ../lib/errno.o ../lib/syscall.o $(LIBS) $(EXTRA_LDFLAGS)
 
 clean:; rm -fr *.o fsec-print *.gcov *.gcda *.gcno *.plist
 

diff --git a/src/fsec-print/fsec_print.h b/src/fsec-print/fsec_print.h
@@ -23,7 +23,6 @@
 #include "../include/seccomp.h"
 #include "../include/syscall.h"
 #include <sys/mman.h>
-#include <sys/prctl.h>
 
 // print.c
 void print(struct sock_filter *filter, int entries);

diff --git a/src/fsec-print/main.c b/src/fsec-print/main.c
@@ -61,10 +61,7 @@ printf("\n");
  return 0;
  }
 
-#ifdef WARN_DUMPABLE
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid())
- fprintf(stderr, "Error fsec-print: I am dumpable\n");
-#endif
+ warn_dumpable();
 
  char *fname = argv[1];
 

diff --git a/src/fseccomp/Makefile.in b/src/fseccomp/Makefile.in
@@ -5,8 +5,8 @@ include ../common.mk
 %.o : %.c $(H_FILE_LIST) ../include/common.h ../include/syscall.h
  $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(INCLUDE) -c $< -o $@
 
-fseccomp: $(OBJS) ../lib/errno.o ../lib/syscall.o
- $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/errno.o ../lib/syscall.o $(LIBS) $(EXTRA_LDFLAGS)
+fseccomp: $(OBJS) ../lib/common.o ../lib/errno.o ../lib/syscall.o
+ $(CC) $(LDFLAGS) -o $@ $(OBJS) ../lib/common.o ../lib/errno.o ../lib/syscall.o $(LIBS) $(EXTRA_LDFLAGS)
 
 clean:; rm -fr *.o fseccomp *.gcov *.gcda *.gcno *.plist
 

diff --git a/src/fseccomp/fseccomp.h b/src/fseccomp/fseccomp.h
@@ -23,7 +23,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <assert.h>
-#include <sys/prctl.h>
 #include "../include/common.h"
 #include "../include/syscall.h"
 

diff --git a/src/fseccomp/main.c b/src/fseccomp/main.c
@@ -69,11 +69,7 @@ printf("\n");
  return 0;
  }
 
-#ifdef WARN_DUMPABLE
- // check FIREJAIL_PLUGIN in order to not print a warning during make
- if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getuid() && getenv("FIREJAIL_PLUGIN"))
- fprintf(stderr, "Error fseccomp: I am dumpable\n");
-#endif
+ warn_dumpable();
 
  char *quiet = getenv("FIREJAIL_QUIET");
  if (quiet && strcmp(quiet, "yes") == 0)

diff --git a/src/include/common.h b/src/include/common.h
@@ -38,11 +38,6 @@
 
 #define errExit(msg) do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
 
-// check if processes run with dumpable flag set
-// currently we get "Error fseccomp: I am dumpable" every time we run a firejail build on Debian 8,
-// regardless what Debian version we run the build on
-//#define WARN_DUMPABLE
-
 // macro to print ip addresses in a printf statement
 #define PRINT_IP(A) \
 ((int) (((A) >> 24) & 0xFF)), ((int) (((A) >> 16) & 0xFF)), ((int) (((A) >> 8) & 0xFF)), ((int) ( (A) & 0xFF))
@@ -126,4 +121,6 @@ char *pid_proc_comm(const pid_t pid);
 char *pid_proc_cmdline(const pid_t pid);
 int pid_proc_cmdline_x11_xpra_xephyr(const pid_t pid);
 int pid_hidepid(void);
+void warn_dumpable(void);
+const char *gnu_basename(const char *path);
 #endif
diff --git a/src/lib/common.c b/src/lib/common.c
@@ -267,7 +267,6 @@ int pid_proc_cmdline_x11_xpra_xephyr(const pid_t pid) {
 }
 
 // return 1 if /proc is mounted hidepid, or if /proc/mouns access is denied
-#define BUFLEN 4096
 int pid_hidepid(void) {
  FILE *fp = fopen("/proc/mounts", "r");
  if (!fp)
@@ -288,6 +287,39 @@ int pid_hidepid(void) {
  return 0;
 }
 
+// print error if unprivileged users can trace the process
+void warn_dumpable(void) {
+ if (getuid() != 0 && prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 1 && getenv("FIREJAIL_PLUGIN")) {
+ fprintf(stderr, "Error: dumpable process\n");
+
+ // best effort to provide detailed debug information
+ // cannot use process name, it is just a file descriptor number
+ char path[BUFLEN];
+ ssize_t len = readlink("/proc/self/exe", path, BUFLEN - 1);
+ if (len < 0)
+ return;
+ path[len] = '\0';
+ // path can refer to a sandbox mount namespace, use basename only
+ const char *base = gnu_basename(path);
+
+ struct stat s;
+ if (stat("/proc/self/exe", &s) == 0 && s.st_uid != 0)
+ fprintf(stderr, "Change owner of %s executable to root\n", base);
+ else if (access("/proc/self/exe", R_OK) == 0)
+ fprintf(stderr, "Remove read permission on %s executable\n", base);
+ }
+}
+
+// Equivalent to the GNU version of basename, which is incompatible with
+// the POSIX basename. A few lines of code saves any portability pain.
+// https://www.gnu.org/software/libc/manual/html_node/Finding-Tokens-in-a-String.html#index-basename
+const char *gnu_basename(const char *path) {
+ const char *last_slash = strrchr(path, '/');
+ if (!last_slash)
+ return path;
+ return last_slash+1;
+}
+
 //**************************
 // time trace based on getticks function
 //**************************