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

security.wrappers: init #890

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions modules/module-list.nix
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
./security/pki
./security/sandbox
./security/sudo.nix
./security/wrappers
./system
./system/base.nix
./system/checks.nix
Expand Down
201 changes: 201 additions & 0 deletions modules/security/wrappers/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
{ config, lib, pkgs, ... }:

let
cfg = config.security;

parentWrapperDir = dirOf cfg.wrapperDir;

wrapperType = lib.types.submodule ({ name, config, ... }: {
options = with lib; {
source = mkOption {
type = types.path;
description = "The absolute path to the program to be wrapped.";
};
program = mkOption {
type = with types; nullOr str;
default = name;
description = "The name of the wrapper program. Defaults to the attribute name.";
};
owner = mkOption {
type = types.str;
description = "The owner of the wrapper program.";
};
group = mkOption {
type = types.str;
description = "The group of the wrapper program.";
};
permissions = mkOption {
type = types.str;
default = "u+rx,g+x,o+x";
description = "The permissions to set on the wrapper.";
};
setuid = mkOption {
type = types.bool;
default = false;
description = "Whether to add the setuid bit to the wrapper program.";
};
setgid = mkOption {
type = types.bool;
default = false;
description = "Whether to add the setgid bit to the wrapper program.";
};
# codesign = mkOption {
# type = types.bool;
# default = false;
# description = "Whether to codesign the wrapper program.";
# };
};
});

mkWrappedPrograms =
builtins.map
(opts: mkWrapper opts)
(builtins.attrValues cfg.wrappers);

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
# '';
# });
};

mkWrapper =
{ program
, source
, owner
, group
, permissions
, setuid
, setgid
, codesign ? false
, ...
}:
let
codesigned = if codesign
then ''
# codesign ${source} to "$wrapperDir/${program}" INSTEAD OF the next line
cp ${securityWrapper source}/bin/security-wrapper "$wrapperDir/${program}"
''
else ''
cp ${securityWrapper source}/bin/security-wrapper "$wrapperDir/${program}"
'';
in
''
${codesigned}

# Prevent races
chmod 0000 "$wrapperDir/${program}"
chown ${owner}:${group} "$wrapperDir/${program}"

chmod "u${if setuid then "+" else "-"}s,g${if setgid then "+" else "-"}s,${permissions}" "$wrapperDir/${program}"
'';
in
{
# probably not necessary since these options never existed in nix-darwin?
imports = [
(lib.mkRemovedOptionModule [ "security" "setuidOwners" ] "Use security.wrappers instead")
(lib.mkRemovedOptionModule [ "security" "setuidPrograms" ] "Use security.wrappers instead")
];

meta.maintainers = [
lib.maintainers.samasaur or "samasaur"
];

###### interface
options.security = {
wrappers = lib.mkOption {
type = lib.types.attrsOf wrapperType;
default = {};
example = lib.literalExpression
''
{
# a setuid root program
doas =
{ setuid = true;
owner = "root";
group = "wheel";
source = "''${pkgs.doas}/bin/doas";
};


# a setgid program
locate =
{ setgid = true;
owner = "root";
group = "mlocate";
source = "''${pkgs.locate}/bin/locate";
};
}
'';
description = ''
This option effectively allows adding setuid/setgid bits and/or changing
file ownership and permissions without directly modifying it. This works
by creating a wrapper program under the {option}`security.wrapperDir`
directory, which is then added to the shell `PATH`.
'';
};
wrapperDir = lib.mkOption {
type = lib.types.path;
default = "/run/wrappers/bin";
internal = true;
description = ''
This option defines the path to the wrapper programs. It
should not be overridden.
'';
};
# codesignIdentity = lib.mkOption {
# type = lib.types.str;
# default = "-";
# description = "Identity to use for codesigning.";
# };
};

###### implementation
config = {
assertions = [
{ assertion = cfg.wrappers != {} -> config.services.activate-system.enable; message = "security.wrappers requires services.activate-system because `/run` is wiped on boot"; }
];

environment.extraInit = ''
# Wrappers override other bin directories.
export PATH="${cfg.wrapperDir}:$PATH"
'';

system.activationScripts.wrappers.text = ''
echo "setting up wrappers..." >&2
if ! test -e /run/wrappers; then mkdir /run/wrappers; fi

# We want to place the tmpdirs for the wrappers to the parent dir.
wrapperDir=$(mktemp --directory --tmpdir="${parentWrapperDir}" wrappers.XXXXXXXXXX)
chmod a+rx $wrapperDir

${builtins.concatStringsSep "\n" mkWrappedPrograms}

if test -L ${cfg.wrapperDir}; then
# Atomically replace the symlink
# See https://axialcorps.com/2013/07/03/atomically-replacing-files-and-directories/
old=$(readlink -f ${cfg.wrapperDir})
ln --symbolic --force --no-dereference $wrapperDir ${cfg.wrapperDir}-tmp
mv --no-target-directory ${cfg.wrapperDir}-tmp ${cfg.wrapperDir}
rm --force --recursive $old
else
# For initial setup
ln --symbolic $wrapperDir ${cfg.wrapperDir}
fi
'';
};
}
128 changes: 128 additions & 0 deletions modules/security/wrappers/wrapper.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
#define _GNU_SOURCE
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <stdnoreturn.h>
#include <sys/errno.h>
// #include <sys/types.h>
// #include <sys/stat.h>
// #include <sys/xattr.h>
// #include <fcntl.h>
// #include <dirent.h>
// #include <errno.h>
// #include <sys/prctl.h>
// #include <limits.h>
// #include <stdint.h>
// #include <syscall.h>
// #include <byteswap.h>

// imported from glibc
// #include "unsecvars.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))

extern char **environ;

// Wrapper debug variable name
static char *wrapper_debug = "WRAPPER_DEBUG";

static noreturn void assert_failure(const char *assertion) {
fprintf(stderr, "Assertion `%s` in NixOS's wrapper.c failed.\n", assertion);
fflush(stderr);
abort();
}

// 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 \
"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"
Comment on lines +41 to +62
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if any of these apply to macOS.


#define UNSECURE_ENVVARS \
"GCONV_PATH\0" \
"GETCONF_DIR\0" \
"HOSTALIASES\0" \
"LD_AUDIT\0" \
"LD_DEBUG\0" \
"LD_DEBUG_OUTPUT\0" \
"LD_DYNAMIC_WEAK\0" \
"LD_HWCAP_MASK\0" \
"LD_LIBRARY_PATH\0" \
"LD_ORIGIN_PATH\0" \
"LD_PRELOAD\0" \
"LD_PROFILE\0" \
"LD_SHOW_AUXV\0" \
"LD_USE_LOAD_BIAS\0" \
Comment on lines +68 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are specific to the Linux dynamic loader interface – meaning that their Darwin DYLD_* equivalents are currently not sanitized at all. I fear that this means the current version of the wrapper is likely vulnerable.

I suspect that there are other Linux‐specific variables still in this list that I’m less confident about, and almost surely Darwin‐specific ones that we’re missing. That makes me really uneasy.

"LOCALDOMAIN\0" \
"LOCPATH\0" \
"MALLOC_TRACE\0" \
"NIS_PATH\0" \
"NLSPATH\0" \
"RESOLV_HOST_CONF\0" \
"RES_OPTIONS\0" \
"TMPDIR\0" \
// GLIBC_TUNABLES_ENVVAR \

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

// argv[0] goes into a lot of places, to a far greater degree than other elements
// of argv. glibc has had buffer overflows relating to argv[0], eg CVE-2023-6246.
// Since we expect the wrappers to be invoked from either $PATH or /run/wrappers/bin,
// there should be no reason to pass any particularly large values here, so we can
// be strict for strictness' sake.
ASSERT(strlen(argv[0]) < 512);

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:
Copy link
Collaborator

Choose a reason for hiding this comment

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

It’s possible that Darwin’s dynamic loader also does this, in which case we could sleep a lot easier about all this. It would be good if someone could investigate that.

//
// 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.
Comment on lines +106 to +108
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not relevant to us, except maybe for codesigning entitlements.

//
// We're using musl, which doesn't drop environment variables in secure mode,
// and we'd also like glibc-specific variables to be covered.
Comment on lines +110 to +111
Copy link
Collaborator

Choose a reason for hiding this comment

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

This (and other glibc‐related comments) would need to be updated.

//
// 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);
}

execve(SOURCE_PROG, argv, environ);

fprintf(stderr, "%s: cannot run `%s': %s\n",
argv[0], SOURCE_PROG, strerror(errno));

return 1;
}
19 changes: 19 additions & 0 deletions modules/security/wrappers/wrapper.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{ stdenv, sourceProg, debug ? false }:
# For testing:
# $ nix-build -E 'with import <nixpkgs> {}; pkgs.callPackage ./wrapper.nix { sourceProg = "${pkgs.hello}/bin/hello"; debug = true; }'
stdenv.mkDerivation {
name = "security-wrapper-${baseNameOf sourceProg}";
dontUnpack = true;
CFLAGS = [
''-DSOURCE_PROG="${sourceProg}"''
] ++ (if debug then [
"-Werror" "-Og" "-g"
] else [
"-Wall" "-O2"
]);
dontStrip = debug;
installPhase = ''
mkdir -p $out/bin
$CC $CFLAGS ${./wrapper.c} -o $out/bin/security-wrapper
'';
}
1 change: 1 addition & 0 deletions modules/services/activate-system/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ in
${config.system.activationScripts.etcChecks.text}
${config.system.activationScripts.etc.text}
${config.system.activationScripts.keyboard.text}
${config.system.activationScripts.wrappers.text}
'';
serviceConfig.RunAtLoad = true;
serviceConfig.KeepAlive.SuccessfulExit = false;
Expand Down
18 changes: 5 additions & 13 deletions modules/services/karabiner-elements/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,11 @@ in
serviceConfig.RunAtLoad = true;
};

# We need this to run every reboot as /run gets nuked so we can't put this
# inside the preActivation script as it only gets run on darwin-rebuild switch.
launchd.daemons.setsuid_karabiner_session_monitor = {
serviceConfig.ProgramArguments = [
"/bin/sh" "-c"
"/bin/wait4path /nix/store &amp;&amp; ${pkgs.writeScript "setsuid_karabiner_session_monitor" ''
rm -rf /run/wrappers
mkdir -p /run/wrappers/bin
install -m4555 "${pkgs.karabiner-elements}/Library/Application Support/org.pqrs/Karabiner-Elements/bin/karabiner_session_monitor" /run/wrappers/bin
''}"
];
serviceConfig.RunAtLoad = true;
serviceConfig.KeepAlive.SuccessfulExit = false;
security.wrappers.karabiner_session_monitor = {
setuid = true;
owner = "root";
group = "wheel";
source = "${pkgs.karabiner-elements}/Library/Application Support/org.pqrs/Karabiner-Elements/bin/karabiner_session_monitor";
};

launchd.user.agents.karabiner_session_monitor = {
Expand Down
1 change: 1 addition & 0 deletions modules/system/activation-scripts.nix
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ in
${cfg.activationScripts.keyboard.text}
${cfg.activationScripts.fonts.text}
${cfg.activationScripts.nvram.text}
${cfg.activationScripts.wrappers.text}

${cfg.activationScripts.postActivation.text}

Expand Down
Loading
Loading