Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "xz: 5.4.6 -> 5.6.1" #300028

Merged
merged 2 commits into from Mar 29, 2024
Merged

Conversation

mweinelt
Copy link
Member

@mweinelt mweinelt commented Mar 29, 2024

Description of changes

The upstream tarball has been tampered with and includes a backdoor for
which we cannot completely rule out, whether we are affected.

https://www.openwall.com/lists/oss-security/2024/03/29/4

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.

@SuperSandro2000
Copy link
Member

We should probably leave a comment to not update to 5.6.0/1 by rrynatm or someone not being aware of this.

@dotlambda dotlambda changed the title Revert "xz: 5.6.0 -> 5.4.6" Revert "xz: 5.4.6 -> 5.6.1" Mar 29, 2024
This reverts commit c53bbe3.

The upstream tarball has been tampered with and includes a backport for
which we cannot completely rule out, whether we are affected.

https://www.openwall.com/lists/oss-security/2024/03/29/4
This reverts commit 5c7c19c.

The upstream tarball has been tampered with and includes a backport for
which we cannot completely rule out, whether we are affected.

https://www.openwall.com/lists/oss-security/2024/03/29/4
@Baughn
Copy link
Contributor

Baughn commented Mar 29, 2024

Is there any way to pull 5.6.0/5.6.1 from the build cache?

@mweinelt mweinelt changed the base branch from staging to staging-next March 29, 2024 16:53
@mweinelt mweinelt marked this pull request as ready for review March 29, 2024 16:57
@mweinelt
Copy link
Member Author

Is there any way to pull 5.6.0/5.6.1 from the build cache?

Yes, but highly impractical, so unlikely to happen.

@LunNova
Copy link
Member

LunNova commented Mar 29, 2024

responding to a now deleted comment which said the tarballs aren't backdoored:

It's using the tarballs associated with the github release and not the tarball created by github for a tag, https://www.openwall.com/lists/oss-security/2024/03/29/4 indicates that those are backdoored.

@simonhollingshead
Copy link
Contributor

We should probably leave a comment to not update to 5.6.0/1 by rrynatm or someone not being aware of this.

Would it be worth, at least temporarily, adding it to the skiplist to avoid the risk of someone accepting a bot-proposed update?

https://github.com/ryantm/nixpkgs-update/blob/main/src/Skiplist.hs

@delroth delroth merged commit d6dc19a into NixOS:staging-next Mar 29, 2024
11 of 13 checks passed
@mweinelt
Copy link
Member Author

mweinelt commented Mar 29, 2024

A committer of theirs added the exploit to the git repo disguised as test artifacts. Why should we trust anything else they have done? Reverting to 5.4.6 and waiting for further analysis is the way we go.

@mweinelt mweinelt deleted the return-to-xz-5.4.6 branch March 29, 2024 17:20
@Lassulus
Copy link
Member

maybe we should mention CVE-2024-3094

LeSuisse added a commit to LeSuisse/nixpkgs that referenced this pull request Mar 29, 2024
This is a follow-up to the downgrade to version older than 5.6.x made in NixOS#300028
(also known as CVE-2024-3094).
A suspicious commit made by the same actor has been spotted in
libarchive and following up discussions a change has been made by
contributor and merged by another maintainer.
LeSuisse added a commit to LeSuisse/nixpkgs that referenced this pull request Mar 29, 2024
This is a follow-up to the downgrade to version older than 5.6.x made in NixOS#300028
(also known as CVE-2024-3094).
A suspicious commit made by the same actor has been spotted in
libarchive and following up discussions a change has been made by
contributor and merged by another maintainer.
@nonetrix
Copy link

nonetrix commented Mar 29, 2024

I feel like security issues like this should be put into unstable quicker is there not a process for this? I am running a infected version seemingly and just updated my system after discovering this, seems my only option is to switch to master branch

❯ xz -V
xz (XZ Utils) 5.6.1
liblzma 5.6.1

@imadnyc
Copy link
Contributor

imadnyc commented Mar 29, 2024

@nonetrix Until someone who is more knowledgeable about the Nixpkgs process replies I think it would be smart to overlay it with the correct version

@norpol
Copy link
Contributor

norpol commented Mar 29, 2024

I feel like security issues like this should be put into unstable quicker is there not a process for this?

You can follow the discussion on the NixOS Discourse.

[...] am running a infected version seemingly [...]

It appears to be not in an exploitable fashion for NixOS, due to built-time checks that specifically targeted deb/rpm systems. We'll have to wait for the first analysis of all the commits that were made since the malicious actor joined the xz project.

@Hayajiro
Copy link
Contributor

