Following a suggestion by @qyliss, I investigated checking the Cargo.lock files of the various buildRustPackage derivations we have in nixpkgs against RustSec’s advisory-db. Using cargo-audit and a bit of glue code, this turned out to be not too hard.
Of course this type of checking is prone to a lot of false positives: A vulnerability in a dependency doesn’t necessarily mean the dependent package is vulnerable as well. The report at least seems to confirm the concerns of “The modern packager’s security nightmare”: Cargo’s lock files and vendoring have lead to for example multiple vulnerable openssl versions coming up in the dependency graph of packages — even though these have been long patched in our normal openssl package.
Feel free to report any issues or suggest improvements in this thread. Additionally I’d be interested in preventable false positives.
I noticed a recurring pattern after manually triaging some of the crates; cargo features play a large role.
Many of the openssl-src vulnerabilities do not apply for nixpkgs because vendoring openssl is often exposed as a non-default feature flag (e.g. cargo-outdated, cargo-generate). Some crates do not even have any vulnerabilities after accounting for disabled features (e.g. cargo-flash, cargo-embed).
Applying the feature flags to the crates to determine which dependencies should be included in the audit would help a lot with the signal to noise ratio. Regardless, this is very useful information, nicely done!
Indeed, I’ll add an extra check for openssl (probably by looking at what the resulting binary links against), cargo features in general are maybe a bit trickier, I’m not sure how we can account for those without a ton of effort, but I also haven’t looked into it yet, really.
Small update: I added a way for maintainers to be pinged when one of their packages has a warning. I made this an explicit whitelist due to the amount of false positives.
If you want to be on there, message me on libera (sterni) or matrix (@sternenseeman:systemli.org) or here. Or email me a patch, I guess.
When I have a bit of motivation again, I’ll work on a better heuristic for OpenSSL where any warnings for openssl-src is dropped if the package dynamically links against the normal nixpkgs openssl package.
Ignoring the determinism issues, should we actually be respecting the Cargo.lock from source trees? cargo install ignores them by default, for example.
I would love to do that as well, however, with the way we build rust packages this is impossible at the moment. Since we rely on fixed output derivations for obtaining the vendored dependencies, determinism is not optional unfortunately. All we can do is basically equivalent to patching the lock file (and thus annoying manual work).
There is a world where we could do this neatly, but I think it requires having a sort of rust crates package set and adding some (preferably not very) clever constraint solving mechanism, I would expect.