Stream: wg-secure-code

Topic: cargo update CVE check


briansmith (Nov 29 2018 at 21:21, on Zulip):

Idea: When you run cargo update, cargo downloads a list of security issues affecting standard libraries (libcore/libstd) and the compiler. Then cargo build will warn/error when building with a version of rustc that has such a vulnerability.

Alex Gaynor (Nov 29 2018 at 21:23, on Zulip):

Relatedly, there's an issue on cargo audit to have it consider the stdlib.

briansmith (Nov 29 2018 at 21:24, on Zulip):

Example from the recent past: We run cargo update. The update downloads the info for CVE-2018-1000810. Before it is fixed, the info will say that it "affects all versions of the toolchain" and building with any version of the toolchain will warn. When Rust 1.29.1 was released then the next cargo update would download the info that says CVE-2018-1000810 was fixed in Rust 1.29.1. Then building with any version older than Rust 1.29.1 would warn/error and building with Rust 1.29.1 or later would not.

briansmith (Nov 29 2018 at 21:25, on Zulip):

@Alex Gaynor Not just the stdlib, but also the compiler and the rest of the toolchain (Cargo, at least).

briansmith (Nov 29 2018 at 21:26, on Zulip):

e.g. when "fastcall" was miscompiled.

briansmith (Nov 29 2018 at 21:26, on Zulip):

Or the recent borrow checker bugs.

briansmith (Nov 29 2018 at 21:27, on Zulip):