I feel like security issues like this should be put into unstable quicker is there not a process for this? I am running a infected version seemingly and just updated my system after discovering this, seems my only option is to switch to master branch

Given the currently available information, the version of xz 5.6.1 that's available in nixpkgs does not contain the exploit code. And even if it would, it wouldn't trigger on the host systems due to the condition that argv[0] == /usr/sbin/sshd. As such, forcefully pushing this before it's available in the binary cache (resulting in massive rebuilds for everyone) seems unreasonable.

However, I understand (and feel the same) that there should be mechanisms in place for when a critical issue arrises that does affect NixOS. Perhaps it is an option to update nixpkgs even though binaries aren't available yet? That way, users would get the update ASAP, even if this means local rebuilds for the time being.

Another thing I've noticed: Did the security team not get a notice about this issue beforehand? Distros like Arch got a security notice at least one day before the vulnerability was disclosed. Or was this a deliberate decision given that binaries from nixpkgs don't seem to be affected?

LeSuisse added a commit to LeSuisse/nixpkgs that referenced this pull request Mar 30, 2024
This is a follow-up to the downgrade to version older than 5.6.x made in NixOS#300028 (also known as CVE-2024-3094).
A suspicious commit made by the same actor has been spotted in libarchive.
Reverting the change seems to be acceptable.
LeSuisse added a commit to LeSuisse/nixpkgs that referenced this pull request Mar 30, 2024
This is a follow-up to the downgrade to version older than 5.6.x made in NixOS#300028 (also known as CVE-2024-3094).
A suspicious commit made by the same actor has been spotted in libarchive.
Reverting the change seems to be acceptable.
@imadnyc
Copy link
Contributor

imadnyc commented Mar 30, 2024

I thought it was in nixpkgs? #300028 (comment). Also, as of right now we only know that it will trip when argv[0] == /usr/sbin/sshd, not only when it's that, so there might be more cases. And maybe I'm mistaken, but couldn't this affect edge cases where it's being ran in an FHSenv?

I do agree with the sentiment of having protocols in place for the future. As for the heads up, from what I can tell it was one person who discovered it and released it publically so the malicious actor couldn't do more to hide it, so I think it may have just been that they weren't aware of NixOS. Of course, I'd like a response from someone on the public team themselves.

@Hayajiro
Copy link
Contributor

Hayajiro commented Mar 30, 2024

I thought it was in nixpkgs?

The source tarball seems to be affected, but the resulting binary doesn't seem to contain the respective exploit code. At least on my machine, I can't find the signature from the openwall report in my liblzma binaries.

Either way, information seems to be too scattered right now to say anything for certain. But given that, with currently available information, we don't seem to be at immediate risk, perhaps it'd be better to let the dust settle and take according measures once we have a clear picture.

@oxalica
Copy link
Contributor

oxalica commented Mar 30, 2024

The xz repository is now disabled on GitHub. This PR now failed to fetch src for me (hash mismatch). I think we need to seek some alternatives and push it to master for the fix.

@Baughn
Copy link
Contributor

Baughn commented Mar 30, 2024

Debian decided to roll back to 5.3.1, as 5.4.6 still includes a lot of potentially compromised commits: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1068024

Perhaps they could be used as a source repository. That rollback is not going to be easy, however.

I assume dropping the dep on xz entirely is functionally impossible?

@Maryse47
Copy link

Maryse47 commented Mar 30, 2024

there is also mirror hosted by original xz maintainer https://git.tukaani.org/?p=xz.git;a=summary

@physics-enthusiast
Copy link

physics-enthusiast commented Mar 30, 2024

However, I understand (and feel the same) that there should be mechanisms in place for when a critical issue arrises that does affect NixOS. Perhaps it is an option to update nixpkgs even though binaries aren't available yet? That way, users would get the update ASAP, even if this means local rebuilds for the time being.

+1 on this. If I understand this correctly, the binary cache was always meant to be a convenience feature to save a bunch of time rebuilding on an otherwise source-based system, rather than an integral part of the functioning of Nix. From that perspective, I think one could argue that the additional time spent on a rebuild is very much worth avoiding a security vulnerability for anyone who pulls in the affected package. On a related note, is anyone aware of the existence of any centralised location to check if any given Nixpkgs commit contains vulnerabilities that were fixed in a later version (for those who do not regularly keep track of Github Issues)?

@vcunat
Copy link
Member

vcunat commented Mar 30, 2024

Nothing prevents you from following staging-next for example, if you want to rebuild stuff yourselves.

As for mechanisms that avoid rebuilds, there is NixOS option system.replaceRuntimeDependencies. I believe it's not commonly used by people; I'd say it's a bit cumbersome. With critical vulnerabilities you can sometimes find people posting snippets how they use it for that particular case.

@vcunat
Copy link
Member

vcunat commented Mar 30, 2024

Source switched to a working mirror in 6aa50d0. GitHub overreacted and made things worse IMHO, but "fortunately" switching URLs is rebuild-free.

@13x1
Copy link

13x1 commented Mar 30, 2024

Current HN discussion mentions that a lot of the older commits from this guy also look suspicious, across other projects and potentially as far as two years back. I don't want to alarm anyone, but I think this'll be a fun week.

FYI, should something be discovered in 5.4.3 (also signed by the same person), we need to change pkgs/os-specific/linux/minimal-bootstrap/xz too, because the bootstrap derivation doesn't depend on the derivation in this PR.

@physics-enthusiast
Copy link

physics-enthusiast commented Mar 30, 2024

Nothing prevents you from following staging-next for example, if you want to rebuild stuff yourselves.

As for mechanisms that avoid rebuilds, there is NixOS option system.replaceRuntimeDependencies. I believe it's not commonly used by people; I'd say it's a bit cumbersome. With critical vulnerabilities you can sometimes find people posting snippets how they use it for that particular case.

Those may be decent measures for someone who regularly checks on their Nix/NixOS installation (manually picking staging-next commits that build for them and/or adding each vuln to system.replaceRuntimeDependencies) but I don't think that's necessarily a fair expectation to impose on regular users who just want to not have remote holes in their system. While keeping up with cybersecurity news could be said to be an important part of ensuring a system is secure, IMHO it's not a good look for us either (in the currently-hypothetical but arguably-inevitable future where one of these hits us directly) to continue to serve a known malicious package to unsuspecting updaters (who may have just run nix flake update and nixos-rebuild without paying attention) for ten days while staging-next gets pushed to master and then to nixos-unstable. And given that the scale of the problem will only grow as Nixpkgs gets larger and mass-rebuilds take longer, it might be good to start preparing a plan now.

Just throwing ideas out there, but possible compromises could include:

  • New -rapidfix branches for those who want to opt in to immediate security fixes even if they have to recompile everything, but don't want to play commit roulette with staging-next (whose explicit purpose is to work through major breaking changes). It would basically be a middle ground between the nixos- branches and staging-next where the aforementioned patches would be merged as soon as they evaluate properly but mundane breaking changes and refactors will not.
  • A significantly cut-down Hydra jobset just for critical, time-sensitive changes. Test that NixOS boots, a curated selection of the most used packages (like DEs, browsers, programming language compilers etc.) build, the most important stuff run, and that's it. Not sure how hard this would be to implement in practice.
  • An "emergency build fund" for temporarily renting build capacity in the cloud. No clue how much this would cost or who would pay for it.

@eliminmax
Copy link

Debian decided to roll back to 5.3.1, as 5.4.6 still includes a lot of potentially compromised commits: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1068024

Perhaps they could be used as a source repository. That rollback is not going to be easy, however.

I assume dropping the dep on xz entirely is functionally impossible?

Debian contributors are discussing that possibility, it's not definitive yet as far as I can tell.

Disclaimer: I'm a Debian user trying to make sense of the situation, not (yet) a Debian contributor, and not a Nix user.

@SuperSandro2000
Copy link
Member

New -rapidfix branches for those who want to opt in to immediate security fixes even if they have to recompile everything

I don't think you understand the significance of rebuilding the bootstrap tools. Even with a build farm and tuned remote builders this can easily take an entire day or more which is extremely impractically for anyone to use. It arguable way easier to ditch the current staging-next branch and replace it in such situations.

Using system.replaceRuntimeDependencies in those situations is easier. Or if you just want to replace xz in openssh, you can work with overlays and get down to probably just a few rebuilds.

And given that the scale of the problem will only grow as Nixpkgs gets larger and mass-rebuilds take longer, it might be good to start preparing a plan now.

Reducing the size of stdenv or the reverse deps of key packages is helping that. We already did that for openssh in the past and now updates to it can go straight to master instead of staging first.

A significantly cut-down Hydra jobset just for critical, time-sensitive changes. Test that NixOS boots, a curated selection of the most used packages (like DEs, browsers, programming language compilers etc.) build, the most important stuff run, and that's it. Not sure how hard this would be to implement in practice.

That already exists and is called nixos-unstable-small.

An "emergency build fund" for temporarily renting build capacity in the cloud. No clue how much this would cost or who would pay for it.

That likely does not help as much as you want it to. xz is a dependency of bootstrapping things and many things in there are not as parallelized. Also adding those builders takes time and hydra is not great at greatly utilizing them and distributing builds.

@physics-enthusiast
Copy link

physics-enthusiast commented Mar 30, 2024

