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

stdenv: fix hardeningEnable with __structuredAttrs = true; #353131

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 1 addition & 1 deletion pkgs/by-name/li/libcpucycles/package.nix
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ stdenv.mkDerivation (finalAttrs: {

nativeBuildInputs = [ python3 ];

inherit (librandombytes) hardeningDisable configurePlatforms env;
inherit (librandombytes) hardeningDisable configurePlatforms env __structuredAttrs;

preFixup = lib.optionalString stdenv.hostPlatform.isDarwin ''
install_name_tool -id "$out/lib/libcpucycles.1.dylib" "$out/lib/libcpucycles.1.dylib"
58 changes: 34 additions & 24 deletions pkgs/stdenv/generic/make-derivation.nix
Original file line number Diff line number Diff line change
@@ -134,6 +134,7 @@ let
"__darwinAllowLocalNetworking"
"__impureHostDeps" "__propagatedImpureHostDeps"
"sandboxProfile" "propagatedSandboxProfile"
"enabledHardeningOptions"
Copy link
Member Author

Choose a reason for hiding this comment

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

This is pretty nasty because it removes each enabledHardeningOptions set in a call to stdenv.mkDerivation.
Any suggestions on how to improve that?

];

# Turn a derivation into its outPath without a string context attached.
@@ -219,6 +220,7 @@ let

, hardeningEnable ? []
, hardeningDisable ? []
, enabledHardeningOptions ? []

, patches ? []

@@ -258,23 +260,6 @@ let
++ depsTargetTarget ++ depsTargetTargetPropagated) == 0;
dontAddHostSuffix = attrs ? outputHash && !noNonNativeDeps || !stdenv.hasCC;

hardeningDisable' = if any (x: x == "fortify") hardeningDisable
# disabling fortify implies fortify3 should also be disabled
then unique (hardeningDisable ++ [ "fortify3" ])
else hardeningDisable;
defaultHardeningFlags =
(if stdenv.hasCC then stdenv.cc else {}).defaultHardeningFlags or
# fallback safe-ish set of flags
(if with stdenv.hostPlatform; isOpenBSD && isStatic
then knownHardeningFlags # Need pie, in fact
else remove "pie" knownHardeningFlags);
enabledHardeningOptions =
if builtins.elem "all" hardeningDisable'
then []
else subtractLists hardeningDisable' (defaultHardeningFlags ++ hardeningEnable);
# hardeningDisable additionally supports "all".
erroneousHardeningFlags = subtractLists knownHardeningFlags (hardeningEnable ++ remove "all" hardeningDisable);

checkDependencyList = checkDependencyList' [];
checkDependencyList' = positions: name: deps:
imap1
@@ -283,11 +268,7 @@ let
else if isList dep then checkDependencyList' ([index] ++ positions) name dep
else throw "Dependency is not of a valid type: ${concatMapStrings (ix: "element ${toString ix} of ") ([index] ++ positions)}${name} for ${attrs.name or attrs.pname}")
deps;
in if builtins.length erroneousHardeningFlags != 0
then abort ("mkDerivation was called with unsupported hardening flags: " + lib.generators.toPretty {} {
inherit erroneousHardeningFlags hardeningDisable hardeningEnable knownHardeningFlags;
})
else let
in let
Copy link
Member Author

Choose a reason for hiding this comment

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

This is actually needed to prevent an infinite recursion.

doCheck = doCheck';
doInstallCheck = doInstallCheck';
buildInputs' = buildInputs
@@ -412,7 +393,7 @@ else let
inherit enableParallelBuilding;
enableParallelChecking = attrs.enableParallelChecking or true;
enableParallelInstalling = attrs.enableParallelInstalling or true;
} // optionalAttrs (hardeningDisable != [] || hardeningEnable != [] || stdenv.hostPlatform.isMusl) {
} // optionalAttrs (! __structuredAttrs && (hardeningDisable != [] || hardeningEnable != [] || stdenv.hostPlatform.isMusl)) {
NIX_HARDENING_ENABLE = enabledHardeningOptions;
} // optionalAttrs (stdenv.hostPlatform.isx86_64 && stdenv.hostPlatform ? gcc.arch) {
requiredSystemFeatures = attrs.requiredSystemFeatures or [] ++ [ "gccarch-${stdenv.hostPlatform.gcc.arch}" ];
@@ -532,6 +513,9 @@ mkDerivationSimple = overrideAttrs:
# but for anything complex, be prepared to debug if enabling.
, __structuredAttrs ? config.structuredAttrsByDefault or false

, hardeningEnable ? []
, hardeningDisable ? []

, env ? { }

, ... } @ attrs:
@@ -549,6 +533,27 @@ assert attrs ? outputHash -> (

let
envIsExportable = isAttrs env && !isDerivation env;
hardeningDisable' = if any (x: x == "fortify") hardeningDisable
# disabling fortify implies fortify3 should also be disabled
then unique (hardeningDisable ++ [ "fortify3" ])
else hardeningDisable;
defaultHardeningFlags =
(if stdenv.hasCC then stdenv.cc else {}).defaultHardeningFlags or
# fallback safe-ish set of flags
(if with stdenv.hostPlatform; isOpenBSD && isStatic
then knownHardeningFlags # Need pie, in fact
else remove "pie" knownHardeningFlags);
enabledHardeningOptions =
if builtins.elem "all" hardeningDisable'
then []
else subtractLists hardeningDisable' (defaultHardeningFlags ++ hardeningEnable);

# hardeningDisable additionally supports "all".
erroneousHardeningFlags = subtractLists knownHardeningFlags (hardeningEnable ++ remove "all" hardeningDisable);

env = (attrs.env or {}) // lib.optionalAttrs __structuredAttrs {
Copy link
Member

@kirillrdy kirillrdy Nov 2, 2024

Choose a reason for hiding this comment

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

running nixpkgs-review

        at /home/kirillvr/.cache/nixpkgs-review/pr-353131/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:554:27:
          553|
          554|   env = (attrs.env or {}) // lib.optionalAttrs __structuredAttrs {
             |                           ^
          555|     NIX_HARDENING_ENABLE = builtins.concatStringsSep " " enabledHardeningOptions;

       error: expected a set but found a string with context: "/nix/store/7awmks5w8v9yjgqbpmv3qa79k2qqg52i-wee-slack-env/lib/python3.12/site-packages"

NIX_HARDENING_ENABLE = builtins.concatStringsSep " " enabledHardeningOptions;
};

derivationArg = makeDerivationArgument
(removeAttrs
@@ -560,6 +565,7 @@ let
// {
cmakeFlags = makeCMakeFlags attrs;
mesonFlags = makeMesonFlags attrs;
inherit enabledHardeningOptions;
});

meta = checkMeta.commonMeta {
@@ -591,7 +597,11 @@ let
# would make it fixed-output.
deleteFixedOutputRelatedAttrs = lib.flip builtins.removeAttrs [ "outputHashAlgo" "outputHash" "outputHashMode" ];

in
in if builtins.length erroneousHardeningFlags != 0
then abort ("mkDerivation was called with unsupported hardening flags: " + lib.generators.toPretty {} {
inherit erroneousHardeningFlags hardeningDisable hardeningEnable knownHardeningFlags;
})
else

extendDerivation
validity.handled
7 changes: 7 additions & 0 deletions pkgs/test/cc-wrapper/hardening.nix
Original file line number Diff line number Diff line change
@@ -178,6 +178,13 @@ in nameDrvAfterAttrName ({
ignorePie = false;
});

pieExplicitEnabledStructuredAttrs = brokenIf stdenv.hostPlatform.isStatic (checkTestBin (f2exampleWithStdEnv stdenv {
hardeningEnable = [ "pie" ];
__structuredAttrs = true;
}) {
ignorePie = false;
});

relROExplicitEnabled = checkTestBin (f2exampleWithStdEnv stdenv {
hardeningEnable = [ "relro" ];
}) {