Nixpkgs: Semantically tag CVE patches

[domenkozar]

I propose we add cvePatches attribute to mkDerivation.

Motivation

Given recent announcement of vulnix, it's clear we need a way to identify if a specific version of software has patches applied. A lot of times we have to patch existing version, for example unzip has 7 CVE patches on 16.03.

Implementation

• It should be a list of CVE patches that should be applied in patchPhase. The name of the patch denotes the CVE being patched
• Stdenv builder would then create $out/nix-support/CVE-BLABLA for each of them. This way CVE scanning software can see what is already applied

Thoughts?

cc @peti @copumpkin @ctheune @edolstra

[vcunat]

• There were some cases when a CVE patch turns out not to fix the CVE completely, but that's probably rare enough to disregard for this purpose.
• Speaking of vulnix, it seems to want to work with _store paths_ and not expressions, which brings some additional complications. For example, we would probably have to store the list somewhere in each output.

[ctheune]

Yes. We were planning on putting them into environment variables that are passed to the builder.
We'd love to work with expressions (or parse trees or _something_) but as we're focussing on looking at things installed on a machine we usually do not have access to the expressions.

[ctheune]

one interesting note is that we look at the derivation files. those are accidentally parsable as Python (yes, haha) and we're able to introspect that. we could also look into the store paths into nix-support, too. that would actually be much more structured and less hacky than "eval" ing .drv files in Python :)

[vcunat]

If you assume you have the derivation, then in its closure there's also the CVE-xxxx-xxxx.patch.drv or the patch directly (depending on how the expression obtained the patch).

[peti]

Having the set of applied CVE patches available as a first-order value in the package set seems like overkill to me. Most CVEs are fixed by virtue of a version update, so there's no patch to begin with. And in those cases where a patch does exist, I'd just call the file "cve-2016-xxx.patch" or write the number in a comment behind the patch's file name.

[domenkozar]

@peti but once you lose .drv you have no idea what CVEs were applied. And most of the time in real world, you don't keep .drv files. It's essential to know that at runtime. Creating empty files should add ~0 overhead and huge gain for accountability.

[peti]

For me, personally, the ability to list all CVE-related patches for any given random binary is not particularly important. I've never needed that before. What I need to know is: what is the state of Nixpkgs revision XYZ (because that's what I have installed). Structured comments or file names in Nixpkgs would be good enough for my purposes.

[ctheune]

Right. That's ultimately two different perspectives on the issue. However, using nix to its full extent does not promise you that you can point to a specific nixpkgs expression. I can also have something in the store that is reachable from an active root that came from a manual nix-build or other thing where we can't automatically determine the source.

[vcunat]

I'd expect that in the cases you care about CVE a lot you typically also want "track" which nixpkgs revisions you use, as the knowledge is useful for other reasons as well. On the other hand, e.g. Debian always installs even a full changelog.

[peti]

using nix to its full extent does not promise you that you can point to a specific nixpkgs expression.

I think it does and I think you can. I manage a set of machines that's very diverse, ranging from desktops to servers and laptops, yet I have no trouble telling for every package I've installed what its source is. Granted, you _can_ install packages in a way that's not reproducible, but I sure hope security-conscious people don't do that much.

[ctheune]

For me that's not about reproducibility. That's separate from knowing where something came from, right? We're providing a co-managed environment where we support our customers installing custom things in a structured way (yay, nix!). We can't guarantee to follow things in the store back to their expressions, though.

[copumpkin]

If we can get deterministic (bit-for-bit) builds and https://github.com/NixOS/nix/pull/709 merged in, we could actually tie all builds back to the source that produced them. That's a decent amount of work though 😄

[peti]

We're providing a co-managed environment where we support our customers installing custom things in a structured way.

It seems to me like the target group for the extension you're suggesting is Nix users who are (a) security conscious and (b) install things into their system where they have no clue what the source code was. I don't know ... that seems like a small set of people. Just my 2 cents.

[Phreedom]

+1 to this, although for
nixpkgs-monitor scanning
patch names would be
enough

[ctheune]

Heh, let's rephrase, I did mean something different. My point is that technically there will easily be cases where someone did not leave the expression around after something was installed. I don't follow that just because someone is security conscious means that on the system where a package is installed the expression must remain present after it was installed.

[domenkozar]

I'd like to summarize observations so far:

• CVE doesn't always map to a version bump or a patch

I'd like to see examples of this to really address it. Hopefully the upstream issues a new CVE if existing patch doesn't fix it, if not I don't really see a way to avoid this

• in general (as Peter noted) we'd like to reason about security based on .drv files where this information is accessible already

In multi-user managed machine (remember, Nix promotes this as a feature, everyone can install any nix expression as a user), we currently don't have a way to track back all built derivations to their source.

Therefore having cvePatches generate $out/nix-support/CVE-XXX would barely add any overhead while giving tools like vulnix a chance to better reason about CVEs. I understand not everyone will benefit from that, but my argument is that there are no cons, just pros.

[ckauhaus]

I'd like to pick up the discussion. vulnix is now able to recognize CVE patches if they are named accordingly. I think this presents a practical solution for Domen's first point in the original description.

This means:

• If a package has published vulnerabilities, but the derivation contains patches that have CVE identifiers in their file names, these vulns are filtered out. Example: unzip
• If upstream releases a fixed version which is not listed in the CVE database and we update, everything is fine anyway. Example: postgresql
• If a single patch fixes several CVEs, its file name should contain all of them.
• If all of the above fails, we can still employ vulnix' whitelist feature NixOS-wide. Note that the whitelist feature is currently not ready for general use.

Comments?

[ckauhaus]

See #39392 for related discussion. We should probably merge the two issues.

[ckauhaus]

I'd propose to close this issue. Discussion on NixCon has shown that including CVE names in patches is a workable practice. This will not cover every case, so the points being discussed in #39392 are still relevant. But we should focus discussion there and not here.