Skip to content

glibc: expose enableCET as overridable argument, default permissive#288052

Merged
risicle merged 1 commit intoNixOS:stagingfrom
risicle:ris-glibc-enable-cet-permissive
Apr 13, 2024
Merged

glibc: expose enableCET as overridable argument, default permissive#288052
risicle merged 1 commit intoNixOS:stagingfrom
risicle:ris-glibc-enable-cet-permissive

Conversation

@risicle
Copy link
Copy Markdown
Contributor

@risicle risicle commented Feb 11, 2024

Description of changes

This should be a gentler way to introduce CET-compiled binaries into general usage: the "permissive" mode will silently disable CET when a non-CET-compiled .so rather than throwing an error.

Non-permissive mode could still easily be enabled for e.g. pkgsExtraHardening.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. label Feb 11, 2024
@ofborg ofborg bot requested review from ConnorBaker, Ma27 and edolstra February 11, 2024 16:42
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Feb 11, 2024
@risicle risicle mentioned this pull request Feb 25, 2024
13 tasks
@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Mar 3, 2024

This should be a gentler way to introduce CET-compiled binaries into general usage: the "permissive" mode will silently disable CET when a non-CET-compiled .so rather than throwing an error.

While I understand the immediate effect, what's the rationale behind that? That this currently produces e.g. libs without CET if built on a builder that's too old (AFAIU --enable-cet=auto is the default for gcc)?

@risicle
Copy link
Copy Markdown
Contributor Author

risicle commented Mar 3, 2024

It's not just libs that were built on older systems, it's that we're probably going to have to disable CET on a bunch of packages that get broken by the funny stack shenanigans that CET performs (JITs maybe?). The default "hard fail" mode is probably stronger than most people are going to tolerate on a normal system.

Granted we're in a slightly better position than most distributions because at the time we're building an executable, we (mostly) know exactly what libs it's going to be linked to, so in theory we could put together some kind of mechanism that could check if any of the closure's packages have had CET disabled - but I see little point in putting such a build-time mechanism together when a runtime one is provided.

(FWIW other distributions have already started building packages with CET enabled, but I think that was probably a mistake on their part, as they've presumably not been able to test them properly until very recently and won't have been able to discover the ones that CET breaks.. suggesting there will be a lot of packages floating around in their ecosystem marked as "CET enabled" which may well break when CET mode is actually enabled)

@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Mar 3, 2024

cc @ajs124 who originally implemented that.

It's not just libs that were built on older systems, it's that we're probably going to have to disable CET on a bunch of packages that get broken by the funny stack shenanigans that CET performs (JITs maybe?). The default "hard fail" mode is probably stronger than most people are going to tolerate on a normal system.

I'm not sure I fully understand yet why this is a "new" problem: is it an existing problem or something new (given that "they've presumably not been able to test them properly until very recently")? in that case, glibc 2.39 + linux kernel >=6.6 is the cause? Asking because that might be relevant on whether we should schedule the upcoming glibc update together with this for instance.

@risicle
Copy link
Copy Markdown
Contributor Author

risicle commented Mar 3, 2024

glibc 2.39 + linux kernel >=6.6 is the cause?

That and the fact that we haven't been building CET-enabled packages so far - I haven't added the hardening flag for that yet. So there's no specific deadline for this, but it's another thing I'd like in place before I go adding hardening flags.

@ajs124
Copy link
Copy Markdown
Member

ajs124 commented Mar 3, 2024

cc @ajs124 who originally implemented that.

Don't have any additional insight, but trust you to do the right thing.

@risicle
Copy link
Copy Markdown
Contributor Author

risicle commented Mar 4, 2024

Does this also affect dependencies of a program where this needs to be disabled?
The glibc INSTALL section only mentions the opposite case when using --enable-cet=permissive, i.e. that a CET-enabled program must not dlopen a non-CET program, so I’m not sure

That is a detail I had missed - I think you're right and that it's only for dlopen. I guess the idea is that libs you're linked against can probably be trusted, but the "immediate failure" is to protect against a program being tricked into opening a .so without CET support and disabling the protection.

Still, I'd argue that our preferred mode should be permissive, at least to start with.

A little bit more detail on support in various components here: https://lpc.events/event/7/contributions/729/attachments/496/903/CET-LPC-2020.pdf

@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Mar 4, 2024

That and the fact that we haven't been building CET-enabled packages so far

OK, that was part of what confused me, sorry!
I misread in https://gcc.gnu.org/install/configure.html that --enable-cet=auto being the default implies CET being enabled on our end. After rebuilding pkgs.hello with -fcf-protection and comparing the results with diffoscope I realized that this is clearly not the case.

My confusion came from the wrong assumption that part of this was already in place and thus at least people with 6.6 should've noticed issues already if any. Given that this is not the case, I agree with your assessment 👍

this should be a gentler way to introduce CET-compiled
binaries into general usage
@risicle risicle force-pushed the ris-glibc-enable-cet-permissive branch from 7305ec8 to da25f95 Compare March 24, 2024 13:45
@risicle risicle marked this pull request as ready for review March 24, 2024 13:46
@risicle
Copy link
Copy Markdown
Contributor Author

risicle commented Apr 13, 2024

Might I be able to get an approval for this?

@ConnorBaker
Copy link
Copy Markdown
Contributor

@risicle just to confirm -- with the mode set to permissive, CET-enabled programs are still able to dlopen non CET-enabled programs, correct? As an example, setting the mode to permissive won't break any programs loading proprietary libraries that are not CET-enabled (like CUDA).

@risicle
Copy link
Copy Markdown
Contributor Author

risicle commented Apr 13, 2024

That is my understanding, yes. This mode is less strict than its current value, so it shouldn't "break" anything.

@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Apr 13, 2024

I'll probably file another trivial glibc change later (#303193). Would consider merging them in one round since both are rebuilds of the same (large) package.

Copy link
Copy Markdown
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

#303868 should be non-controversial, so we could merge them together into staging.

@risicle risicle merged commit 90b4ef8 into NixOS:staging Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants