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

cc-wrapper: add support for shadowstack hardening flag #326819

Merged
merged 9 commits into from
Jul 28, 2024
10 changes: 10 additions & 0 deletions doc/stdenv/stdenv.chapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,16 @@ Adds the `-fPIE` compiler and `-pie` linker options. Position Independent Execut
Static libraries need to be compiled with `-fPIE` so that executables can link them in with the `-pie` linker option.
If the libraries lack `-fPIE`, you will get the error `recompile with -fPIE`.

#### `shadowstack` {#shadowstack}

Adds the `-fcf-protection=return` compiler option. This enables the Shadow Stack feature supported by some newer processors, which maintains a user-inaccessible copy of the program's stack containing only return-addresses. When returning from a function, the processor compares the return-address value on the two stacks and throws an error if they do not match, considering it a sign of corruption and possible tampering. This should significantly increase the difficulty of ROP attacks.

For the Shadow Stack to be enabled at runtime, all code linked into a process must be built with Shadow Stack enabled, so this is probably only useful to enable on a wide scale, so that all of a packages dependencies also have the feature enabled.

This is currently only supported on some newer Intel and AMD processors as part of the Intel CET set of features. However, the generated code should continue to work on older processors which will simply omit any of this checking.

This breaks some code that does advanced stack management or exception handling. If enabling this hardening flag it is important to test the result on a system that has known working and enabled CET support, so that any such breakage can be discovered.

#### `trivialautovarinit` {#trivialautovarinit}

Adds the `-ftrivial-auto-var-init=pattern` compiler option. This causes "trivially-initializable" uninitialized stack variables to be forcibly initialized with a nonzero value that is likely to cause a crash (and therefore be noticed). Uninitialized variables generally take on their values based on fragments of previous program state, and attackers can carefully manipulate that state to craft malicious initial values for these variables.
Expand Down
2 changes: 2 additions & 0 deletions nixos/doc/manual/release-notes/rl-2411.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,8 @@
- Nemo is now built with gtk-layer-shell support, note that for now it will be expected to see nemo-desktop
listed as a regular entry in Cinnamon Wayland session's window list applet.

- The `shadowstack` hardening flag has been added, though disabled by default.

- Support for *runner registration tokens* has been [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/380872)
in `gitlab-runner` 15.6 and is expected to be removed in `gitlab-runner` 18.0. Configuration of existing runners
should be changed to using *runner authentication tokens* by configuring
Expand Down
6 changes: 5 additions & 1 deletion pkgs/build-support/cc-wrapper/add-hardening.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ if [[ -n "${hardeningEnableMap[fortify3]-}" ]]; then
fi

if (( "${NIX_DEBUG:-0}" >= 1 )); then
declare -a allHardeningFlags=(fortify fortify3 stackprotector stackclashprotection pie pic strictoverflow format trivialautovarinit zerocallusedregs)
declare -a allHardeningFlags=(fortify fortify3 shadowstack stackprotector stackclashprotection pie pic strictoverflow format trivialautovarinit zerocallusedregs)
declare -A hardeningDisableMap=()

# Determine which flags were effectively disabled so we can report below.
Expand Down Expand Up @@ -75,6 +75,10 @@ for flag in "${!hardeningEnableMap[@]}"; do
;;
esac
;;
shadowstack)
if (( "${NIX_DEBUG:-0}" >= 1 )); then echo HARDENING: enabling shadowstack >&2; fi
hardeningCFlagsBefore+=('-fcf-protection=return')
;;
stackprotector)
if (( "${NIX_DEBUG:-0}" >= 1 )); then echo HARDENING: enabling stackprotector >&2; fi
hardeningCFlagsBefore+=('-fstack-protector-strong' '--param' 'ssp-buffer-size=4')
Expand Down
6 changes: 6 additions & 0 deletions pkgs/development/compilers/gcc/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,12 @@ pipe ((callFile ./common/builder.nix {}) ({
) "stackclashprotection"
++ optional (!atLeast11) "zerocallusedregs"
++ optionals (!atLeast12) [ "fortify3" "trivialautovarinit" ]
++ optional (!(
atLeast8
&& targetPlatform.isLinux
&& targetPlatform.isx86_64
&& targetPlatform.libc == "glibc"
)) "shadowstack"
++ optionals (langFortran) [ "fortify" "format" ];
};

Expand Down
5 changes: 5 additions & 0 deletions pkgs/development/compilers/llvm/common/clang/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ let
isClang = true;
hardeningUnsupportedFlagsByTargetPlatform = targetPlatform:
[ "fortify3" ]
++ lib.optional (
(lib.versionOlder release_version "7")
|| !targetPlatform.isLinux
|| !targetPlatform.isx86_64
) "shadowstack"
++ lib.optional (
(lib.versionOlder release_version "11")
|| (targetPlatform.isAarch64 && (lib.versionOlder release_version "18.1"))
Expand Down
1 change: 1 addition & 0 deletions pkgs/development/compilers/llvm/common/llvm/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ stdenv.mkDerivation (rec {

hardeningDisable = [
"trivialautovarinit"
"shadowstack"
];

nativeBuildInputs = [ cmake ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
Revert 55d63e731253de82e96ed4ddca2e294076cd0bc5

--- b/sysdeps/x86/cpu-features.c
+++ a/sysdeps/x86/cpu-features.c
@@ -110,7 +110,7 @@
if (!CPU_FEATURES_CPU_P (cpu_features, RTM_ALWAYS_ABORT))
CPU_FEATURE_SET_ACTIVE (cpu_features, RTM);

+#if CET_ENABLED
-#if CET_ENABLED && 0
CPU_FEATURE_SET_ACTIVE (cpu_features, IBT);
CPU_FEATURE_SET_ACTIVE (cpu_features, SHSTK);
#endif
reverted:
--- b/sysdeps/x86/cpu-tunables.c
+++ a/sysdeps/x86/cpu-tunables.c
@@ -35,17 +35,6 @@
break; \
}

-#define CHECK_GLIBC_IFUNC_CPU_BOTH(f, cpu_features, name, len) \
- _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (tunable_str_comma_strcmp_cte (&f, #name)) \
- { \
- if (f.disable) \
- CPU_FEATURE_UNSET (cpu_features, name) \
- else \
- CPU_FEATURE_SET_ACTIVE (cpu_features, name) \
- break; \
- }
-
/* Disable a preferred feature NAME. We don't enable a preferred feature
which isn't available. */
#define CHECK_GLIBC_IFUNC_PREFERRED_OFF(f, cpu_features, name, len) \
@@ -142,13 +131,11 @@
}
break;
case 5:
- {
- CHECK_GLIBC_IFUNC_CPU_BOTH (n, cpu_features, SHSTK, 5);
- }
if (n.disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, LZCNT, 5);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, MOVBE, 5);
+ CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SHSTK, 5);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSSE3, 5);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, XSAVE, 5);
}
5 changes: 4 additions & 1 deletion pkgs/development/libraries/glibc/common.nix
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
, profilingLibraries ? false
, withGd ? false
, enableCET ? false
, enableCETRuntimeDefault ? false
, extraBuildInputs ? []
, extraNativeBuildInputs ? []
, ...
Expand All @@ -50,6 +51,7 @@ in

assert withLinuxHeaders -> linuxHeaders != null;
assert withGd -> gd != null && libpng != null;
assert enableCET == false -> !enableCETRuntimeDefault;

stdenv.mkDerivation ({
version = version + patchSuffix;
Expand Down Expand Up @@ -114,7 +116,8 @@ stdenv.mkDerivation ({
lib.optional (isAarch64 && isLinux) ./0001-aarch64-math-vector.h-add-NVCC-include-guard.patch
)
++ lib.optional stdenv.hostPlatform.isMusl ./fix-rpc-types-musl-conflicts.patch
++ lib.optional stdenv.buildPlatform.isDarwin ./darwin-cross-build.patch;
++ lib.optional stdenv.buildPlatform.isDarwin ./darwin-cross-build.patch
++ lib.optional enableCETRuntimeDefault ./2.39-revert-cet-default-disable.patch;

postPatch =
''
Expand Down
3 changes: 2 additions & 1 deletion pkgs/development/libraries/glibc/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
, profilingLibraries ? false
, withGd ? false
, enableCET ? if stdenv.hostPlatform.isx86_64 then "permissive" else false
, enableCETRuntimeDefault ? false
, pkgsBuildBuild
, libgcc
}:
Expand All @@ -16,7 +17,7 @@ let
in

(callPackage ./common.nix { inherit stdenv; } {
inherit withLinuxHeaders withGd profilingLibraries enableCET;
inherit withLinuxHeaders withGd profilingLibraries enableCET enableCETRuntimeDefault;
pname = "glibc" + lib.optionalString withGd "-gd" + lib.optionalString (stdenv.cc.isGNU && libgcc==null) "-nolibgcc";
}).overrideAttrs(previousAttrs: {

Expand Down
9 changes: 6 additions & 3 deletions pkgs/development/libraries/pcre/default.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{ lib, stdenv, fetchurl, fetchpatch
, pcre, windows ? null
# Disable jit on Apple Silicon, https://github.com/zherczeg/sljit/issues/51
, enableJit ? !(stdenv.hostPlatform.isDarwin && stdenv.hostPlatform.isAarch64)
, variant ? null
}:

Expand All @@ -18,11 +20,12 @@ stdenv.mkDerivation rec {

outputs = [ "bin" "dev" "out" "doc" "man" ];

# Disable jit on Apple Silicon, https://github.com/zherczeg/sljit/issues/51
configureFlags = lib.optional (!(stdenv.hostPlatform.isDarwin && stdenv.hostPlatform.isAarch64)) "--enable-jit=auto" ++ [
hardeningDisable = lib.optional enableJit "shadowstack";

configureFlags = [
"--enable-unicode-properties"
"--disable-cpp"
]
] ++ lib.optional enableJit "--enable-jit=auto"
++ lib.optional (variant != null) "--enable-${variant}";

patches = [
Expand Down
1 change: 1 addition & 0 deletions pkgs/stdenv/darwin/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ in
isFromBootstrapFiles = true;
hardeningUnsupportedFlags = [
"fortify3"
"shadowstack"
"stackclashprotection"
"zerocallusedregs"
];
Expand Down
1 change: 1 addition & 0 deletions pkgs/stdenv/generic/make-derivation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ let
"format"
"fortify"
"fortify3"
"shadowstack"
"pic"
"pie"
"relro"
Expand Down
8 changes: 7 additions & 1 deletion pkgs/stdenv/linux/bootstrap-tools-musl/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,11 @@ derivation ({
langC = true;
langCC = true;
isGNU = true;
hardeningUnsupportedFlags = [ "fortify3" "zerocallusedregs" "trivialautovarinit" ];
hardeningUnsupportedFlags = [
"fortify3"
"shadowstack"
"stackclashprotection"
"trivialautovarinit"
"zerocallusedregs"
];
} // extraAttrs)
1 change: 1 addition & 0 deletions pkgs/stdenv/linux/bootstrap-tools/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ derivation ({
isGNU = true;
hardeningUnsupportedFlags = [
"fortify3"
"shadowstack"
"stackclashprotection"
"trivialautovarinit"
"zerocallusedregs"
Expand Down
7 changes: 5 additions & 2 deletions pkgs/tools/package-management/lix/common.nix
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,11 @@ stdenv.mkDerivation {
meson test --no-rebuild "''${flagsArray[@]}"
runHook postInstallCheck
'';
# strictoverflow is disabled because we trap on signed overflow instead
hardeningDisable = [ "strictoverflow" ] ++ lib.optional stdenv.hostPlatform.isStatic "pie";
hardeningDisable = [
"shadowstack"
# strictoverflow is disabled because we trap on signed overflow instead
"strictoverflow"
] ++ lib.optional stdenv.hostPlatform.isStatic "pie";
# hardeningEnable = lib.optionals (!stdenv.isDarwin) [ "pie" ];
# hardeningDisable = lib.optional stdenv.hostPlatform.isMusl "fortify";
separateDebugInfo = stdenv.isLinux && !enableStatic;
Expand Down
4 changes: 3 additions & 1 deletion pkgs/tools/package-management/nix/common.nix
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ self = stdenv.mkDerivation {

hardeningEnable = lib.optionals (!stdenv.isDarwin) [ "pie" ];

hardeningDisable = lib.optional stdenv.hostPlatform.isMusl "fortify";
hardeningDisable = [
"shadowstack"
] ++ lib.optional stdenv.hostPlatform.isMusl "fortify";

nativeBuildInputs = [
pkg-config
Expand Down
10 changes: 10 additions & 0 deletions pkgs/top-level/stage.nix
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,20 @@ let
pkgsExtraHardening = super';
stdenv = super'.withDefaultHardeningFlags (
super'.stdenv.cc.defaultHardeningFlags ++ [
"shadowstack"
"stackclashprotection"
"trivialautovarinit"
]
) super'.stdenv;
glibc = super'.glibc.override rec {
enableCET = if self'.stdenv.hostPlatform.isx86_64 then "permissive" else false;
enableCETRuntimeDefault = enableCET != false;
};
} // lib.optionalAttrs (with super'.stdenv.hostPlatform; isx86_64 && isLinux) {
# causes shadowstack disablement
pcre = super'.pcre.override { enableJit = false; };
pcre-cpp = super'.pcre-cpp.override { enableJit = false; };
pcre16 = super'.pcre16.override { enableJit = false; };
Comment on lines +305 to +308
Copy link
Member

Choose a reason for hiding this comment

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

These are the first packages that pkgsExtraHardening is making an explicit domain‐specific hardening vs. functionality/performance trade‐off for, rather than just blanket‐defaulting a hardening switch on and letting individual packages decide what to do, right?

I’m not opposed necessarily, just want to check that this is something new that’s happening. I do worry a little about bit rot as package options drift without anyone checking pkgsExtraHardening. And it might be hard to make judgement calls in future when you’re faced with whether to turn all JITs ever off for security reasons.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this answers the question, but here I think is the starting point that lead to this: #326819 (comment) . TLDR: legacy PCRE JIT is broken with shadow stack, so either JIT or shadow stack have to be disabled, and the latter has cascade effects.

Copy link
Member

Choose a reason for hiding this comment

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

Right, okay. The fact that it would require disabling it for all the downstream packages makes this make sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is the first time we're overriding an actual package.

})
] ++ overlays;
};
Expand Down