From 045ee374387cb8fd9b1d83b14574c6d92694063d Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 14 Apr 2024 14:10:23 +0200 Subject: [PATCH] libstore/local-derivation-goal: prohibit creating setuid/setgid binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Linux kernel >=6.6 & glibc 2.39 a `fchmodat2(2)` is available that isn't filtered away by the libseccomp sandbox. Being able to use this to bypass that restriction has surprising results for some builds such as lxc[1]: > With kernel ≥6.6 and glibc 2.39, lxc's install phase uses fchmodat2, > which slips through https://github.com/NixOS/nix/blob/9b88e5284608116b7db0dbd3d5dd7a33b90d52d7/src/libstore/build/local-derivation-goal.cc#L1650-L1663. > The fixupPhase then uses fchmodat, which fails. > With older kernel or glibc, setting the suid bit fails in the > install phase, which is not treated as fatal, and then the > fixup phase does not try to set it again. Please note that there are still ways to bypass this sandbox[2] and this is mostly a fix for the breaking builds. This change works by creating a syscall filter for the `fchmodat2` syscall (number 452 on most systems). The problem is that glibc 2.39 is needed to have the correct syscall number available via `__NR_fchmodat2` / `__SNR_fchmodat2`, but this flake is still on nixpkgs 23.11. To have this change everywhere and not dependent on the glibc this package is built against, I added a header "fchmodat2-compat.hh" that sets the syscall number based on the architecture. On most platforms its 452 according to glibc with a few exceptions: $ rg --pcre2 'define __NR_fchmodat2 (?!452)' sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h 58:#define __NR_fchmodat2 1073742276 sysdeps/unix/sysv/linux/mips/mips64/n32/arch-syscall.h 67:#define __NR_fchmodat2 6452 sysdeps/unix/sysv/linux/mips/mips64/n64/arch-syscall.h 62:#define __NR_fchmodat2 5452 sysdeps/unix/sysv/linux/mips/mips32/arch-syscall.h 70:#define __NR_fchmodat2 4452 sysdeps/unix/sysv/linux/alpha/arch-syscall.h 59:#define __NR_fchmodat2 562 I added a small regression-test to the setuid integration-test that attempts to set the suid bit on a file using the fchmodat2 syscall. I confirmed that the test fails without the change in local-derivation-goal. Additionally, we require libseccomp 2.5.5 or greater now: as it turns out, libseccomp maintains an internal syscall table and validates each rule against it. This means that when using libseccomp 2.5.4 or older, one may pass `452` as syscall number against it, but since it doesn't exist in the internal structure, `libseccomp` will refuse to create a filter for that. This happens with nixpkgs-23.11, i.e. on stable NixOS and when building Lix against the project's flake. To work around that * a backport of libseccomp 2.5.5 on upstream nixpkgs has been scheduled[3]. * the package now uses libseccomp 2.5.5 on its own already. This is to provide a quick fix since the correct fix for 23.11 is still a staging cycle away. We still need the compat header though since `SCMP_SYS(fchmodat2)` internally transforms this into `__SNR_fchmodat2` which points to `__NR_fchmodat2` from glibc 2.39, so it wouldn't build on glibc 2.38. The updated syscall table from libseccomp 2.5.5 is NOT used for that step, but used later, so we need both, our compat header and their syscall table 🤷 Relevant PRs in CppNix: * https://github.com/NixOS/nix/pull/10591 * https://github.com/NixOS/nix/pull/10501 [1] https://github.com/NixOS/nixpkgs/issues/300635#issuecomment-2031073804 [2] https://github.com/NixOS/nixpkgs/issues/300635#issuecomment-2030844251 [3] https://github.com/NixOS/nixpkgs/pull/306070 (cherry picked from commit ba6804518772e6afb403dd55478365d4b863c854) Change-Id: I6921ab5a363188c6bff617750d00bb517276b7fe --- doc/manual/rl-next/fchmodat2-sandbox.md | 8 +++++ flake.nix | 3 +- meson.build | 2 +- package.nix | 22 ++++++++++-- src/libstore/build/local-derivation-goal.cc | 5 +++ src/libstore/linux/fchmodat2-compat.hh | 37 +++++++++++++++++++++ tests/nixos/default.nix | 2 +- tests/nixos/setuid/fchmodat2-suid.c | 21 ++++++++++++ tests/nixos/{ => setuid}/setuid.nix | 28 +++++++++++++++- 9 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 doc/manual/rl-next/fchmodat2-sandbox.md create mode 100644 src/libstore/linux/fchmodat2-compat.hh create mode 100644 tests/nixos/setuid/fchmodat2-suid.c rename tests/nixos/{ => setuid}/setuid.nix (79%) diff --git a/doc/manual/rl-next/fchmodat2-sandbox.md b/doc/manual/rl-next/fchmodat2-sandbox.md new file mode 100644 index 000000000..82429a93d --- /dev/null +++ b/doc/manual/rl-next/fchmodat2-sandbox.md @@ -0,0 +1,8 @@ +--- +synopsis: Creating setuid/setgid binaries with fchmodat2 is now prohibited by the build sandbox +prs: 10501 +--- + +The build sandbox blocks any attempt to create setuid/setgid binaries, but didn't check +for the use of the `fchmodat2` syscall which was introduced in Linux 6.6 and is used by +glibc >=2.39. This is fixed now. diff --git a/flake.nix b/flake.nix index e8526f5c4..d437cd3b8 100644 --- a/flake.nix +++ b/flake.nix @@ -163,9 +163,10 @@ busybox-sandbox-shell = final.busybox-sandbox-shell or final.default-busybox-sandbox-shell; }; - # Export the patched version of boehmgc that Lix uses into the overlay + # Export the patched version of boehmgc & libseccomp that Lix uses into the overlay # for consumers of this flake. boehmgc-nix = final.nix.boehmgc-nix; + libseccomp-nix = final.nix.libseccomp-nix; }; in { diff --git a/meson.build b/meson.build index d40a9029a..968255ef6 100644 --- a/meson.build +++ b/meson.build @@ -181,7 +181,7 @@ deps += cpuid # seccomp only makes sense on Linux seccomp_required = is_linux ? get_option('seccomp-sandboxing') : false -seccomp = dependency('libseccomp', 'seccomp', required : seccomp_required) +seccomp = dependency('libseccomp', 'seccomp', required : seccomp_required, version : '>=2.5.5') configdata += { 'HAVE_SECCOMP': seccomp.found().to_int(), } diff --git a/package.nix b/package.nix index c9cc17c29..ae26d49fe 100644 --- a/package.nix +++ b/package.nix @@ -21,12 +21,14 @@ curl, doxygen, editline, + fetchurl, flex, git, gtest, jq, libarchive, libcpuid, + libseccomp-nix ? __forDefaults.libseccomp-nix, libseccomp, libsodium, lsof, @@ -82,6 +84,18 @@ }; lix-doc = pkgs.callPackage ./lix-doc/package.nix { }; + + # remove when we drop 23.11 support (which includes a version too old to know about fchmodat2) + # see src/libstore/linux/fchmodat2-compat.hh + libseccomp-nix = + assert lib.versionOlder (lib.getVersion libseccomp) "2.5.5"; + libseccomp.overrideAttrs (_: rec { + version = "2.5.5"; + src = fetchurl { + url = "https://github.com/seccomp/libseccomp/releases/download/v${version}/libseccomp-${version}.tar.gz"; + hash = "sha256-JIosik2bmFiqa69ScSw0r+/PnJ6Ut23OAsHJqiX7M3U="; + }; + }); }, }: let @@ -273,7 +287,7 @@ stdenv.mkDerivation (finalAttrs: { lix-doc ] ++ lib.optionals stdenv.hostPlatform.isLinux [ - libseccomp + libseccomp-nix busybox-sandbox-shell ] ++ lib.optional internalApiDocs rapidcheck @@ -411,7 +425,9 @@ stdenv.mkDerivation (finalAttrs: { passthru.perl-bindings = pkgs.callPackage ./perl { inherit fileset stdenv buildWithMeson; }; - # Export the patched version of boehmgc. + # Export the patched version of boehmgc & libseccomp. # flake.nix exports that into its overlay. - passthru.boehmgc-nix = __forDefaults.boehmgc-nix; + passthru = { + inherit (__forDefaults) boehmgc-nix libseccomp-nix; + }; }) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 35e7ce907..248362c68 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -35,6 +35,7 @@ /* Includes required for chroot support. */ #if __linux__ #include +#include "linux/fchmodat2-compat.hh" #include #include #include @@ -1664,6 +1665,10 @@ void setupSeccomp() if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), SCMP_SYS(fchmodat), 1, SCMP_A2(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) != 0) throw SysError("unable to add seccomp rule"); + + if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), NIX_SYSCALL_FCHMODAT2, 1, + SCMP_A2(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) != 0) + throw SysError("unable to add seccomp rule"); } /* Prevent builders from creating EAs or ACLs. Not all filesystems diff --git a/src/libstore/linux/fchmodat2-compat.hh b/src/libstore/linux/fchmodat2-compat.hh new file mode 100644 index 000000000..b05da6786 --- /dev/null +++ b/src/libstore/linux/fchmodat2-compat.hh @@ -0,0 +1,37 @@ +/* + * Determine the syscall number for `fchmodat2`. + * + * On most platforms this is 452. Exceptions can be found on + * a glibc git checkout via `rg --pcre2 'define __NR_fchmodat2 (?!452)'`. + * + * The problem is that glibc 2.39 and libseccomp 2.5.5 are needed to + * get the syscall number. However, a Nix built against nixpkgs 23.11 + * (glibc 2.38) should still have the issue fixed without depending + * on the build environment. + * + * To achieve that, the macros below try to determine the platform and + * set the syscall number which is platform-specific, but + * in most cases 452. + * + * TODO: remove this when 23.11 is EOL and the entire (supported) ecosystem + * is on glibc 2.39. + */ + +#pragma once +///@file + +#if HAVE_SECCOMP +# if defined(__alpha__) +# define NIX_SYSCALL_FCHMODAT2 562 +# elif defined(__x86_64__) && SIZE_MAX == 0xFFFFFFFF // x32 +# define NIX_SYSCALL_FCHMODAT2 1073742276 +# elif defined(__mips__) && defined(__mips64) && defined(_ABIN64) // mips64/n64 +# define NIX_SYSCALL_FCHMODAT2 5452 +# elif defined(__mips__) && defined(__mips64) && defined(_ABIN32) // mips64/n32 +# define NIX_SYSCALL_FCHMODAT2 6452 +# elif defined(__mips__) && defined(_ABIO32) // mips32 +# define NIX_SYSCALL_FCHMODAT2 4452 +# else +# define NIX_SYSCALL_FCHMODAT2 452 +# endif +#endif // HAVE_SECCOMP diff --git a/tests/nixos/default.nix b/tests/nixos/default.nix index 3ef1217ac..f7a8588e5 100644 --- a/tests/nixos/default.nix +++ b/tests/nixos/default.nix @@ -153,7 +153,7 @@ in setuid = lib.genAttrs ["i686-linux" "x86_64-linux"] - (system: runNixOSTestFor system ./setuid.nix); + (system: runNixOSTestFor system ./setuid/setuid.nix); ca-fd-leak = runNixOSTestFor "x86_64-linux" ./ca-fd-leak; diff --git a/tests/nixos/setuid/fchmodat2-suid.c b/tests/nixos/setuid/fchmodat2-suid.c new file mode 100644 index 000000000..931489ad7 --- /dev/null +++ b/tests/nixos/setuid/fchmodat2-suid.c @@ -0,0 +1,21 @@ +#include +#include +#include +#include +#include +#include +#include + +int main(void) { + char *name = getenv("out"); + FILE *fd = fopen(name, "w"); + fprintf(fd, "henlo :3"); + fclose(fd); + + // FIXME use something nicer here that's less + // platform-dependent as soon as we go to 24.05 + // and the glibc is new enough to support fchmodat2 + long rs = syscall(452, NULL, name, S_ISUID, 0); + assert(rs == -1); + assert(errno == EPERM); +} diff --git a/tests/nixos/setuid.nix b/tests/nixos/setuid/setuid.nix similarity index 79% rename from tests/nixos/setuid.nix rename to tests/nixos/setuid/setuid.nix index 2b66320dd..c4dc8dccb 100644 --- a/tests/nixos/setuid.nix +++ b/tests/nixos/setuid/setuid.nix @@ -5,6 +5,16 @@ let pkgs = config.nodes.machine.nixpkgs.pkgs; + fchmodat2-builder = pkgs.runCommandCC "fchmodat2-suid" { + passAsFile = [ "code" ]; + code = builtins.readFile ./fchmodat2-suid.c; + # Doesn't work with -O0, shuts up the warning about that. + hardeningDisable = [ "fortify" ]; + } '' + mkdir -p $out/bin/ + $CC -x c "$codePath" -O0 -g -o $out/bin/fchmodat2-suid + ''; + in { name = "setuid"; @@ -14,13 +24,29 @@ in { virtualisation.writableStore = true; nix.settings.substituters = lib.mkForce [ ]; nix.nixPath = [ "nixpkgs=${lib.cleanSource pkgs.path}" ]; - virtualisation.additionalPaths = [ pkgs.stdenvNoCC pkgs.pkgsi686Linux.stdenvNoCC ]; + virtualisation.additionalPaths = [ + pkgs.stdenvNoCC + pkgs.pkgsi686Linux.stdenvNoCC + fchmodat2-builder + ]; + # need at least 6.6 to test for fchmodat2 + boot.kernelPackages = pkgs.linuxKernel.packages.linux_6_6; + }; testScript = { nodes }: '' # fmt: off start_all() + with subtest("fchmodat2 suid regression test"): + machine.succeed(""" + nix-build -E '(with import {}; runCommand "fchmodat2-suid" { + BUILDER = builtins.storePath ${fchmodat2-builder}; + } " + exec \\"$BUILDER\\"/bin/fchmodat2-suid + ")' + """) + # Copying to /tmp should succeed. machine.succeed(r""" nix-build --no-sandbox -E '(with import {}; runCommand "foo" {} "