I don't think you understand the significance of rebuilding the bootstrap tools. Even with a build farm and tuned remote builders this can easily take an entire day or more which is extremely impractically for anyone to use. It arguable way easier to ditch the current staging-next branch and replace it in such situations.

I've had Actions Runners time out after only making it to bootstrap-stage2-stdenv-linux when attempting emulated "cross"-compilation, so I'm very much aware. But surely bootstrap-tools + only the packages you need > bootstrap-tools + all 200k dependents. A day is still better than 10, especially since I'm not suggesting we force everyone onto this branch, just that it be an option. If this happens regularly it would indeed be impractical, but (for example) RCE in a package fundamental enough to cause such a rebuild would justify it, no? When there is no ongoing crisis nixos-unstable-quickfix would mirror nixos-unstable, so suddenly having long build times could actually double as an indicator that something is wrong.

Using system.replaceRuntimeDependencies in those situations is easier. Or if you just want to replace xz in openssh, you can work with overlays and get down to probably just a few rebuilds.

Again, that is a fine solution for those of us who are already here. Having nix flake update give an infected package to anyone not paying attention for 10 days after we find out is still a bad look.

Reducing the size of stdenv or the reverse deps of key packages is helping that. We already did that for openssh in the past and now updates to it can go straight to master instead of staging first.

That helps with the build times, but the fundamental problem remains that we are delaying vital security updates by rebuilding the universe for the sake of a convenience feature, and that universe is only going to get bigger.

That already exists and is called nixos-unstable-small.

Aren't merges to that branch also blocked by staging-next? Not a rhetorical question, I haven't looked too closely at the -small channels yet.

That likely does not help as much as you want it to. xz is a dependency of bootstrapping things and many things in there are not as parallelized. Also adding those builders takes time and hydra is not great at greatly utilizing them and distributing builds.

Fair enough.

Apologies if I'm coming off as a bit too harsh, just legitimately perplexed that there is no fast-ring with just the critical fixes. It seems to me (and I could be wrong) that as of now the only options are a) get patches for exploitable code more than a week late, or b) regression beta-tester.

NovaViper added a commit to NovaViper/NixConfig that referenced this pull request Mar 30, 2024
Switched to nixpkgs staging-next to address NixOS/nixpkgs/pull/300028

Flake lock file updates:

• Updated input 'home-manager':
    'github:nix-community/home-manager/179f6acaf7c068c7870542cdae72afec9427a5b0' (2024-03-27)
  → 'github:nix-community/home-manager/c0ef0dab55611c676ad7539bf4e41b3ec6fa87d2' (2024-03-28)
• Updated input 'nix-gaming':
    'github:fufexan/nix-gaming/63ded1ffc0846c259f9cd0f62aa2ea9f7f804f56' (2024-03-27)
  → 'github:fufexan/nix-gaming/350a57349ed40f0166e2afaf1d5c1ee955c53111' (2024-03-29)
• Updated input 'nixpkgs':
    'github:nixos/nixpkgs/fd84c1ff8937685294342c57a656a7066800d01c' (2024-03-26)
  → 'github:nixos/nixpkgs/48d06167c6d80ba706833f8f76f8a8fe7c33786c' (2024-03-30)
• Updated input 'nur':
    'github:nix-community/NUR/e9fb06187654059328b8a266ea9a6d2f36cf1cf6' (2024-03-28)
  → 'github:nix-community/NUR/dd0c7591edf10dbe65223bb7353927b2a1f1b41e' (2024-03-30)
• Updated input 'plasma-manager':
    'github:pjones/plasma-manager/298f345f3ca9528ca88dbbbdcadfc270b7582ded' (2024-03-27)
  → 'github:pjones/plasma-manager/25b222a95a764bb76b7082746e8a1115c46ae2d8' (2024-03-30)
@LunNova
Copy link
Member

LunNova commented Mar 30, 2024

Might be better to raise a separate issue for an extended discussion of rapidly hotfixing when many rebuilds are involved.

As far as I know this revert was out of precaution but there's not a rush to jump to it because the nix build of liblzma didn't have the backdoor injected. There's more discussion in https://discourse.nixos.org/t/cve-2024-3094-malicious-code-in-xz-5-6-0-and-5-6-1-tarballs/42405?u=lun and there was some on matrix earlier.

@Sepero
Copy link

Sepero commented Mar 30, 2024

I definitely agree that more discussion is needed. NixOS lucked out this time. In the future we might not be so lucky, and it might be a much larger Exploit. Security patches need to come out instantly.

@K900
Copy link
Contributor

K900 commented Mar 30, 2024

This conversation is going way off topic. Future policy discussion should not be done on this PR.

@NixOS NixOS locked and limited conversation to collaborators Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet