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.

Sign up for GitHub

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

stdenv: fix hardeningEnable with __structuredAttrs = true; #353131

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33bb624
stdenv: fix `hardeningEnable` with `__structuredAttrs = true;`
Ma27 Nov 2, 2024
1e84a7f
libcpucycles: hacky build fix
Ma27 Nov 2, 2024
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
58 changes: 34 additions & 24 deletions pkgs/stdenv/generic/make-derivation.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -219,6 +220,7 @@ let

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

, patches ? []

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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}" ];
Expand Down Expand Up @@ -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:
Expand All @@ -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 •
edited
Loading

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
Expand All @@ -560,6 +565,7 @@ let
// {
cmakeFlags = makeCMakeFlags attrs;
mesonFlags = makeMesonFlags attrs;
inherit enabledHardeningOptions;
});

meta = checkMeta.commonMeta {
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions pkgs/test/cc-wrapper/hardening.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" ];
}) {
Expand Down
Loading