Skip to content

release: build insecure packages#351429

Closed
SuperSandro2000 wants to merge 1 commit into
NixOS:masterfrom
SuperSandro2000:insecure-cache
Closed

release: build insecure packages#351429
SuperSandro2000 wants to merge 1 commit into
NixOS:masterfrom
SuperSandro2000:insecure-cache

Conversation

@SuperSandro2000

Copy link
Copy Markdown
Member

Not caching insecure packages is just security by obscurity. We should give users a choice if they want to continue to use the insecure version and if they decide that way, they should not require to compile the packages.

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.11 Release Notes (or backporting 23.11 and 24.05 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.

@SuperSandro2000 SuperSandro2000 added the 9.needs: community feedback This needs feedback from more community members. label Oct 26, 2024
@emilazy emilazy added the 1.severity: security Issues which raise a security issue, or PRs that fix one label Oct 26, 2024
@emilazy emilazy requested a review from a team October 26, 2024 12:49
@emilazy

emilazy commented Oct 26, 2024

Copy link
Copy Markdown
Member

This would result in Hydra running known‐insecure code with the protection of only the relatively porous Nix sandbox on Linux and no protection at all on Darwin. Given the huge potential for supply‐chain security attacks through a compromised builder, I don’t think that’s an acceptable risk.

@ctheune

ctheune commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

This would result in Hydra running known‐insecure code with the protection of only the relatively porous Nix sandbox on Linux and no protection at all on Darwin. Given the huge potential for supply‐chain security attacks through a compromised builder, I don’t think that’s an acceptable risk.

I'd like to interpret this as: there are multiple dimensions in security and one of them means we must be really really strict about whether we can continue building the package - e.g. we have proof of a supply chain attack that tries to steal the hydra keys.

I'd like to invoke the "common cause / special cause" distinction: a supply chain attack on hydra (that is discovered!) is a special cause, whereas regular security issues are common causes and warrant different - more streamlined - handling.

@ctheune

ctheune commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

Something that came to my mind at the same time: hydra at the moment doesn't even build the packages - it only does the eval, signing, compression, etc. So that already improves (solves?) the situation against supply chain attacks on hydra.

@RossComputerGuy

Copy link
Copy Markdown
Member

I can understand not wanting to build these packages locally but it's probably for the best to avoid caching it.

@emilazy

emilazy commented Oct 26, 2024

Copy link
Copy Markdown
Member

I am using the conventional metonym of Hydra to refer to the entire builder farm – which invariably has a high level of trust placed in it even if it’s not quite as high as the central coordinator node. It’s about systematic security posture rather than responding reactively to threats once it’s already too late. Admittedly, we should in any case work on significantly tightening the sandboxing of our trusted builds.

I also just don’t think it’s a good use of resources for us to spend CPU cycles, storage, and bandwidth on building old browser engines, end‐of‐life Node.js versions, insecure JDKs, and so on, just to make it easier for people to expose themselves to RCE vulnerabilities; the packages that are the biggest attack surfaces are also often the packages that take the longest to build and produce the largest cached outputs.

@ctheune

ctheune commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

Well, the CPU resources are actively then spent in other places so for the overall community those cycles are spend over and over in the various places so that's both economically and ecologically worse.

The systematic security posture that you're referencing is misguided IMHO as it fails to meet the user's needs. We delivered a stable distribution, e.g. 24.05 that contains package X and now it doesn't anymore. Among a number of other reasons, we should give people a chance to move off a broken thing onto a not-broken thing and just denying access to the broken thing doesn't help. It basically makes people spend more time maintaining the broken thing and not spend their time on moving to the not broken thing.

@ctheune

ctheune commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

Oh and by the way: I'm only talking about our posture on stable branches, not on unstable. I'd be happy to not build insecure stuff on unstable ...

@ctheune

ctheune commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

I also see a dynamic where maintainers actively do not mark things as unstable exactly because they want to avoid tripping over the Hydra situation. I would love to give an example, but I don't have a specific one.

Not wanting to spam the discussion too much, so I'm going quiet for a while. My point has been made: I think we can do better than the blanket policy like the current one.

@LeSuisse

Copy link
Copy Markdown
Member

I think packages tagged with knownVulnerabilities mostly fall into 2 categories:

  • things that are in a bad shape and either need to be fixed upstream (or, more rarely, need a nixpkgs maintainer with enough internal knowledge to handle a backport for the version we have on stable). One of the example I have in mind for this is freeimage.
  • things for which we are not in position to evaluate because it will depend on the specific threat model of everybody. I think the most recent example of that is likely libolm.

I'm not sure if we should build the package in the first category, it often means that upstreams are not responsive/projects are not maintained anymore and we should remove them from the package set after some time, it is not going to be better if we wait.

@ofborg ofborg Bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 26, 2024
@risicle

risicle commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

Nixpkgs is ever-expanding enough as it is, with quite varied package quality. Expanding to "meet the needs" of users who need to run packages with knownVulnerabilities (with the same convenience as other packages) is a step further down the quality scale than I'd advocate spending resources on.

@SuperSandro2000

Copy link
Copy Markdown
Member Author

This change would allow us to freely set meta.knownVulnerabilties on any package no matter where in the dependency tree it is without having to worry that people are being gated and actively kept away from using the package or peer dependencies or having to disable parts of tests. See for example js2py #349550 where we are now no longer running tests in some packages. I also don't think that building js2py on hydra should have a security impact, builds have no network access as otherwise exploiting hydra would be dead easy.


I think if any package would have such a described supplychain attack we should remove it immediately instead of actively marking it with knownVulnerabilities and then still letting others build it and be exposed to the exact same issue. Not removing the package in that situation is absurd.

Also how much does knownVulnerabilities protect us really? It is usually set afterwards at which point we already build the code on the builders probably at least once. Do we just hope no one knew how to exploit the vulnerability before we set it and hopefully no old job is restarted?

I also just don’t think it’s a good use of resources for us to spend CPU cycles, storage, and bandwidth on building old browser engines, end‐of‐life Node.js versions, insecure JDKs

We have meta.hydraPlatforms for this exact use case and right now we are abusing knownVulnerabilities for it. Should we migrate all current knownVulnerabilities for bigger to compile packages to an empty hydraPlatforms to free resources?

with the same convenience as other packages

You still need to allow them, you just don't need to build them afterwards. What alternatives do users have when they sometimes just cannot migrate away from that package for a variety of reasons? Not all CVEs do always apply.
One option would be to just not upgrade nixpkgs at all to keep a cache and stay on an old version for potential a longer amount of time. That would be even worse!

I also see a dynamic where maintainers actively do not mark things as unstable exactly because they want to avoid tripping over the Hydra situation. I would love to give an example, but I don't have a specific one.

One example where we hack around this already is resholve through oil because that still uses python 2.7 which CVEs don't really apply to it.


We had this already in #233028 which I kinda forgot about.

@SuperSandro2000

Copy link
Copy Markdown
Member Author

I also find it really hard to justify to anyone why they need to compile a software with the reasoning that the software is just EOL in the release cycle. We basically make people compile software as a precaution and probably wasting more CPU cycles in total, just distributed in our community and it sometimes feels a bit like f you.

@emilazy

emilazy commented Oct 30, 2024

Copy link
Copy Markdown
Member

Per an unrelated conversation I had with vcunat earlier, hydraPlatforms = [ ]; doesn’t stop builds on Hydra, it only prevents them from being created as their own jobs; if any other automatically‐created job for a package depends on the insecure package, it would trigger a build. It’s a weak mechanism that only really works for leaf packages. (It would also get in the way if, say, someone wanted to build and cache insecure packages on their own Hydra instance, as I suggested would be a good alternative.)

The previous PR seems to have related to OpenSSL 1.1, which was going EOL in the middle of a release cycle and was handled more precisely through the existing exception mechanism in #233024 that we are currently also using for libolm. Thankfully there are no longer large swathes of the ecosystem stuck on that version.

resholve is in fact marked vulnerable and specifically cautions about the risks of exposing untrusted files to the unmaintained CPython 2.7 interpreter. We permit it specifically for Nix builds only as a stop‐gap measure until the C++‐transpiled Oils codebase provides the necessary APIs. That exception won’t live forever (and I’m not in love with it – I think CPython 2.7 should be removed entirely, which is why I’ve put some preliminary work into seeing if resholve can migrate off earlier).

I think I’ve already said all I have to say about the rest, and agree with the security team members who have commented here since.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 10, 2024
@emilazy emilazy mentioned this pull request Jan 3, 2025
13 tasks
@wolfgangwalther

Copy link
Copy Markdown
Contributor

Also how much does knownVulnerabilities protect us really? It is usually set afterwards at which point we already build the code on the builders probably at least once. Do we just hope no one knew how to exploit the vulnerability before we set it and hopefully no old job is restarted?

I think that's quite important of an observation. The only way I can imagine a supply-chain attack to happen is from within, aka the bad code needs to be already committed and actively run by the builders. I don't see any reason to believe that building an insecure package (without malicious code targeting an attack in it!) on the builders would allow attacking the builders from the outside (or from other packages). Thus, we would need to guarantee to not build insecure packages ever. But how likely is it, that we're marking a package as insecure before we update to the insecure version?

It's the opposite: In the vast majority of cases, we will consider a package secure and build it - until bugs are found in existing code. But at this stage, we have already built this code so often on the builders.


Thus, I think this comes down to two things:

  • Do we want to spend the resources on it?
  • Do we want to encourage using insecure packages?

The status-quo makes it possible, but annoying to use insecure packages. Without knowing more about the specific vulnerabilities or the specific situation that applies to a user, it's impossible to judge whether it's OK to use or not. And in that case, we should certainly err on the side of caution.

Long build times are probably more effective communicating the "you might need to look into this for your specific use-case" compared to warnings only. So, I'd say that's a good thing.


I also find it really hard to justify to anyone why they need to compile a software with the reasoning that the software is just EOL in the release cycle.

Is that happening? The docs for knownVulnerabilities that I read are this:

A list of known vulnerabilities affecting the package, usually identified by CVE identifiers.

Being EOL doesn't strike me as a "known vulnerability" in itself. In fact quite the opposite, "potentially an unknown vulnerability".

@wolfgangwalther

Copy link
Copy Markdown
Contributor

This currently has a label of needs: community feedback - and it leads the PRs with the most 👎 at https://github.com/NixOS/nixpkgs/pulls?q=is%3Aopen%20is%3Apr%20sort%3Areactions--1-desc.

I'd argue feedback has been given.

@SuperSandro2000

Copy link
Copy Markdown
Member Author

Long build times are probably more effective communicating the "you might need to look into this for your specific use-case" compared to warnings only. So, I'd say that's a good thing.

No, this is the wrong approach. We make the usability of NixOS for certain people very poor and rather drives them into other package managers like Flatpak, other distros or even pinning an older nixpkgs commit instead of offering them real ways to stay informed about security issues and giving them a way to acknowledge security issues and also to ignore them if they are not applicable.

Is that happening? The docs for knownVulnerabilities that I read are this:

Recently packages are rather outright removed if they go EOL within a release cycle even if it is towards the end of it which often forces early migrations and extra stress. That makes very much sense if the EOL date is a few weeks after the release leaving us with a very long period of time with no support but not if the EOL date almost high fives our very next release.

This currently has a label of needs: community feedback - and it leads the PRs with the most 👎 at NixOS/nixpkgs/pulls (is:open sort:reactions--1-desc).

I'd argue feedback has been given.

If you take a look at the down voted and closed PRs, you will notice that a big group of them are regarding "insecure" packages and that we offer no good way to slowly migrate of them without using lots of compute on individual end users computers. This is a big topic for many people and we have not adequately addressed it.

hydraPlatforms = [ ]; doesn’t stop builds on Hydra, it only prevents them from being created as their own jobs

This is a very unintuitive behavior and does not match most user expectations. When doing nix-build then you are prompted for those cases and IIRC the package rebuild count is also not honoring those packages. I don't think hydra should treat packages much different as we do when creating PRs as we otherwise might unintentionally break packages without having any chance of knowing it which is very frustrating.

@wolfgangwalther

Copy link
Copy Markdown
Contributor

Is that happening? The docs for knownVulnerabilities that I read are this:

Recently packages are rather outright removed if they go EOL within a release cycle even if it is towards the end of it which often forces early migrations and extra stress. That makes very much sense if the EOL date is a few weeks after the release leaving us with a very long period of time with no support but not if the EOL date almost high fives our very next release.

I can see a compromise:

  • implement meta.problems: [RFC 0127] Nixpkgs "problem" infrastructure rfcs#127
  • Make "EOL" a distinct problem from "knownVulnerabilities"
  • Similar to insecure, give an eval failure for EOL software that can be ignored.
  • Keep building EOL packages, but don't build insecure packages.

This is a very unintuitive behavior and does not match most user expectations. When doing nix-build then you are prompted for those cases and IIRC the package rebuild count is also not honoring those packages. I don't think hydra should treat packages much different as we do when creating PRs as we otherwise might unintentionally break packages without having any chance of knowing it which is very frustrating.

Yeah, that's correct. We don't eval insecure packages. We'd need better eval to be able to keep doing that for insecure packages without listing them as hydra-rebuilds and possibly also without building them by default with nixpkgs-review, I'd say. I'm not opposed to that either, but it's not straight forward to implement.

@emilazy

emilazy commented Aug 30, 2025

Copy link
Copy Markdown
Member

The main things we mark as EOL in knownVulnerabilities are highly security‐sensitive things like browsers, JIT engines, cryptography libraries, and so on, where vulnerabilities are both high‐impact and fairly inevitable to occur for a package that doesn’t get updated. (e.g. you do not need to wait four months after the EOL of Qt 5 WebEngine to know that it is going to have severe security vulnerabilities by then – although in reality we did end up neglecting it until then and there were of course multiple severe security vulnerabilities upstream in Chromium by that point.)

Browser engines are also some of the most expensive single builds we have, so the trade‐off of building EOL ones on Hydra seems bad to me, given that – spending significant amounts of Hydra build capacity to make it more convenient for users to wield one of the most dangerous footguns we can offer.

I also think that the UX of removing things that will go EOL during the release cycle ahead of time is much better than having to mark packages knownVulnerabilities late in the release’s lifetime. We don’t treat knownVulnerabilities as breaking, because the issue is present before we slap a notice on it, but it’s still good to avoid it in cases when we know we’d be hassling users with it outside of a defined update point.

I’m not sure what the user expectations thing has to do with hydraPlatforms = [ ];. It’s only that that declaration prevents creating a job for a package but does not prevent other builds from pulling it in; useful semantics or not, it’s what we have.

@wolfgangwalther

Copy link
Copy Markdown
Contributor

Yeah, that's correct. We don't eval insecure packages. We'd need better eval to be able to keep doing that for insecure packages without listing them as hydra-rebuilds and possibly also without building them by default with nixpkgs-review, I'd say. I'm not opposed to that either, but it's not straight forward to implement.

I was wrong here. We do eval insecure packages and they are also listed in the rebuild counts. They are just not built by hydra. nixpkgs-review tries to build them but will fail without explicitly allowing insecure packages - IIRC, these are then listed as "broken" in its output.

So there are no eval regressions to assume for insecure-marked packages.

@SuperSandro2000

Copy link
Copy Markdown
Member Author

I can see a compromise:

  • implement meta.problems: [RFC 0127] Nixpkgs "problem" infrastructure rfcs#127
  • Make "EOL" a distinct problem from "knownVulnerabilities"
  • Similar to insecure, give an eval failure for EOL software that can be ignored.
  • Keep building EOL packages, but don't build insecure packages.

Those are all pretty good ideas to make this problem less painful. 👍🏼

I was wrong here. We do eval insecure packages and they are also listed in the rebuild counts. They are just not built by hydra. nixpkgs-review tries to build them but will fail without explicitly allowing insecure packages - IIRC, these are then listed as "broken" in its output.

So there are no eval regressions to assume for insecure-marked packages.

😆 we should make this somehow less confusing as even we are pretty confused by it and regularly confuse things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.severity: security Issues which raise a security issue, or PRs that fix one 2.status: merge conflict This PR has merge conflicts with the target branch 9.needs: community feedback This needs feedback from more community members. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants