securedrop icon indicating copy to clipboard operation
securedrop copied to clipboard

Determine plan for auditing Rust dependencies

Open legoktm opened this issue 1 year ago • 1 comments

As part of using Sequoia, we'll start pulling in Rust dependencies (161 crates by my current count!). The process for performing "diff reviews" of Python dependencies is documented on the wiki.

We may want to take advantage of some Rust tools that allow for sharing audit status like:

  • https://github.com/crev-dev/cargo-crev
  • https://github.com/mozilla/cargo-vet (see LWN coverage: https://lwn.net/Articles/897435/)

legoktm avatar Jul 21 '22 17:07 legoktm

I reviewed both the cargo-crev and cargo-vet documentation, and while it seems less mature, my preference is for cargo-vet.

cargo-crev is designed for individuals doing reviews, each person gets a crev ID and hosts their own crev proof repository. While cargo-vet seems more aligned with trusting orgs for reviews.

cargo-crev is built on the web of trust, so when you trust person, you now implicitly trust all the other people they trust. I think that's fine for casual projects, but I think we're a lot more paranoid than that! I imagine we would trust the audits done by Mozilla/Firefox since if we don't trust them we're already pwned, but not necessarily every person who someone who works on Firefox trusts.

And ultimately cargo-vet seems conceptually simpler, there's no cryptography involved or keys or IDs, it just relies on git repos (so HTTPS).

legoktm avatar Aug 01 '22 21:08 legoktm

I tried out cargo vet today, it seems to work pretty nicely. The main "issue" I ran into was that we are deploying to a very specific platform ("target" in Rust lingo), Linux x86_64. There are a bunch of Windows/WASM/Redox dependencies in the vet list that we don't actually use that I filtered out manually based on https://github.com/mozilla/cargo-vet/issues/63#issuecomment-1382836458. That ticket has some suggestions on how upstream can implement some form of target limiting.

legoktm avatar Jan 30 '23 17:01 legoktm

Thinking closely about the Python diff review workflow, there are two steps missing:

  • We don't verify that the version we review matches checksum of what is included in Cargo.lock. This probably needs to be fixed upstream.
  • There's no opportunity to PGP sign the review. If we consider this to be a requirement, I would suggest we just mandate GPG-signing of commits that add new audits.

legoktm avatar Jan 30 '23 23:01 legoktm

I think we're mostly set on going forward with cargo vet, but FWIW Google has also adopted it: https://opensource.googleblog.com/2023/05/open-sourcing-our-rust-crate-audits.html

legoktm avatar Jun 06 '23 02:06 legoktm

@legoktm, thanks for your research and recommended links here. I've read the cargo-vet manual and like both the trust model it implements and the review ergonomics it offers.

Re: https://github.com/freedomofpress/securedrop/issues/6500#issuecomment-1409539888 (modulo my conversion to ordered list :-), the cargo-vet team has addressed both of these topics in mozilla/cargo-vet#449:

Thinking closely about the Python diff review workflow, there are two steps missing:

  1. We don't verify that the version we review matches checksum of what is included in Cargo.lock. This probably needs to be fixed upstream.

Their claim:

Cargo-vet primarily operates on releases from crates.io, for which digests are published in the public append-only crates.io registry index. In other words, the index maps (crate, version) to digest, and so by depending on the index, cargo-vet can map that tuple to digest without including the digest inline.

I agree with you that this still requires trusting cargo (as we do for pip) and crates.io and the index (as we do not do for pypi.org). However, it appears to be a deliberate design decision.


  1. There's no opportunity to PGP sign the review. If we consider this to be a requirement, I would suggest we just mandate GPG-signing of commits that add new audits.

Their claim:

Cargo-vet operates at the granularity of organizations/projects rather than individuals. Organizations can (and sometimes do) cryptographically bind audits to authors (by requiring them to sign the relevant git commit) but cargo-vet operates only on the audit file, not any VCS history that may have been involved in creating it.

To feel comfortable with this, I think we'd want to upgrade our "commits should be signed" to "commits MUST be signed", enforced by CI.

See also:

  • rust-lang/rfcs#2474
  • rust-lang/rfcs#3403

cfm avatar Jun 21 '23 23:06 cfm

To feel comfortable with this, I think we'd want to upgrade our "commits should be signed" to "commits MUST be signed", enforced by CI.

If we want to enforce it by CI (a good idea IMO), then I think we should just sign the relevant files, similar to how we sign sha256sums in securedrop-builder, so we're not dependent on tracking the entire git history to ensure it hasn't been touched by an unsigned commit, we can just check the latest state.

legoktm avatar Jun 22 '23 21:06 legoktm

Not that I'm any kind of stakeholder here, but it seems to me that if you consider commit signing important enough to enforce for third-party code integrity, it would make sense to enforce it for first-party code as well.

bholley avatar Jun 26 '23 17:06 bholley

Generating signed hashes for the audit files seems like an extra hurdle, I'd be in favour of adding an overall commit signing policy - only downside I can see is inconvenience/barrier to entry.

Failing that, a more lightweight approach would be to define code ownership for said files and ensuring the owner group is a) signing their commits and b) comfortable reviewing changes to the cargo vet audits.

zenmonkeykstop avatar Sep 22 '23 22:09 zenmonkeykstop

We have some discussion re: signing policy at https://github.com/freedomofpress/securedrop/issues/6943. Ultimately since these audits are all being stored in the Git history and attributable, I don't think we need to require PGP signatures (in contrast our diff reviews are posted on wiki pages without any pre-review).

Also, @bholley, thanks for commenting and your work on cargo-vet :) I've put up our initial configuration at https://github.com/freedomofpress/securedrop/pull/6981 if you're interested in taking a look.

legoktm avatar Oct 10 '23 20:10 legoktm