Skip to content

Commit

Permalink
stdenv: fix custom hardening settings when using `__structuredAttrs =…
Browse files Browse the repository at this point in the history
… true;`

Replaces / Closes NixOS#353131

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;`.

All values of `NIX_HARDENING_ENABLE` are well-known, so we don't have to
worry about escaping issues. Just forcing it to a string by
concatenating the list everytime solves the issue without additional
issues like eval errors when inheriting `env` from a structuredAttrs
derivation[5]. The price we're paying is a full rebuild.

[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
[5] NixOS@1e84a7f
  • Loading branch information
Ma27 committed Nov 2, 2024
1 parent d32fe41 commit aaeeef5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkgs/stdenv/generic/make-derivation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ else let
enableParallelChecking = attrs.enableParallelChecking or true;
enableParallelInstalling = attrs.enableParallelInstalling or true;
} // optionalAttrs (hardeningDisable != [] || hardeningEnable != [] || stdenv.hostPlatform.isMusl) {
NIX_HARDENING_ENABLE = enabledHardeningOptions;
NIX_HARDENING_ENABLE = builtins.concatStringsSep " " enabledHardeningOptions;
} // optionalAttrs (stdenv.hostPlatform.isx86_64 && stdenv.hostPlatform ? gcc.arch) {
requiredSystemFeatures = attrs.requiredSystemFeatures or [] ++ [ "gccarch-${stdenv.hostPlatform.gcc.arch}" ];
} // optionalAttrs (stdenv.buildPlatform.isDarwin) (
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 aaeeef5

Please sign in to comment.