cc-wrapper: add support for strictflexarrays1 & strictflexarrays3 hardening flags#400408
Merged
philiptaron merged 15 commits intoNixOS:stagingfrom May 26, 2025
Merged
Conversation
github-actions
bot
added
6.topic: nixos
github-actions
bot
added
10.rebuild-darwin: 5001+
risicle
force-pushed
the
ris-hardening-strictflexarrays1-3
branch
from
84c8446 to
c12424d
Compare
13 tasks
risicle
force-pushed
the
ris-hardening-strictflexarrays1-3
branch
2 times, most recently
from
52b82c3 to
1100a62
Compare
github-actions
bot
removed
6.topic: nixos
Contributor
Author
|
(moved release notes addition from the nixos release notes to the nixpkgs release notes) |
risicle
force-pushed
the
ris-hardening-strictflexarrays1-3
branch
from
1100a62 to
ec618e8
Compare
Contributor
Author
|
Rebased to adapt to #400351 |
philiptaron
reviewed
May 23, 2025
Contributor
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
I'm trying to test this with the expression:
nix-build --expr 'with import ./. { }; let inherit (tests) hardeningFlags hardeningFlags-clang hardeningFlags-gcc; f = v: let r = builtins.tryEval v.outPath or null; in if r.success then v else null; vs = lib.flatten (map builtins.attrValues [ hardeningFlags hardeningFlags-clang hardeningFlags-gcc ]); evald = lib.filter (v: v != null && lib.isDerivation v) (map f vs); in evald' --keep-going
This calls and runs all the tests in tests.hardeningFlags, tests.hardeningFlags-clang, and tests.hardeningFlags-gcc.
I get many failures.
error: build of '/nix/store/02d9xk9phzxsd7bsw6ckmvrw076n4a6p-test-relROExplicitEnabled.drv', '/nix/store/0fypyjdf3b0yygxhaiqjv538fanyxr2q-test-fortify3EnabledEnvEnablesFortify1.drv', '/nix/store/12smdx2xmfh4h85whbkwfg67lz121sbp-test-fortify1ExplicitDisabledCmdlineEnabled.drv', '/nix/store/187vl8qg9nahsw43f4d0k8i0az24ia3n-test-fortify3ExplicitDisabledDoesntDisableFortify.drv', '/nix/store/2178km0b6wi32z5qx353y7k4axhj47dx-test-sfa3explicitDisabledDoesntDisableSfa1.drv', '/nix/store/2dsdy611sg64ww3bl0yy9l0g4fznnwlz-test-fortifyExplicitEnabled.drv', '/nix/store/3yjf21cr4jh07fiqgqdgrfh9a8cpa4kx-test-stackProtectorReenabledEnv.drv', '/nix/store/44kz1hdr8l6j1f8hi1nqy5mmjml23997-test-sfa3explicitEnabledProtectsDefLen1.drv', '/nix/store/471j6pdkjp1x0sx8d8syx5vjnn0dymfz-test-fortify3StdenvUnsuppDoesntUnsuppFortify1.drv', '/nix/store/6dvqjbc6n0bpnvl39c25vi1199r9f079-test-sfa3explicitEnabledProtectsDefLen1.drv', '/nix/store/6f52i0hbq7nc8xhrch52lxpf2fpf58na-test-stackProtectorReenabledEnv.drv', '/nix/store/8qig3dfnd5xvzdb53p96fidry15zm46q-test-stackProtectorExplicitEnabled.drv', '/nix/store/91l26mbn9wigxhay04mfwqp5nwi9casl-test-pieExplicitEnabled.drv', '/nix/store/9brgyhzvrrdi7zknm5dqv6bnwhvpsz5j-test-fortify1ExplicitEnabledCmdlineDisabled.drv', '/nix/store/9kjkfnc19pk96nxm0nk0w15xybybf0l7-test-fortify1ExplicitEnabledCmdlineDisabled.drv', '/nix/store/9krm5fxhcbk9jp7n5gm7k5hjg3ay37vr-test-sfa3EnabledEnvEnablesSfa1.drv', '/nix/store/9nm2khnrc84vavz0j3lanqvk7fh33bcd-test-fortifyExplicitEnabled.drv', '/nix/store/ab0bgp03km1b165bcww12as3ialjwf5p-test-fortify3ExplicitDisabledDoesntDisableFortify.drv', '/nix/store/b8ijn0y57h810z8xvsppmmg3ih0s3yls-test-fortify3EnabledEnvEnablesFortify1.drv', '/nix/store/ca39j7mx5gs6b0q2p06z36k8pp36lby6-test-fortify3ExplicitEnabled.drv', '/nix/store/czxxsz7bcq7f31s4mpx849773qk5z1sr-test-bindNowExplicitEnabled.drv', '/nix/store/d6fy5a79ikkibnvp1mdl5da79ncqd3bi-test-stackProtectorReenabledFromAllEnv.drv', '/nix/store/gbh9ycrnzjyi6q5kg8g9jf0a1qr17c6q-test-bindNowExplicitEnabled.drv', '/nix/store/gc5hbpad75pfzgr29rkl9pp2h5cdfnij-test-pieExplicitEnabledStructuredAttrs.drv', '/nix/store/gm0rm5fhyx24zpa1ig2dwsl8mfjv16dd-test-pieExplicitEnabledStructuredAttrs.drv', '/nix/store/hyngdz68zsz1i592ii47ym9sgcp7hfp1-test-sfa3EnabledEnvEnablesSfa1.drv', '/nix/store/j5742w0nfbdkl3wq05i36ry5qkkp82a0-test-stackProtectorReenabledFromAllEnv.drv', '/nix/store/jlwxshk6igm84j2ffmgzq9av902l375q-test-stackProtectorExplicitEnabled.drv', '/nix/store/pl8m71rb3qjn53hwdn791whfcg6k7mk3-test-relROExplicitEnabled.drv', '/nix/store/q3bd6xwdfgad005kyvgk3mnqxx1f69zw-test-fortify3StdenvUnsuppDoesntUnsuppFortify1.drv', '/nix/store/r21vwdc0izfmgm14a0wshfbzb03mgv5q-test-sfa3StdenvUnsuppDoesntUnsuppSfa1.drv', '/nix/store/rf4iwpzq3d7mzajipnigzdymiqb220vl-test-sfa3explicitDisabledDoesntDisableSfa1.drv', '/nix/store/vls4jfgz45cdwcc45afk7jqa5vxnyain-test-fortify1ExplicitDisabledCmdlineEnabled.drv', '/nix/store/w1sl16bifkjn3iyjq65k0w18xcrssq4l-test-sfa3StdenvUnsuppDoesntUnsuppSfa1.drv', '/nix/store/x7yjfn2fjbcav5qqr4vcagldn56s6whr-test-pieExplicitEnabled.drv', '/nix/store/y97gb3p50bac0m368df17mypavz066j2-test-sfa1explicitEnabled.drv', '/nix/store/zqqd2xzh5nbyzrj1fpq54rw2cc0iy4mr-test-sfa1explicitEnabled.drv' failed
Any other way I should test this? This is on x86_64-linux on NixOS.
| any (x: x == "fortify") hardeningDisable | ||
| concretizeFlagImplications = | ||
| flag: impliesFlags: list: | ||
| if any (x: x == flag) list then unique (list ++ impliesFlags) else list; |
Contributor
There was a problem hiding this comment.
unique is sometimes costly since it's O(n^2) with the length of the list as I understand it -- could we solve the multiple entries in bash? Do we already do that?
Contributor
Author
There was a problem hiding this comment.
I've been hoping for the sort-based uniq from #119286 to be included for a while now for exactly this reason, but it feels like I'm the only one interested in it.
It's done in nix instead of bash in attempt to avoid rebuilds for equivalent flag-sets (though it doesn't yet do this fully - if I had the above, faster uniq implementation, I would add more calls to it to make it watertight).
You do raise a point though - I could reduce these all to a single unique call (which we already do for fortify/fortify3).
Contributor
Author
|
re the test failures - you should find that all the failing tests are marked Trying to recall the exact expression I used for this, think it's something along the lines of:
|
Contributor
Author
|
Oh hold on, no I know what the failures are - a new version of debian-devscripts has landed that has a new flag I need to add, |
risicle
force-pushed
the
ris-hardening-strictflexarrays1-3
branch
from
ec618e8 to
a7d90a9
Compare
nix-owners
bot
requested review from
Ericson2314,
dtzWill,
emilazy,
lovek323 and
rrbutani
nix-owners
bot
requested review from
Artturin,
RossComputerGuy,
alyssais,
reckenrode and
sternenseemann
…dening flags adding strictflexarrays1 to pkgsExtraHardening
…/glibc this appears to work now
introduced with new debian-devscripts
risicle
force-pushed
the
ris-hardening-strictflexarrays1-3
branch
from
a7d90a9 to
b7771e1
Compare
philiptaron
approved these changes
May 24, 2025
| ) | ||
| ); | ||
|
|
||
| pieExplicitDisabled = brokenIf (stdenv.hostPlatform.isMusl && stdenv.cc.isClang) ( |
Contributor
There was a problem hiding this comment.
allExplicitDisabledPie and this one, pieExplicitDisabled, are the only tests in this file that fails for me on x86_64-linux now.
Contributor
Author
There was a problem hiding this comment.
Interesting - for clang or gcc? They all pass for me now on x86_64 nixos.
Contributor
There was a problem hiding this comment.
For clang.
tests.hardeningFlags-clang.allExplicitDisabledPie
/nix/store/dpx82dk9ncs8igknbynirq9np03y9s52-test-bin/bin/test-bin:
Position Independent Executable: yes
Stack protected: no, not found! (ignored)
Fortify Source functions: no, only unprotected functions found! (ignored)
Read-only relocations: yes
Immediate binding: no, not found! (ignored)
Stack clash protection: unknown, no -fstack-clash-protection instructions found
Control flow integrity: no, not found! (ignored)
Branch Protection: no, not found! (ignored)
ERROR: Expected hardening-check to fail, but it passed!
tests.hardeningFlags-clang.pieExplicitDisabled
/nix/store/qfpr0mg2h676c5qa969my6mss2w40dcx-test-bin/bin/test-bin:
Position Independent Executable: yes
Stack protected: yes
Fortify Source functions: yes
Read-only relocations: yes
Immediate binding: yes
Stack clash protection: unknown, no -fstack-clash-protection instructions found
Control flow integrity: no, not found! (ignored)
Branch Protection: no, not found! (ignored)
ERROR: Expected hardening-check to fail, but it passed!
Contributor
Author
There was a problem hiding this comment.
Thanks, I'll have to look into this...
wegank
added
the
12.approvals: 1
13 tasks
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
More background on these flags: https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html#enable-strict-flexible-arrays
In short, they improve coverage of fortify checks by reducing the number of cases the compiler has to be permissive with.
Initially I was just going to add
strictflexarrays3and be satisfied with leaving it as apkgsExtraHardeningflag until projects had caught up and stopped using old-fashioned flexible array declarations, but as you can see from the number ofstrictflexarrays3disablements I've added, there are quite a lot of projects with this problem (and this is only the tip of the iceberg - cpython heavily uses[1]-style declarations in its API, meaning much of the python ecosystem will fail to compile withstrictflexarrays3- see python/cpython#84301). So I've also addedstrictflexarrays1which is the strictest mode python will compile with, and used that forpkgsExtraHardening.strictflexarrays3is left for those who want to experiment and/or don't need many python packages.The behaviour of the two flags exactly mirrors the behaviours of
fortifyandfortify3and in fact is largely implemented through a copy-pasta of most of those code fragments. A number oftests.hardeningFlagstests are added to prove this. As ever, tinkering with the hardening flags tests caused me to do a bit of refactoring.As ever, neither of these new flags are enabled by default for normal packagesets.
Tested build of many packages with
strictflexarrays3default-enabled on aarch64-linux and macos 12 x86_64 (enough to gather the various fixes included in this PR). Successfully bootstappedpkgsMusl,pkgsStatic,pkgsCross.riscv64on aarch64-linux.Tested
tests.hardeningFlagson the default packageset for aarch64-linux and macos 12 x86_64,pkgsLLVM,pkgsi686Linux,pkgsMusl,pkgsStaticwith expected results.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.