diff options
| -rw-r--r-- | nixos/modules/security/wrappers/default.nix | 13 | ||||
| -rw-r--r-- | nixos/modules/security/wrappers/wrapper.c | 32 | ||||
| -rw-r--r-- | nixos/modules/security/wrappers/wrapper.nix | 3 | ||||
| -rw-r--r-- | nixos/tests/wrappers.nix | 7 |
4 files changed, 16 insertions, 39 deletions
diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix index 2f886cef3a7..95ef7fa2c4d 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 parentWrapperDir 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}" @@ -251,10 +249,9 @@ in security.apparmor.includes = lib.mapAttrs' (wrapName: wrap: lib.nameValuePair "nixos/security.wrappers/${wrapName}" '' include "${pkgs.apparmorRulesFromClosure { name="security.wrappers.${wrapName}"; } [ - securityWrapper + (securityWrapper wrap.source) ]}" mrpx ${wrap.source}, - r /run/wrappers/wrappers.*/${wrapName}.real, '') wrappers; ###### wrappers activation script diff --git a/nixos/modules/security/wrappers/wrapper.c b/nixos/modules/security/wrappers/wrapper.c index 17776a97af8..b00ec942320 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 @@ -198,11 +202,10 @@ int main(int argc, char **argv) { int didnt_sgid = (rgid == egid) && (egid == sgid); + // TODO: Determine if this is still useful, in particular if + // make_caps_ambient somehow relies on these properties. // 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. + // i.e., `safe_wrapper_dir'. int len = strlen(wrapper_dir); if (len > 0 && '/' == wrapper_dir[len - 1]) --len; @@ -230,23 +233,6 @@ int main(int argc, char **argv) { // 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! @@ -256,10 +242,10 @@ int main(int argc, char **argv) { } 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..afc5042768e 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, parentWrapperDir, sourceProg, debug ? false }: # For testing: # $ nix-build -E 'with import <nixpkgs> {}; pkgs.callPackage ./wrapper.nix { parentWrapperDir = "/run/wrappers"; debug = true; }' stdenv.mkDerivation { @@ -8,6 +8,7 @@ stdenv.mkDerivation { hardeningEnable = [ "pie" ]; CFLAGS = [ ''-DWRAPPER_DIR="${parentWrapperDir}"'' + ''-DSOURCE_PROG="${sourceProg}"'' ] ++ (if debug then [ "-Werror" "-Og" "-g" ] else [ diff --git a/nixos/tests/wrappers.nix b/nixos/tests/wrappers.nix index 4c7a82f7dd0..fc32ed41026 100644 --- a/nixos/tests/wrappers.nix +++ b/nixos/tests/wrappers.nix @@ -92,13 +92,6 @@ in 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 |