diff options
author | Pierre Bourdon <delroth@gmail.com> | 2023-09-10 09:51:36 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-10 09:51:36 +0200 |
commit | bfdf28becf033434fdeb1e6a481abf663d00c1a4 (patch) | |
tree | daff2fbb06a9e774d113f482e9adffc78e5ddf01 /nixos/modules/security | |
parent | cd971bb6f0b97f5d6fd8a3c4a53baf664f28ef48 (diff) | |
parent | 13d3b0c73350d1bcee16316a1d5c0f327f466f5c (diff) | |
download | nixpkgs-bfdf28becf033434fdeb1e6a481abf663d00c1a4.tar nixpkgs-bfdf28becf033434fdeb1e6a481abf663d00c1a4.tar.gz nixpkgs-bfdf28becf033434fdeb1e6a481abf663d00c1a4.tar.bz2 nixpkgs-bfdf28becf033434fdeb1e6a481abf663d00c1a4.tar.lz nixpkgs-bfdf28becf033434fdeb1e6a481abf663d00c1a4.tar.xz nixpkgs-bfdf28becf033434fdeb1e6a481abf663d00c1a4.tar.zst nixpkgs-bfdf28becf033434fdeb1e6a481abf663d00c1a4.zip |
Merge pull request #251770 from robryk/suidwrapapparm
nixos/security/wrappers: simplifications and a fix for #98863 (respin of #199599)
Diffstat (limited to 'nixos/modules/security')
-rw-r--r-- | nixos/modules/security/wrappers/default.nix | 20 | ||||
-rw-r--r-- | nixos/modules/security/wrappers/wrapper.c | 109 | ||||
-rw-r--r-- | nixos/modules/security/wrappers/wrapper.nix | 4 |
3 files changed, 19 insertions, 114 deletions
diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix index 12255d8392f..ad65f80bb2c 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 17776a97af8..2cf1727a31c 100644 --- a/nixos/modules/security/wrappers/wrapper.c +++ b/nixos/modules/security/wrappers/wrapper.c @@ -17,6 +17,10 @@ #include <syscall.h> #include <byteswap.h> +#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 <self>.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 e3620fb222d..aec43412404 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 <nixpkgs> {}; 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 [ |