From ec8d29ab82a4e664abd900898ea5d0afe4b36f7c Mon Sep 17 00:00:00 2001 From: Robert Scott Date: Sun, 3 Sep 2023 19:59:13 +0100 Subject: [PATCH 1/3] cc-wrapper hardeningFlags tests: fix expected behaviour in corner cases also use fortify1-based tests in some places that it may allow us to better test the behaviour of toolchains that only support that --- pkgs/test/cc-wrapper/hardening.nix | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkgs/test/cc-wrapper/hardening.nix b/pkgs/test/cc-wrapper/hardening.nix index 41ddaefdfea8d..ed7eef7250542 100644 --- a/pkgs/test/cc-wrapper/hardening.nix +++ b/pkgs/test/cc-wrapper/hardening.nix @@ -237,13 +237,14 @@ in nameDrvAfterAttrName ({ expectFailure = true; }; - fortify3StdenvUnsuppDoesntUnsuppFortify = brokenIf stdenv.hostPlatform.isMusl (checkTestBin (f2exampleWithStdEnv (stdenvUnsupport ["fortify3"]) { + # musl implementation undetectable by this means even if present + fortify3StdenvUnsuppDoesntUnsuppFortify1 = brokenIf stdenv.hostPlatform.isMusl (checkTestBin (f1exampleWithStdEnv (stdenvUnsupport ["fortify3"]) { hardeningEnable = [ "fortify" ]; }) { ignoreFortify = false; }); - fortify3StdenvUnsuppDoesntUnsuppFortifyExecTest = fortifyExecTest (f2exampleWithStdEnv (stdenvUnsupport ["fortify3"]) { + fortify3StdenvUnsuppDoesntUnsuppFortify1ExecTest = fortifyExecTest (f1exampleWithStdEnv (stdenvUnsupport ["fortify3"]) { hardeningEnable = [ "fortify" ]; }); @@ -285,7 +286,8 @@ in nameDrvAfterAttrName ({ expectFailure = true; }; - fortify3EnabledEnvEnablesFortify = brokenIf stdenv.hostPlatform.isMusl (checkTestBin (f2exampleWithStdEnv stdenv { + # musl implementation undetectable by this means even if present + fortify3EnabledEnvEnablesFortify1 = brokenIf stdenv.hostPlatform.isMusl (checkTestBin (f1exampleWithStdEnv stdenv { hardeningDisable = [ "fortify" "fortify3" ]; preBuild = '' export NIX_HARDENING_ENABLE="fortify3" @@ -294,7 +296,7 @@ in nameDrvAfterAttrName ({ ignoreFortify = false; }); - fortify3EnabledEnvEnablesFortifyExecTest = fortifyExecTest (f2exampleWithStdEnv stdenv { + fortify3EnabledEnvEnablesFortify1ExecTest = fortifyExecTest (f1exampleWithStdEnv stdenv { hardeningDisable = [ "fortify" "fortify3" ]; preBuild = '' export NIX_HARDENING_ENABLE="fortify3" @@ -312,7 +314,6 @@ in nameDrvAfterAttrName ({ }; # NIX_HARDENING_ENABLE can't enable an unsupported feature - stackProtectorUnsupportedEnabledEnv = checkTestBin (f2exampleWithStdEnv (stdenvUnsupport ["stackprotector"]) { preBuild = '' export NIX_HARDENING_ENABLE="stackprotector" @@ -322,6 +323,9 @@ in nameDrvAfterAttrName ({ expectFailure = true; }; + # current implementation prevents the command-line from disabling + # fortify if cc-wrapper is enabling it. + # undetectable by this means on static even if present fortify1ExplicitEnabledCmdlineDisabled = brokenIf stdenv.hostPlatform.isStatic (checkTestBin (f1exampleWithStdEnv stdenv { hardeningEnable = [ "fortify" ]; @@ -330,9 +334,12 @@ in nameDrvAfterAttrName ({ ''; }) { ignoreFortify = false; - expectFailure = true; + expectFailure = false; }); + # current implementation doesn't force-disable fortify if + # command-line enables it even if we use hardeningDisable. + # musl implementation undetectable by this means even if present fortify1ExplicitDisabledCmdlineEnabled = brokenIf ( stdenv.hostPlatform.isMusl || stdenv.hostPlatform.isStatic From 38b580b21abf4182681dd6f6aa068acfa02da8d9 Mon Sep 17 00:00:00 2001 From: Robert Scott Date: Sat, 13 Jul 2024 18:16:29 +0100 Subject: [PATCH 2/3] cc-wrapper hardeningFlags tests: add tests for stackclashprotection --- pkgs/test/cc-wrapper/hardening.nix | 100 ++++++++++++++++++++++------- 1 file changed, 77 insertions(+), 23 deletions(-) diff --git a/pkgs/test/cc-wrapper/hardening.nix b/pkgs/test/cc-wrapper/hardening.nix index ed7eef7250542..f8cd3c08ccdfe 100644 --- a/pkgs/test/cc-wrapper/hardening.nix +++ b/pkgs/test/cc-wrapper/hardening.nix @@ -3,6 +3,7 @@ , runCommand , runCommandWith , runCommandCC +, hello , debian-devscripts }: @@ -18,6 +19,7 @@ let allowSubstitutes = false; } // env; } '' + [ -n "$postConfigure" ] && eval "$postConfigure" [ -n "$preBuild" ] && eval "$preBuild" n=$out/bin/test-bin mkdir -p "$(dirname "$n")" @@ -29,6 +31,17 @@ let f2exampleWithStdEnv = writeCBinWithStdenv ./fortify2-example.c; f3exampleWithStdEnv = writeCBinWithStdenv ./fortify3-example.c; + # for when we need a slightly more complicated program + helloWithStdEnv = stdenv': env: (hello.override { stdenv = stdenv'; }).overrideAttrs ({ + preBuild = '' + export CFLAGS="$TEST_EXTRA_FLAGS" + ''; + NIX_DEBUG = "1"; + postFixup = '' + cp $out/bin/hello $out/bin/test-bin + ''; + } // env); + stdenvUnsupport = additionalUnsupported: stdenv.override { cc = stdenv.cc.override { cc = (lib.extendDerivation true { @@ -45,24 +58,39 @@ let ignorePie ? true, ignoreRelRO ? true, ignoreStackProtector ? true, + ignoreStackClashProtection ? true, expectFailure ? false, }: let + stackClashStr = "Stack clash protection: yes"; expectFailureClause = lib.optionalString expectFailure - " && echo 'ERROR: Expected hardening-check to fail, but it passed!' >&2 && exit 1"; + " && echo 'ERROR: Expected hardening-check to fail, but it passed!' >&2 && false"; in runCommandCC "check-test-bin" { nativeBuildInputs = [ debian-devscripts ]; buildInputs = [ testBin ]; - meta.platforms = lib.platforms.linux; # ELF-reliant - } '' - hardening-check --nocfprotection \ - ${lib.optionalString ignoreBindNow "--nobindnow"} \ - ${lib.optionalString ignoreFortify "--nofortify"} \ - ${lib.optionalString ignorePie "--nopie"} \ - ${lib.optionalString ignoreRelRO "--norelro"} \ - ${lib.optionalString ignoreStackProtector "--nostackprotector"} \ - $(PATH=$HOST_PATH type -P test-bin) ${expectFailureClause} - touch $out - ''; + meta.platforms = if ignoreStackClashProtection + then lib.platforms.linux # ELF-reliant + else [ "x86_64-linux" ]; # stackclashprotection test looks for x86-specific instructions + } ('' + if ${lib.optionalString (!expectFailure) "!"} { + hardening-check --nocfprotection \ + ${lib.optionalString ignoreBindNow "--nobindnow"} \ + ${lib.optionalString ignoreFortify "--nofortify"} \ + ${lib.optionalString ignorePie "--nopie"} \ + ${lib.optionalString ignoreRelRO "--norelro"} \ + ${lib.optionalString ignoreStackProtector "--nostackprotector"} \ + $(PATH=$HOST_PATH type -P test-bin) | tee $out + '' + lib.optionalString (!ignoreStackClashProtection) '' + # stack clash protection doesn't actually affect the exit code of + # hardening-check (likely authors think false negatives too common) + { grep -F '${stackClashStr}' $out || { echo "Didn't find '${stackClashStr}' in output" && false ;} ;} + '' + '' + } ; then + '' + lib.optionalString expectFailure '' + echo 'ERROR: Expected hardening-check to fail, but it passed!' >&2 + '' + '' + exit 2 + fi + ''); nameDrvAfterAttrName = builtins.mapAttrs (name: drv: drv.overrideAttrs (_: { name = "test-${name}"; }) @@ -151,6 +179,13 @@ in nameDrvAfterAttrName ({ ignoreStackProtector = false; }); + # protection patterns generated by clang not detectable? + stackClashProtectionExplicitEnabled = brokenIf stdenv.cc.isClang (checkTestBin (helloWithStdEnv stdenv { + hardeningEnable = [ "stackclashprotection" ]; + }) { + ignoreStackClashProtection = false; + }); + bindNowExplicitDisabled = checkTestBin (f2exampleWithStdEnv stdenv { hardeningDisable = [ "bindnow" ]; }) { @@ -211,6 +246,13 @@ in nameDrvAfterAttrName ({ expectFailure = true; }; + stackClashProtectionExplicitDisabled = checkTestBin (helloWithStdEnv stdenv { + hardeningDisable = [ "stackclashprotection" ]; + }) { + ignoreStackClashProtection = false; + expectFailure = true; + }; + # most flags can't be "unsupported" by compiler alone and # binutils doesn't have an accessible hardeningUnsupportedFlags # mechanism, so can only test a couple of flags through altered @@ -255,12 +297,19 @@ in nameDrvAfterAttrName ({ expectFailure = true; }; + stackClashProtectionStdenvUnsupp = checkTestBin (helloWithStdEnv (stdenvUnsupport ["stackclashprotection"]) { + hardeningEnable = [ "stackclashprotection" ]; + }) { + ignoreStackClashProtection = false; + expectFailure = true; + }; + # NIX_HARDENING_ENABLE set in the shell overrides hardeningDisable # and hardeningEnable stackProtectorReenabledEnv = checkTestBin (f2exampleWithStdEnv stdenv { hardeningDisable = [ "stackprotector" ]; - preBuild = '' + postConfigure = '' export NIX_HARDENING_ENABLE="stackprotector" ''; }) { @@ -269,7 +318,7 @@ in nameDrvAfterAttrName ({ stackProtectorReenabledFromAllEnv = checkTestBin (f2exampleWithStdEnv stdenv { hardeningDisable = [ "all" ]; - preBuild = '' + postConfigure = '' export NIX_HARDENING_ENABLE="stackprotector" ''; }) { @@ -278,7 +327,7 @@ in nameDrvAfterAttrName ({ stackProtectorRedisabledEnv = checkTestBin (f2exampleWithStdEnv stdenv { hardeningEnable = [ "stackprotector" ]; - preBuild = '' + postConfigure = '' export NIX_HARDENING_ENABLE="" ''; }) { @@ -289,7 +338,7 @@ in nameDrvAfterAttrName ({ # musl implementation undetectable by this means even if present fortify3EnabledEnvEnablesFortify1 = brokenIf stdenv.hostPlatform.isMusl (checkTestBin (f1exampleWithStdEnv stdenv { hardeningDisable = [ "fortify" "fortify3" ]; - preBuild = '' + postConfigure = '' export NIX_HARDENING_ENABLE="fortify3" ''; }) { @@ -298,14 +347,14 @@ in nameDrvAfterAttrName ({ fortify3EnabledEnvEnablesFortify1ExecTest = fortifyExecTest (f1exampleWithStdEnv stdenv { hardeningDisable = [ "fortify" "fortify3" ]; - preBuild = '' + postConfigure = '' export NIX_HARDENING_ENABLE="fortify3" ''; }); fortifyEnabledEnvDoesntEnableFortify3 = checkTestBin (f3exampleWithStdEnv stdenv { hardeningDisable = [ "fortify" "fortify3" ]; - preBuild = '' + postConfigure = '' export NIX_HARDENING_ENABLE="fortify" ''; }) { @@ -315,7 +364,7 @@ in nameDrvAfterAttrName ({ # NIX_HARDENING_ENABLE can't enable an unsupported feature stackProtectorUnsupportedEnabledEnv = checkTestBin (f2exampleWithStdEnv (stdenvUnsupport ["stackprotector"]) { - preBuild = '' + postConfigure = '' export NIX_HARDENING_ENABLE="stackprotector" ''; }) { @@ -329,7 +378,7 @@ in nameDrvAfterAttrName ({ # undetectable by this means on static even if present fortify1ExplicitEnabledCmdlineDisabled = brokenIf stdenv.hostPlatform.isStatic (checkTestBin (f1exampleWithStdEnv stdenv { hardeningEnable = [ "fortify" ]; - preBuild = '' + postConfigure = '' export TEST_EXTRA_FLAGS='-D_FORTIFY_SOURCE=0' ''; }) { @@ -345,7 +394,7 @@ in nameDrvAfterAttrName ({ stdenv.hostPlatform.isMusl || stdenv.hostPlatform.isStatic ) (checkTestBin (f1exampleWithStdEnv stdenv { hardeningDisable = [ "fortify" ]; - preBuild = '' + postConfigure = '' export TEST_EXTRA_FLAGS='-D_FORTIFY_SOURCE=1' ''; }) { @@ -354,14 +403,14 @@ in nameDrvAfterAttrName ({ fortify1ExplicitDisabledCmdlineEnabledExecTest = fortifyExecTest (f1exampleWithStdEnv stdenv { hardeningDisable = [ "fortify" ]; - preBuild = '' + postConfigure = '' export TEST_EXTRA_FLAGS='-D_FORTIFY_SOURCE=1' ''; }); fortify1ExplicitEnabledCmdlineDisabledNoWarn = f1exampleWithStdEnv stdenv { hardeningEnable = [ "fortify" ]; - preBuild = '' + postConfigure = '' export TEST_EXTRA_FLAGS='-D_FORTIFY_SOURCE=0 -Werror' ''; }; @@ -400,4 +449,9 @@ in { ignoreStackProtector = false; expectFailure = true; }; + + allExplicitDisabledStackClashProtection = checkTestBin tb { + ignoreStackClashProtection = false; + expectFailure = true; + }; })) From 2e0d7e230a022096bb4d9c744f636f417ad71c84 Mon Sep 17 00:00:00 2001 From: Robert Scott Date: Sat, 13 Jul 2024 23:47:23 +0100 Subject: [PATCH 3/3] cc-wrapper hardeningFlags tests: fix stdenvUnsupport-based tests these were not updated to understand hardeningUnsupportedFlagsByTargetPlatform when it was added causing more tests to fail for clang than otherwise would --- pkgs/test/cc-wrapper/hardening.nix | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/pkgs/test/cc-wrapper/hardening.nix b/pkgs/test/cc-wrapper/hardening.nix index f8cd3c08ccdfe..270e9a2e87616 100644 --- a/pkgs/test/cc-wrapper/hardening.nix +++ b/pkgs/test/cc-wrapper/hardening.nix @@ -44,8 +44,19 @@ let stdenvUnsupport = additionalUnsupported: stdenv.override { cc = stdenv.cc.override { - cc = (lib.extendDerivation true { - hardeningUnsupportedFlags = (stdenv.cc.cc.hardeningUnsupportedFlags or []) ++ additionalUnsupported; + cc = (lib.extendDerivation true rec { + # this is ugly - have to cross-reference from + # hardeningUnsupportedFlagsByTargetPlatform to hardeningUnsupportedFlags + # because the finalAttrs mechanism that hardeningUnsupportedFlagsByTargetPlatform + # implementations use to do this won't work with lib.extendDerivation. + # but it's simplified by the fact that targetPlatform is already fixed + # at this point. + hardeningUnsupportedFlagsByTargetPlatform = _: hardeningUnsupportedFlags; + hardeningUnsupportedFlags = ( + if stdenv.cc.cc ? hardeningUnsupportedFlagsByTargetPlatform + then stdenv.cc.cc.hardeningUnsupportedFlagsByTargetPlatform stdenv.targetPlatform + else (stdenv.cc.cc.hardeningUnsupportedFlags or []) + ) ++ additionalUnsupported; } stdenv.cc.cc); }; allowedRequisites = null; @@ -258,7 +269,7 @@ in nameDrvAfterAttrName ({ # mechanism, so can only test a couple of flags through altered # stdenv trickery - fortifyStdenvUnsupp = checkTestBin (f2exampleWithStdEnv (stdenvUnsupport ["fortify"]) { + fortifyStdenvUnsupp = checkTestBin (f2exampleWithStdEnv (stdenvUnsupport ["fortify" "fortify3"]) { hardeningEnable = [ "fortify" ]; }) { ignoreFortify = false;