Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/security/wrappers: use musl rather than glibc and explicitly unset insecure env vars #259039

Merged
merged 1 commit into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion nixos/modules/security/wrappers/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,29 @@ let

parentWrapperDir = dirOf wrapperDir;

securityWrapper = sourceProg : pkgs.callPackage ./wrapper.nix {
# This is security-sensitive code, and glibc vulns happen from time to time.
# musl is security-focused and generally more minimal, so it's a better choice here.
# The dynamic linker is still a fairly complex piece of code, and the wrappers are
# quite small, so linking it statically is more appropriate.
securityWrapper = sourceProg : pkgs.pkgsStatic.callPackage ./wrapper.nix {
inherit sourceProg;

# glibc definitions of insecure environment variables
#
# We extract the single header file we need into its own derivation,
# so that we don't have to pull full glibc sources to build wrappers.
#
# They're taken from pkgs.glibc so that we don't have to keep as close
# an eye on glibc changes. Not every relevant variable is in this header,
# so we maintain a slightly stricter list in wrapper.c itself as well.
unsecvars = lib.overrideDerivation (pkgs.srcOnly pkgs.glibc)
({ name, ... }: {
name = "${name}-unsecvars";
installPhase = ''
mkdir $out
cp sysdeps/generic/unsecvars.h $out
'';
});
};

fileModeType =
Expand Down
49 changes: 49 additions & 0 deletions nixos/modules/security/wrappers/wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
#include <syscall.h>
#include <byteswap.h>

// imported from glibc
#include "unsecvars.h"
edef1c marked this conversation as resolved.
Show resolved Hide resolved

#ifndef SOURCE_PROG
#error SOURCE_PROG should be defined via preprocessor commandline
#endif
Expand Down Expand Up @@ -151,9 +154,55 @@ static int make_caps_ambient(const char *self_path) {
return 0;
}

// These are environment variable aliases for glibc tunables.
// This list shouldn't grow further, since this is a legacy mechanism.
// Any future tunables are expected to only be accessible through GLIBC_TUNABLES.
//
// They are not included in the glibc-provided UNSECURE_ENVVARS list,
// since any SUID executable ignores them. This wrapper also serves
// executables that are merely granted ambient capabilities, rather than
// being SUID, and hence don't run in secure mode. We'd like them to
// defend those in depth as well, so we clear these explicitly.
//
// Except for MALLOC_CHECK_ (which is marked SXID_ERASE), these are all
// marked SXID_IGNORE (ignored in secure mode), so even the glibc version
// of this wrapper would leave them intact.
#define UNSECURE_ENVVARS_TUNABLES \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is our process to review the list of new environment variables in future?
Since we no longer use glibc, this is now a manual task now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was already mentioned somewhere in this (long) PR thread, but: this hardcoded list is not expected to grow, since glibc has switched to controlling these things through GLIBC_TUNABLES instead. And other environment variables are grabbed directly from the glibc source (UNSECURE_ENVVARS).

"MALLOC_CHECK_\0" \
"MALLOC_TOP_PAD_\0" \
"MALLOC_PERTURB_\0" \
"MALLOC_MMAP_THRESHOLD_\0" \
"MALLOC_TRIM_THRESHOLD_\0" \
"MALLOC_MMAP_MAX_\0" \
"MALLOC_ARENA_MAX\0" \
"MALLOC_ARENA_TEST\0"

int main(int argc, char **argv) {
ASSERT(argc >= 1);

int debug = getenv(wrapper_debug) != NULL;

// Drop insecure environment variables explicitly
//
// glibc does this automatically in SUID binaries, but we'd like to cover this:
//
// a) before it gets to glibc
// b) in binaries that are only granted ambient capabilities by the wrapper,
// but don't run with an altered effective UID/GID, nor directly gain
// capabilities themselves, and thus don't run in secure mode.
//
// We're using musl, which doesn't drop environment variables in secure mode,
delroth marked this conversation as resolved.
Show resolved Hide resolved
// and we'd also like glibc-specific variables to be covered.
//
// If we don't explicitly unset them, it's quite easy to just set LD_PRELOAD,
// have it passed through to the wrapped program, and gain privileges.
for (char *unsec = UNSECURE_ENVVARS_TUNABLES UNSECURE_ENVVARS; *unsec; unsec = strchr(unsec, 0) + 1) {
if (debug) {
fprintf(stderr, "unsetting %s\n", unsec);
}
unsetenv(unsec);
}

// 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!
Expand Down
4 changes: 2 additions & 2 deletions nixos/modules/security/wrappers/wrapper.nix
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ stdenv, linuxHeaders, sourceProg, debug ? false }:
{ stdenv, unsecvars, linuxHeaders, sourceProg, debug ? false }:
# For testing:
# $ nix-build -E 'with import <nixpkgs> {}; pkgs.callPackage ./wrapper.nix { parentWrapperDir = "/run/wrappers"; debug = true; }'
stdenv.mkDerivation {
Expand All @@ -16,6 +16,6 @@ stdenv.mkDerivation {
dontStrip = debug;
installPhase = ''
mkdir -p $out/bin
$CC $CFLAGS ${./wrapper.c} -o $out/bin/security-wrapper
$CC $CFLAGS ${./wrapper.c} -I${unsecvars} -o $out/bin/security-wrapper
'';
}