nixos/security/wrappers: stop using .real
files
Before this change it was crucial that nonprivileged users are unable to create hardlinks to SUID wrappers, lest they be able to provide a different `.real` file alongside. That was ensured by not providing a location writable to them in the /run/wrappers tmpfs, (unless disabled) by the fs.protected_hardlinks=1 sysctl, and by the explicit own-path check in the wrapper. After this change, ensuring that property is no longer important, and the check is most likely redundant. The simplification of expectations of the wrapper will make it easier to remove some of the assertions in the wrapper (which currently cause the wrapper to fail in no_new_privs environments, instead of executing the target with non-elevated privileges). Note that wrappers had to be copied (not symlinked) into /run/wrappers due to the SUID/capability bits, and they couldn't be hard/softlinks of each other due to those bits potentially differing. Thus, this change doesn't increase the amount of memory used by /run/wrappers. This change removes part of the test that is obsoleted by the removal of `.real` files.
This commit is contained in:
parent
44fde723be
commit
1bdbc0b0fe
|
@ -5,8 +5,8 @@ let
|
||||||
|
|
||||||
parentWrapperDir = dirOf wrapperDir;
|
parentWrapperDir = dirOf wrapperDir;
|
||||||
|
|
||||||
securityWrapper = pkgs.callPackage ./wrapper.nix {
|
securityWrapper = sourceProg : pkgs.callPackage ./wrapper.nix {
|
||||||
inherit parentWrapperDir;
|
inherit parentWrapperDir sourceProg;
|
||||||
};
|
};
|
||||||
|
|
||||||
fileModeType =
|
fileModeType =
|
||||||
|
@ -91,8 +91,7 @@ let
|
||||||
, ...
|
, ...
|
||||||
}:
|
}:
|
||||||
''
|
''
|
||||||
cp ${securityWrapper}/bin/security-wrapper "$wrapperDir/${program}"
|
cp ${securityWrapper source}/bin/security-wrapper "$wrapperDir/${program}"
|
||||||
echo -n "${source}" > "$wrapperDir/${program}.real"
|
|
||||||
|
|
||||||
# Prevent races
|
# Prevent races
|
||||||
chmod 0000 "$wrapperDir/${program}"
|
chmod 0000 "$wrapperDir/${program}"
|
||||||
|
@ -119,8 +118,7 @@ let
|
||||||
, ...
|
, ...
|
||||||
}:
|
}:
|
||||||
''
|
''
|
||||||
cp ${securityWrapper}/bin/security-wrapper "$wrapperDir/${program}"
|
cp ${securityWrapper source}/bin/security-wrapper "$wrapperDir/${program}"
|
||||||
echo -n "${source}" > "$wrapperDir/${program}.real"
|
|
||||||
|
|
||||||
# Prevent races
|
# Prevent races
|
||||||
chmod 0000 "$wrapperDir/${program}"
|
chmod 0000 "$wrapperDir/${program}"
|
||||||
|
@ -251,10 +249,9 @@ in
|
||||||
security.apparmor.includes = lib.mapAttrs' (wrapName: wrap: lib.nameValuePair
|
security.apparmor.includes = lib.mapAttrs' (wrapName: wrap: lib.nameValuePair
|
||||||
"nixos/security.wrappers/${wrapName}" ''
|
"nixos/security.wrappers/${wrapName}" ''
|
||||||
include "${pkgs.apparmorRulesFromClosure { name="security.wrappers.${wrapName}"; } [
|
include "${pkgs.apparmorRulesFromClosure { name="security.wrappers.${wrapName}"; } [
|
||||||
securityWrapper
|
(securityWrapper wrap.source)
|
||||||
]}"
|
]}"
|
||||||
mrpx ${wrap.source},
|
mrpx ${wrap.source},
|
||||||
r /run/wrappers/wrappers.*/${wrapName}.real,
|
|
||||||
'') wrappers;
|
'') wrappers;
|
||||||
|
|
||||||
###### wrappers activation script
|
###### wrappers activation script
|
||||||
|
|
|
@ -17,6 +17,10 @@
|
||||||
#include <syscall.h>
|
#include <syscall.h>
|
||||||
#include <byteswap.h>
|
#include <byteswap.h>
|
||||||
|
|
||||||
|
#ifndef SOURCE_PROG
|
||||||
|
#error SOURCE_PROG should be defined via preprocessor commandline
|
||||||
|
#endif
|
||||||
|
|
||||||
// aborts when false, printing the failed expression
|
// aborts when false, printing the failed expression
|
||||||
#define ASSERT(expr) ((expr) ? (void) 0 : assert_failure(#expr))
|
#define ASSERT(expr) ((expr) ? (void) 0 : assert_failure(#expr))
|
||||||
// aborts when returns non-zero, printing the failed expression and errno
|
// 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);
|
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,
|
// Make sure that we are being executed from the right location,
|
||||||
// i.e., `safe_wrapper_dir'. This is to prevent someone from creating
|
// i.e., `safe_wrapper_dir'.
|
||||||
// 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);
|
int len = strlen(wrapper_dir);
|
||||||
if (len > 0 && '/' == wrapper_dir[len - 1])
|
if (len > 0 && '/' == wrapper_dir[len - 1])
|
||||||
--len;
|
--len;
|
||||||
|
@ -230,23 +233,6 @@ int main(int argc, char **argv) {
|
||||||
// And, of course, we shouldn't be writable.
|
// And, of course, we shouldn't be writable.
|
||||||
ASSERT(!(st.st_mode & (S_IWGRP | S_IWOTH)));
|
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
|
// Read the capabilities set on the wrapper and raise them in to
|
||||||
// the ambient set so the program we're wrapping receives the
|
// the ambient set so the program we're wrapping receives the
|
||||||
// capabilities too!
|
// capabilities too!
|
||||||
|
@ -256,10 +242,10 @@ int main(int argc, char **argv) {
|
||||||
}
|
}
|
||||||
free(self_path);
|
free(self_path);
|
||||||
|
|
||||||
execve(source_prog, argv, environ);
|
execve(SOURCE_PROG, argv, environ);
|
||||||
|
|
||||||
fprintf(stderr, "%s: cannot run `%s': %s\n",
|
fprintf(stderr, "%s: cannot run `%s': %s\n",
|
||||||
argv[0], source_prog, strerror(errno));
|
argv[0], SOURCE_PROG, strerror(errno));
|
||||||
|
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,4 +1,4 @@
|
||||||
{ stdenv, linuxHeaders, parentWrapperDir, debug ? false }:
|
{ stdenv, linuxHeaders, parentWrapperDir, sourceProg, debug ? false }:
|
||||||
# For testing:
|
# For testing:
|
||||||
# $ nix-build -E 'with import <nixpkgs> {}; pkgs.callPackage ./wrapper.nix { parentWrapperDir = "/run/wrappers"; debug = true; }'
|
# $ nix-build -E 'with import <nixpkgs> {}; pkgs.callPackage ./wrapper.nix { parentWrapperDir = "/run/wrappers"; debug = true; }'
|
||||||
stdenv.mkDerivation {
|
stdenv.mkDerivation {
|
||||||
|
@ -8,6 +8,7 @@ stdenv.mkDerivation {
|
||||||
hardeningEnable = [ "pie" ];
|
hardeningEnable = [ "pie" ];
|
||||||
CFLAGS = [
|
CFLAGS = [
|
||||||
''-DWRAPPER_DIR="${parentWrapperDir}"''
|
''-DWRAPPER_DIR="${parentWrapperDir}"''
|
||||||
|
''-DSOURCE_PROG="${sourceProg}"''
|
||||||
] ++ (if debug then [
|
] ++ (if debug then [
|
||||||
"-Werror" "-Og" "-g"
|
"-Werror" "-Og" "-g"
|
||||||
] else [
|
] else [
|
||||||
|
|
|
@ -92,13 +92,6 @@ in
|
||||||
machine.succeed(cmd_as_regular('/run/wrappers/bin/capsh_with_chown --has-p=CAP_CHOWN'))
|
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'))
|
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
|
# 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
|
# 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
|
# actually makes the apparmor policy for ping, but there's no convenient
|
||||||
|
|
Loading…
Reference in a new issue