From ee31d6b817e3777e625f23f460aa1daeda3637f7 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Sat, 24 Jul 2021 13:42:06 +0200 Subject: organize program startup fixes a number of smaller issues: * enable allow-debuggers option for Firejail login shells * dhcp: noblacklist /sbin and /usr/sbin also when configuring a Firejail login shell * don't print error when built with disable-suid: firejail --nonewprivs --quiet firejail * don't process appimage option twice * no unnecessary argument parsing when run via firecfg symbolic link * process quiet option earlier, so it is available to init_cfg --- src/firejail/main.c | 189 +++++++++++++++++++++++----------------------- src/firejail/no_sandbox.c | 3 +- src/firejail/util.c | 5 +- 3 files changed, 96 insertions(+), 101 deletions(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 374afed11..a59d508e5 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -988,24 +988,16 @@ int main(int argc, char **argv, char **envp) { int arg_caps_cmdline = 0; // caps requested on command line (used to break out of --chroot) char **ptr; -#ifndef HAVE_SUID - if (geteuid() != 0) { - fprintf(stderr, "Error: Firejail needs to be SUID.\n"); - fprintf(stderr, "Assuming firejail is installed in /usr/bin, execute the following command as root:\n"); - fprintf(stderr, " chmod u+s /usr/bin/firejail\n"); - } -#endif - // sanitize the umask orig_umask = umask(022); - // check standard streams before printing anything - fix_std_streams(); - // drop permissions by default and rise them when required EUID_INIT(); EUID_USER(); + // check standard streams before opening any file + fix_std_streams(); + // argument count should be larger than 0 if (argc == 0 || !argv || strlen(argv[0]) == 0) { fprintf(stderr, "Error: argv is invalid\n"); @@ -1015,16 +1007,6 @@ int main(int argc, char **argv, char **envp) { exit(1); } - // Stash environment variables - for (i = 0, ptr = envp; ptr && *ptr && i < MAX_ENVS; i++, ptr++) - env_store(*ptr, SETENV); - - // sanity check for environment variables - if (i >= MAX_ENVS) { - fprintf(stderr, "Error: too many environment variables\n"); - exit(1); - } - // sanity check for arguments for (i = 0; i < argc; i++) { if (*argv[i] == 0) { @@ -1037,82 +1019,29 @@ int main(int argc, char **argv, char **envp) { } } + // Stash environment variables + for (i = 0, ptr = envp; ptr && *ptr && i < MAX_ENVS; i++, ptr++) + env_store(*ptr, SETENV); + + // sanity check for environment variables + if (i >= MAX_ENVS) { + fprintf(stderr, "Error: too many environment variables\n"); + exit(1); + } + // Reapply a minimal set of environment variables env_apply_whitelist(); - // check if the user is allowed to use firejail - init_cfg(argc, argv); - - // get starting timestamp, process --quiet - timetrace_start(); + // process --quiet const char *env_quiet = env_get("FIREJAIL_QUIET"); if (check_arg(argc, argv, "--quiet", 1) || (env_quiet && strcmp(env_quiet, "yes") == 0)) arg_quiet = 1; - // cleanup at exit - EUID_ROOT(); - atexit(clear_atexit); - - // build /run/firejail directory structure - preproc_build_firejail_dir(); - const char *container_name = env_get("container"); - if (!container_name || strcmp(container_name, "firejail")) { - lockfd_directory = open(RUN_DIRECTORY_LOCK_FILE, O_WRONLY | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR); - if (lockfd_directory != -1) { - int rv = fchown(lockfd_directory, 0, 0); - (void) rv; - flock(lockfd_directory, LOCK_EX); - } - preproc_clean_run(); - flock(lockfd_directory, LOCK_UN); - close(lockfd_directory); - } - EUID_USER(); - - // --ip=dhcp - we need access to /sbin and /usr/sbin directories in order to run ISC DHCP client (dhclient) - // these paths are disabled in disable-common.inc - if ((i = check_arg(argc, argv, "--ip", 0)) != 0) { - if (strncmp(argv[i] + 4, "=dhcp", 5) == 0) { - profile_add("noblacklist /sbin"); - profile_add("noblacklist /usr/sbin"); - } - } - - // for appimages we need to remove "include disable-shell.inc from the profile - // a --profile command can show up before --appimage - if (check_arg(argc, argv, "--appimage", 1)) - arg_appimage = 1; - - // process allow-debuggers - if (check_arg(argc, argv, "--allow-debuggers", 1)) { - // check kernel version - struct utsname u; - int rv = uname(&u); - if (rv != 0) - errExit("uname"); - int major; - int minor; - if (2 != sscanf(u.release, "%d.%d", &major, &minor)) { - fprintf(stderr, "Error: cannot extract Linux kernel version: %s\n", u.version); - exit(1); - } - if (major < 4 || (major == 4 && minor < 8)) { - fprintf(stderr, "Error: --allow-debuggers is disabled on Linux kernels prior to 4.8. " - "A bug in ptrace call allows a full bypass of the seccomp filter. " - "Your current kernel version is %d.%d.\n", major, minor); - exit(1); - } - - arg_allow_debuggers = 1; - char *cmd = strdup("noblacklist ${PATH}/strace"); - if (!cmd) - errExit("strdup"); - profile_add(cmd); - } + // check if the user is allowed to use firejail + init_cfg(argc, argv); - // profile builder - if (check_arg(argc, argv, "--build", 0)) // supports both --build and --build=filename - run_builder(argc, argv); // this function will not return + // get starting timestamp + timetrace_start(); // check argv[0] symlink wrapper if this is not a login shell if (*argv[0] != '-') @@ -1137,15 +1066,40 @@ int main(int argc, char **argv, char **envp) { __builtin_unreachable(); } } - EUID_ASSERT(); + // profile builder + if (check_arg(argc, argv, "--build", 0)) // supports both --build and --build=filename + run_builder(argc, argv); // this function will not return - // check firejail directories EUID_ROOT(); - delete_run_files(sandbox_pid); +#ifndef HAVE_SUID + if (geteuid() != 0) { + fprintf(stderr, "Error: Firejail needs to be SUID.\n"); + fprintf(stderr, "Assuming firejail is installed in /usr/bin, execute the following command as root:\n"); + fprintf(stderr, " chmod u+s /usr/bin/firejail\n"); + } +#endif + + // build /run/firejail directory structure + preproc_build_firejail_dir(); + const char *container_name = env_get("container"); + if (!container_name || strcmp(container_name, "firejail")) { + lockfd_directory = open(RUN_DIRECTORY_LOCK_FILE, O_WRONLY | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR); + if (lockfd_directory != -1) { + int rv = fchown(lockfd_directory, 0, 0); + (void) rv; + flock(lockfd_directory, LOCK_EX); + } + preproc_clean_run(); + flock(lockfd_directory, LOCK_UN); + close(lockfd_directory); + } + + delete_run_files(getpid()); + atexit(clear_atexit); EUID_USER(); - //check if the parent is sshd daemon + // check if the parent is sshd daemon int parent_sshd = 0; { pid_t ppid = getppid(); @@ -1202,7 +1156,8 @@ int main(int argc, char **argv, char **envp) { } EUID_ASSERT(); - // is this a login shell, or a command passed by sshd, insert command line options from /etc/firejail/login.users + // is this a login shell, or a command passed by sshd, + // insert command line options from /etc/firejail/login.users if (*argv[0] == '-' || parent_sshd) { if (argc == 1) login_shell = 1; @@ -1254,6 +1209,47 @@ int main(int argc, char **argv, char **envp) { #endif EUID_ASSERT(); + // --ip=dhcp - we need access to /sbin and /usr/sbin directories in order to run ISC DHCP client (dhclient) + // these paths are disabled in disable-common.inc + if ((i = check_arg(argc, argv, "--ip", 0)) != 0) { + if (strncmp(argv[i] + 4, "=dhcp", 5) == 0) { + profile_add("noblacklist /sbin"); + profile_add("noblacklist /usr/sbin"); + } + } + + // process allow-debuggers + if (check_arg(argc, argv, "--allow-debuggers", 1)) { + // check kernel version + struct utsname u; + int rv = uname(&u); + if (rv != 0) + errExit("uname"); + int major; + int minor; + if (2 != sscanf(u.release, "%d.%d", &major, &minor)) { + fprintf(stderr, "Error: cannot extract Linux kernel version: %s\n", u.version); + exit(1); + } + if (major < 4 || (major == 4 && minor < 8)) { + fprintf(stderr, "Error: --allow-debuggers is disabled on Linux kernels prior to 4.8. " + "A bug in ptrace call allows a full bypass of the seccomp filter. " + "Your current kernel version is %d.%d.\n", major, minor); + exit(1); + } + + arg_allow_debuggers = 1; + char *cmd = strdup("noblacklist ${PATH}/strace"); + if (!cmd) + errExit("strdup"); + profile_add(cmd); + } + + // for appimages we need to remove "include disable-shell.inc from the profile + // a --profile command can show up before --appimage + if (check_arg(argc, argv, "--appimage", 1)) + arg_appimage = 1; + // check for force-nonewprivs in /etc/firejail/firejail.config file if (checkcfg(CFG_FORCE_NONEWPRIVS)) arg_nonewprivs = 1; @@ -2652,8 +2648,9 @@ int main(int argc, char **argv, char **envp) { //************************************* else if (strncmp(argv[i], "--timeout=", 10) == 0) cfg.timeout = extract_timeout(argv[i] + 10); - else if (strcmp(argv[i], "--appimage") == 0) - arg_appimage = 1; + else if (strcmp(argv[i], "--appimage") == 0) { + // already handled + } else if (strcmp(argv[i], "--shell=none") == 0) { arg_shell_none = 1; if (cfg.shell) { diff --git a/src/firejail/no_sandbox.c b/src/firejail/no_sandbox.c index 665bef73d..0e5562d90 100644 --- a/src/firejail/no_sandbox.c +++ b/src/firejail/no_sandbox.c @@ -49,6 +49,7 @@ int check_namespace_virt(void) { // check PID 1 container environment variable EUID_ROOT(); FILE *fp = fopen("/proc/1/environ", "re"); + EUID_USER(); if (fp) { int c = 0; while (c != EOF) { @@ -69,7 +70,6 @@ int check_namespace_virt(void) { // found it if (is_container(buf + 10)) { fclose(fp); - EUID_USER(); return 1; } } @@ -79,7 +79,6 @@ int check_namespace_virt(void) { fclose(fp); } - EUID_USER(); return 0; } diff --git a/src/firejail/util.c b/src/firejail/util.c index 8fcaf3f7b..2ff2d2973 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -1516,8 +1516,7 @@ void check_homedir(const char *dir) { exit(1); } // symlinks are rejected in many places - if (has_link(dir)) { - fprintf(stderr, "No full support for symbolic links in path of user directory.\n" + if (has_link(dir)) + fmessage("No full support for symbolic links in path of user directory.\n" "Please provide resolved path in password database (/etc/passwd).\n\n"); - } } -- cgit v1.2.3-70-g09d2