diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix index 12255d8392fe..ad65f80bb2ca 100644 --- a/nixos/modules/security/wrappers/default.nix +++ b/nixos/modules/security/wrappers/default.nix @@ -5,8 +5,8 @@ let parentWrapperDir = dirOf wrapperDir; - securityWrapper = pkgs.callPackage ./wrapper.nix { - inherit parentWrapperDir; + securityWrapper = sourceProg : pkgs.callPackage ./wrapper.nix { + inherit sourceProg; }; fileModeType = @@ -91,8 +91,7 @@ let , ... }: '' - cp ${securityWrapper}/bin/security-wrapper "$wrapperDir/${program}" - echo -n "${source}" > "$wrapperDir/${program}.real" + cp ${securityWrapper source}/bin/security-wrapper "$wrapperDir/${program}" # Prevent races chmod 0000 "$wrapperDir/${program}" @@ -119,8 +118,7 @@ let , ... }: '' - cp ${securityWrapper}/bin/security-wrapper "$wrapperDir/${program}" - echo -n "${source}" > "$wrapperDir/${program}.real" + cp ${securityWrapper source}/bin/security-wrapper "$wrapperDir/${program}" # Prevent races chmod 0000 "$wrapperDir/${program}" @@ -248,11 +246,13 @@ in export PATH="${wrapperDir}:$PATH" ''; - security.apparmor.includes."nixos/security.wrappers" = '' - include "${pkgs.apparmorRulesFromClosure { name="security.wrappers"; } [ - securityWrapper + security.apparmor.includes = lib.mapAttrs' (wrapName: wrap: lib.nameValuePair + "nixos/security.wrappers/${wrapName}" '' + include "${pkgs.apparmorRulesFromClosure { name="security.wrappers.${wrapName}"; } [ + (securityWrapper wrap.source) ]}" - ''; + mrpx ${wrap.source}, + '') wrappers; ###### wrappers activation script system.activationScripts.wrappers = diff --git a/nixos/modules/security/wrappers/wrapper.c b/nixos/modules/security/wrappers/wrapper.c index 17776a97af81..2cf1727a31c8 100644 --- a/nixos/modules/security/wrappers/wrapper.c +++ b/nixos/modules/security/wrappers/wrapper.c @@ -17,6 +17,10 @@ #include #include +#ifndef SOURCE_PROG +#error SOURCE_PROG should be defined via preprocessor commandline +#endif + // aborts when false, printing the failed expression #define ASSERT(expr) ((expr) ? (void) 0 : assert_failure(#expr)) // aborts when returns non-zero, printing the failed expression and errno @@ -24,10 +28,6 @@ extern char **environ; -// The WRAPPER_DIR macro is supplied at compile time so that it cannot -// be changed at runtime -static char *wrapper_dir = WRAPPER_DIR; - // Wrapper debug variable name static char *wrapper_debug = "WRAPPER_DEBUG"; @@ -151,115 +151,20 @@ static int make_caps_ambient(const char *self_path) { return 0; } -int readlink_malloc(const char *p, char **ret) { - size_t l = FILENAME_MAX+1; - int r; - - for (;;) { - char *c = calloc(l, sizeof(char)); - if (!c) { - return -ENOMEM; - } - - ssize_t n = readlink(p, c, l-1); - if (n < 0) { - r = -errno; - free(c); - return r; - } - - if ((size_t) n < l-1) { - c[n] = 0; - *ret = c; - return 0; - } - - free(c); - l *= 2; - } -} - int main(int argc, char **argv) { ASSERT(argc >= 1); - char *self_path = NULL; - int self_path_size = readlink_malloc("/proc/self/exe", &self_path); - if (self_path_size < 0) { - fprintf(stderr, "cannot readlink /proc/self/exe: %s", strerror(-self_path_size)); - } - - unsigned int ruid, euid, suid, rgid, egid, sgid; - MUSTSUCCEED(getresuid(&ruid, &euid, &suid)); - MUSTSUCCEED(getresgid(&rgid, &egid, &sgid)); - - // If true, then we did not benefit from setuid privilege escalation, - // where the original uid is still in ruid and different from euid == suid. - int didnt_suid = (ruid == euid) && (euid == suid); - // If true, then we did not benefit from setgid privilege escalation - int didnt_sgid = (rgid == egid) && (egid == sgid); - - - // Make sure that we are being executed from the right location, - // i.e., `safe_wrapper_dir'. This is to prevent someone from creating - // hard link `X' from some other location, along with a false - // `X.real' file, to allow arbitrary programs from being executed - // with elevated capabilities. - int len = strlen(wrapper_dir); - if (len > 0 && '/' == wrapper_dir[len - 1]) - --len; - ASSERT(!strncmp(self_path, wrapper_dir, len)); - ASSERT('/' == wrapper_dir[0]); - ASSERT('/' == self_path[len]); - - // If we got privileges with the fs set[ug]id bit, check that the privilege we - // got matches the one one we expected, ie that our effective uid/gid - // matches the uid/gid of `self_path`. This ensures that we were executed as - // `self_path', and not, say, as some other setuid program. - // We don't check that if we did not benefit from the set[ug]id bit, as - // can be the case in nosuid mounts or user namespaces. - struct stat st; - ASSERT(lstat(self_path, &st) != -1); - - // if the wrapper gained privilege with suid, check that we got the uid of the file owner - ASSERT(!((st.st_mode & S_ISUID) && !didnt_suid) || (st.st_uid == euid)); - // if the wrapper gained privilege with sgid, check that we got the gid of the file group - ASSERT(!((st.st_mode & S_ISGID) && !didnt_sgid) || (st.st_gid == egid)); - // same, but with suid instead of euid - ASSERT(!((st.st_mode & S_ISUID) && !didnt_suid) || (st.st_uid == suid)); - ASSERT(!((st.st_mode & S_ISGID) && !didnt_sgid) || (st.st_gid == sgid)); - - // And, of course, we shouldn't be writable. - ASSERT(!(st.st_mode & (S_IWGRP | S_IWOTH))); - - // Read the path of the real (wrapped) program from .real. - char real_fn[PATH_MAX + 10]; - int real_fn_size = snprintf(real_fn, sizeof(real_fn), "%s.real", self_path); - ASSERT(real_fn_size < sizeof(real_fn)); - - int fd_self = open(real_fn, O_RDONLY); - ASSERT(fd_self != -1); - - char source_prog[PATH_MAX]; - len = read(fd_self, source_prog, PATH_MAX); - ASSERT(len != -1); - ASSERT(len < sizeof(source_prog)); - ASSERT(len > 0); - source_prog[len] = 0; - - close(fd_self); // Read the capabilities set on the wrapper and raise them in to // the ambient set so the program we're wrapping receives the // capabilities too! - if (make_caps_ambient(self_path) != 0) { - free(self_path); + if (make_caps_ambient("/proc/self/exe") != 0) { return 1; } - free(self_path); - execve(source_prog, argv, environ); + execve(SOURCE_PROG, argv, environ); fprintf(stderr, "%s: cannot run `%s': %s\n", - argv[0], source_prog, strerror(errno)); + argv[0], SOURCE_PROG, strerror(errno)); return 1; } diff --git a/nixos/modules/security/wrappers/wrapper.nix b/nixos/modules/security/wrappers/wrapper.nix index e3620fb222d2..aec43412404e 100644 --- a/nixos/modules/security/wrappers/wrapper.nix +++ b/nixos/modules/security/wrappers/wrapper.nix @@ -1,4 +1,4 @@ -{ stdenv, linuxHeaders, parentWrapperDir, debug ? false }: +{ stdenv, linuxHeaders, sourceProg, debug ? false }: # For testing: # $ nix-build -E 'with import {}; pkgs.callPackage ./wrapper.nix { parentWrapperDir = "/run/wrappers"; debug = true; }' stdenv.mkDerivation { @@ -7,7 +7,7 @@ stdenv.mkDerivation { dontUnpack = true; hardeningEnable = [ "pie" ]; CFLAGS = [ - ''-DWRAPPER_DIR="${parentWrapperDir}"'' + ''-DSOURCE_PROG="${sourceProg}"'' ] ++ (if debug then [ "-Werror" "-Og" "-g" ] else [ diff --git a/nixos/modules/tasks/network-interfaces.nix b/nixos/modules/tasks/network-interfaces.nix index eb1c7512d920..0d4033ca9430 100644 --- a/nixos/modules/tasks/network-interfaces.nix +++ b/nixos/modules/tasks/network-interfaces.nix @@ -1396,14 +1396,12 @@ in security.apparmor.policies."bin.ping".profile = lib.mkIf config.security.apparmor.policies."bin.ping".enable (lib.mkAfter '' /run/wrappers/bin/ping { include - include + include rpx /run/wrappers/wrappers.*/ping, } /run/wrappers/wrappers.*/ping { include - include - r /run/wrappers/wrappers.*/ping.real, - mrpx ${config.security.wrappers.ping.source}, + include capability net_raw, capability setpcap, } diff --git a/nixos/tests/wrappers.nix b/nixos/tests/wrappers.nix index 391e9b42b45b..1d4fa85d7399 100644 --- a/nixos/tests/wrappers.nix +++ b/nixos/tests/wrappers.nix @@ -21,6 +21,8 @@ in }; }; + security.apparmor.enable = true; + security.wrappers = { suidRoot = { owner = "root"; @@ -84,17 +86,27 @@ in test_as_regular_in_userns_mapped_as_root('/run/wrappers/bin/sgid_root_busybox id -g', '0') test_as_regular_in_userns_mapped_as_root('/run/wrappers/bin/sgid_root_busybox id -rg', '0') + # Test that in nonewprivs environment the wrappers simply exec their target. + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/suid_root_busybox id -u', '${toString userUid}') + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/suid_root_busybox id -ru', '${toString userUid}') + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/suid_root_busybox id -g', '${toString usersGid}') + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/suid_root_busybox id -rg', '${toString usersGid}') + + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/sgid_root_busybox id -u', '${toString userUid}') + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/sgid_root_busybox id -ru', '${toString userUid}') + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/sgid_root_busybox id -g', '${toString usersGid}') + test_as_regular('${pkgs.util-linux}/bin/setpriv --no-new-privs /run/wrappers/bin/sgid_root_busybox id -rg', '${toString usersGid}') + # We are only testing the permitted set, because it's easiest to look at with capsh. machine.fail(cmd_as_regular('${pkgs.libcap}/bin/capsh --has-p=CAP_CHOWN')) machine.fail(cmd_as_regular('${pkgs.libcap}/bin/capsh --has-p=CAP_SYS_ADMIN')) machine.succeed(cmd_as_regular('/run/wrappers/bin/capsh_with_chown --has-p=CAP_CHOWN')) machine.fail(cmd_as_regular('/run/wrappers/bin/capsh_with_chown --has-p=CAP_SYS_ADMIN')) - # test a few "attacks" against which the wrapper protects itself - machine.succeed("cp /run/wrappers/bin/suid_root_busybox{,.real} /tmp/") - machine.fail(cmd_as_regular("/tmp/suid_root_busybox id -u")) - - machine.succeed("chmod u+s,a+w /run/wrappers/bin/suid_root_busybox") - machine.fail(cmd_as_regular("/run/wrappers/bin/suid_root_busybox id -u")) + # Test that the only user of apparmor policy includes generated by + # wrappers works. Ideally this'd be located in a test for the module that + # actually makes the apparmor policy for ping, but there's no convenient + # test for that one. + machine.succeed("ping -c 1 127.0.0.1") ''; })