cc-wrapper: add support for shadowstack hardening flag
#326819
Conversation
github-actions
bot
added
6.topic: nixos
8.has: documentation
8.has: changelog
6.topic: stdenv
|
Something doesn't look right, I don't see |
|
Somehow the patch does not get applied: But the correct flag is applied: |
|
Well caught - think I know what I've forgotten... |
|
Forgot to pass the |
|
I tried the same packages as in #320597 (comment) again. |
|
Have disabled for |
seems we missed adding stackclashprotection
…fault this appears to have been added to glibc because of the number of packages in some distributions that were built with CET enabled before a CET enabled machine was available to test for breakage with. we don't have that problem to such an extent and users of hardened systems will likely want to enable this by default.
|
Apologies for the rebase - had to put it on top of #327093 |
|
The Doesn't work whether you build it with or without shadow stack. |
|
Ah - this sounds like it might be resolved with glibc's Out of interest, does it give you the same error with/without |
|
No, with |
|
Let's try with |
|
Oh... but before you go for the rebuild you could also try adding |
This does not help, for some reason (same "rebuild shared object with SHSTK support enabled" errors). |
|
It's possible the env var gets stripped by the test harness machinery. (I'm only guessing this is during the Any good if you do a rebuild all the way from glibc with the new branch? |
|
All packages I mentioned in the last PR as well as the ones you suggested on Matrix (in total: ffmpeg, fish, lix, nginx, nsncd, openssh, polkit, postgresql_16, python3Packages.scipy, tmux) have been built now on the latest state. LLVM builds now, and the following failures have been observed:
No other build failures have been observed, in particular |
|
Awesome. That's extremely useful. In the short term we can simply mark these with I'm wondering though whether, when we have more time to investigate them individually, we'll find that it's simply the tests doing things that shadow-stack doesn't like. For example - digging into But on the other hand I wouldn't want to barge in and "fix" the postgres tests by runtime-disabling shadow-stack, as I've no reason to believe it's not a genuine problem being exhibited. And come to think, runtime-disabling shadow-stack for These runtime-optional security features are a bit of a minefield. FWIW the postgres failure not responding the |
|
I regret that I have to inform you that the |
|
Regarding PCRE, it seems that the only thing not compatible with shadow stack is the JIT in the EOL version (PCRE2 works fine). We might consider disabling the JIT, or identifying and backporting the shadow stack support, instead. |
|
Another day of big rebuilds later, I am back with the following results (relative to before all the "disable shadowstack hardening flag" commits):
Given the nature of the observed failures (Boost coroutines and JITs), I have performed some additional builds, which are summarized below.
Furthermore, I have observed binaries in the following packages to start without shadow stack, even though it has not been explicitly disabled: ffmpeg, luajit, nodejs, nsncd, openjdk, spidermonkey. I'm not sure to what extent this is fully intentional (for example, ffmpeg lacks the required note on a random assortment of libraries, nsncd itself doesn't have it (possibly because rustc doesn't set it?), openjdk links against gnutls which doesn't have the note; luajit, nodejs and spidermonkey lacking the note I assume to be intentional). |
risicle
force-pushed
the
ris-shadowstack
branch
2 times, most recently
from
b3165be
to
de43e7c
Compare
|
How does the current HEAD look for you? |
|
Basically the same as in the previous comment. It seems that LLVM (and by extension, rustc) does not support generating shadow stack-compatible code on x86_64. I also built a Go program (lazygit), and have seen what I expected (works, but no shadow stack). I think this is good for the initial iteration now. |
| # causes shadowstack disablement | ||
| pcre = super'.pcre.override { enableJit = false; }; | ||
| pcre-cpp = super'.pcre-cpp.override { enableJit = false; }; | ||
| pcre16 = super'.pcre16.override { enableJit = false; }; |
There was a problem hiding this comment.
These are the first packages that pkgsExtraHardening is making an explicit domain‐specific hardening vs. functionality/performance trade‐off for, rather than just blanket‐defaulting a hardening switch on and letting individual packages decide what to do, right?
I’m not opposed necessarily, just want to check that this is something new that’s happening. I do worry a little about bit rot as package options drift without anyone checking pkgsExtraHardening. And it might be hard to make judgement calls in future when you’re faced with whether to turn all JITs ever off for security reasons.
There was a problem hiding this comment.
Not sure if this answers the question, but here I think is the starting point that lead to this: #326819 (comment) . TLDR: legacy PCRE JIT is broken with shadow stack, so either JIT or shadow stack have to be disabled, and the latter has cascade effects.
There was a problem hiding this comment.
Right, okay. The fact that it would require disabling it for all the downstream packages makes this make sense to me.
There was a problem hiding this comment.
Yes, this is the first time we're overriding an actual package.
Description of changes
Some background: https://discourse.nixos.org/t/future-design-of-hardening-flags/38826
This, along with #324429, is a replacement for #320597, abandoning the idea of combining multiple technologies into a single
hardbackedgecfiflag.This also adds an optional reversion of a glibc commit that default-disabled shadow-stack support at runtime, presumably because of distributions that started building with these flags enabled before they were able to test them properly. We don't have that problem, and for package sets like
pkgsExtraHardeningwe probably want that on by default so that these packages get more testing.Again, I do not have access to a shadowstack-capable machine, so I would greatly appreciate someone who does building as many packages as possible on this branch and telling me which packages need to have
hardeningDisableset for them.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.