(The borrow checker bugs didn't get CVEs but I argued that they should have in https://www.reddit.com/r/rust/comments/8xqrz5/announcing_rust_1271/e25yugs/)

Tony Arcieri (Nov 30 2018 at 00:54, on Zulip):

std is how the issue is filed, but yes the issue is more general, it's the whole Rust toolchain. anyway here's the issue https://github.com/RustSec/cargo-audit/issues/46

briansmith (Nov 30 2018 at 19:21, on Zulip):

@Tony Arcieri IMO cargo audit shouldn't be a separate command but should instead be built into the cargo update process, in the long term. But otherwise, I agree that that seems to be what I'm talking about.

Tony Arcieri (Nov 30 2018 at 19:31, on Zulip):

FWIW cargo-audit is distinct from cargo because the response from the core team, at the time, was to prototype it out-of-tree, but maybe it's worth attempting to upstream now https://github.com/rust-lang/rfcs/pull/1752

Shnatsel (Dec 01 2018 at 18:18, on Zulip):

I feel cargo-audit is more than a prototype at this point, and it's about time that info got actually shipped to users - be it via cargo, github, crates-audit or all of them.
By the way, performing these checks only on cargo update might not be the best idea. That's what yarn and npm do, but they only work that way because that's the only place they could put that information. We could also do that on cargo build, for example.

Shnatsel (Dec 01 2018 at 18:23, on Zulip):

What people probably care most about is running vulnerable services in production, which is not mitigated by notifying on any cargo command. Some kind of pre-packaged tool that ingests Cargo.lock of a production service and pokes me if something goes wrong would be great. I'm pretty sure 90% of that is already implemented in cargo-audit, all it needs is a way to poke me when something goes wrong - email, tweet, something. Since checking a cargo.lock is not a lot of work, I guess this could even be hosted as a service.

Tony Arcieri (Dec 01 2018 at 18:58, on Zulip):

@Shnatsel re: looking for deployed vulnerable build artifacts, yup, I noted as much in cargo-audit #46

Tony Arcieri (Dec 01 2018 at 18:59, on Zulip):

at one of my previous employers we had an agent which collected the Ruby equivalent of Cargo.lock from production servers and cross-referenced that with the RubySec database

Tony Arcieri (Dec 01 2018 at 18:59, on Zulip):

and autofiled JIRA ( :weary: ) tickets about production vulnerabilities, and closed them when the responsible team deployed a version which resolved the vulnerabilities

briansmith (Dec 01 2018 at 19:04, on Zulip):

@Shnatsel cargo build shouldn't do networking.

Joshua Liebow-Feeser (Dec 01 2018 at 19:04, on Zulip):

Doesn't it already since it can pull down dependencies?

briansmith (Dec 01 2018 at 19:06, on Zulip):

Nobody should use cargo build, only cargo build --locked. cargo build --locked should become the default behavior. If cargo build pulls dependencies then you haven't verified your dependencies are safe to use before running their build script.

Joshua Liebow-Feeser (Dec 01 2018 at 19:07, on Zulip):

While that's certainly reasonable, it also departs significantly from current behavior and the community's attitude towards this stuff. I'm not sure that cargo audit shouldn't follow the same approach - network by default, but optionally locked for folks who either don't have network access, or want a reproducible build, or care more about security, etc.

briansmith (Dec 01 2018 at 19:08, on Zulip):

Pretty sure nobody wants a network ping on every build.

briansmith (Dec 01 2018 at 19:08, on Zulip):

Obviously, if/when cargo build does an implicit cargo update then it would do everything that cargo update does.

Joshua Liebow-Feeser (Dec 01 2018 at 19:08, on Zulip):

I feel like the empirical evidence - that cargo build has that exact behavior - suggests otherwise.

Shnatsel (Dec 01 2018 at 19:09, on Zulip):

I was thinking cross-referencing against a local instance of vulnerabilities database. AFAIK crates.io index is already cached locally in full

Joshua Liebow-Feeser (Dec 01 2018 at 19:09, on Zulip):

@Shnatsel I believe that's the case.

briansmith (Dec 01 2018 at 19:09, on Zulip):

Does cargo build do a networking request on every build, or only when it needs to update dependencies/

Joshua Liebow-Feeser (Dec 01 2018 at 19:09, on Zulip):

Ah, I'm not sure about that.

Joshua Liebow-Feeser (Dec 01 2018 at 19:09, on Zulip):

I think only the latter

Joshua Liebow-Feeser (Dec 01 2018 at 19:10, on Zulip):

I guess a vuln database is meaningfully different in that the latest version is a much more appropriate version to use than whatever version we have installed that works.

Joshua Liebow-Feeser (Dec 01 2018 at 19:10, on Zulip):

For versions of source code, it's a much less important distinction.

briansmith (Dec 01 2018 at 19:12, on Zulip):

I don't think people would accept default behavior of a network ping on every build

Joshua Liebow-Feeser (Dec 01 2018 at 19:12, on Zulip):

Yeah you're probably right.

briansmith (Dec 01 2018 at 19:13, on Zulip):

Especially, if you think there's a possibility that a build.rs or a test is malicious or equivalently buggy, you couldn't even start the build unless/until you've got the response.

Joshua Liebow-Feeser (Dec 01 2018 at 19:13, on Zulip):

Maybe cargo audit could use a heuristic of how frequently it checks? E.g., the DB is considered stale after n hours or something like that, unless you pass a flag to force update.

Shnatsel (Dec 01 2018 at 19:13, on Zulip):

hourly database refreshes should be more than enough for non-production stuff like cargo build

briansmith (Dec 01 2018 at 19:14, on Zulip):

More generally, cargo build could be configurable to indicate how often cargo update should be run.

briansmith (Dec 01 2018 at 19:15, on Zulip):

Not much difference between needing to update a dependency because somebody wrote up a CVE and the developer fixed it, vs. the developer fixed a bug without a CVE.

briansmith (Dec 01 2018 at 19:15, on Zulip):

Especially since what's considered advisory-worthy is very subjective.

Joshua Liebow-Feeser (Dec 01 2018 at 19:15, on Zulip):

Yeah, I think that's reasonable.

briansmith (Dec 01 2018 at 19:18, on Zulip):

e.g. OpenSSL didn't apply for CVEs for a few side-channel bugs that I would consider serious, and they're close to making that their normal policy. And BoringSSL only has one CVE even though there were multiple serious bugs, IIRC.

Tony Arcieri (Dec 01 2018 at 19:26, on Zulip):

@Shnatsel checking against the local database is at least possible, as a copy is kept locally in ~/.git/advisory-db

Tony Arcieri (Dec 01 2018 at 19:26, on Zulip):

err, .cargo

Last update: Nov 11 2019 at 22:15UTC