Skip to content

Commit

Permalink
stdenv: fix hardeningEnable with __structuredAttrs = true;
Browse files Browse the repository at this point in the history
A while ago `postgresql` switched to using structured attrs[1]. In the
PR it was reported that this made postgresql notably slower when
importing SQL dumps[2].

After a bit of debugging it turned out that the hardening was entirely
missing and the following combination of settings was the culprit:

    hardeningEnable = [ "pie" ];
    __structuredAttrs = true;

I.e. the combination of custom hardening settings and structured attrs.

What happened here is that internally, the default and enabled hardening
flags get written into NIX_HARDENING_ENABLE. However, the value is a list
and the setting is not in the `env` section. This means that in the
structured-attrs case we get something like

    declare -ax NIX_HARDENING_ENABLE=([0]="bindnow" [1]="format" [2]="fortify" [3]="fortify3" [4]="pic" [5]="relro" [6]="stackprotector" [7]="strictoverflow" [8]="zerocallusedregs" [9]="pie")

i.e. an actual array rather than a string with all hardening flags being
space-separated which is what the hardening code of the cc-wrapper
expects[3].

This only happens if `hardeningEnable` or `hardeningDisable` are
explicitly set by a derivation: if none of those are set,
`NIX_HARDENING_ENABLE` won't be set by `stdenv.mkDerivation` and the
default hardening flags are configured by the setup hook of the
cc-wrapper[4].

In other words, this _only_ applies to derivations that have both custom
hardening settings _and_ `__structuredAttrs = true;`.

I figured that it's far easier to just write a space-separated string
for NIX_HARDENING_ENABLE into `env` if needed rather than changing the
code in `add-hardening.sh` which would've caused a full rebuild. The
flags are well-known identifiers anyways, so there's no risk of issues
coming from missing escapes.

[1] NixOS#294504
[2] NixOS#294504 (comment)
[3] https://github.com/NixOS/nixpkgs/blob/cf3e5d3744dc26c3498aa5dadfa0e078c632cede/pkgs/build-support/cc-wrapper/add-hardening.sh#L9
[4] https://github.com/NixOS/nixpkgs/blob/cf3e5d3744dc26c3498aa5dadfa0e078c632cede/pkgs/build-support/cc-wrapper/setup-hook.sh#L114
  • Loading branch information
Ma27 committed Nov 2, 2024
1 parent 344b92a commit 33bb624
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 24 deletions.
58 changes: 34 additions & 24 deletions pkgs/stdenv/generic/make-derivation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ let
"__darwinAllowLocalNetworking"
"__impureHostDeps" "__propagatedImpureHostDeps"
"sandboxProfile" "propagatedSandboxProfile"
"enabledHardeningOptions"
];

# 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
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 {
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
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

0 comments on commit 33bb624

Please sign in to comment.