Rust might be particularly exposed in that merely building a project can execute arbitrary code from any dependency via build scripts and proc macros. There are efforts to address this problem, but as far as I can see, a solution is not imminent;
I’ve been working on what I hope is a pragmatic remedy to these problems:
While working on this, I realized that in addition to simply sandboxing Cargo, a meaningful security improvement can be obtained by separating fetching and building so that, for example, an invocation of cargo build is split into two phases:
A first phase that roughly runs cargo fetch in a sandbox with network enabled.
A second phase that runs cargo build --locked in a sandbox with network disabled.
The security gain from the above is that if a compromised dependency tried to exfiltrate user data over the network (even if a sandbox is used, sensitive project data might be available), it would have a much harder time doing so.
The hard problem is reproducing exactly the fetches that a particular Cargo/rust-analyzer invocation would perform, including features, targets, manifests, sysroot/library details, etc. The Cargo and rust-analyzer wrappers that I’ve been working on try their best, but with current Cargo this is necessarily a brittle affair. For example, truly replicating the fetches that some cargo build invocation would perform without running cargo build seems very difficult currently. The same problem exists for rust-analyzer’s fetches. My wrapper has to resort to running magic commands like
I wonder whether it would be a good idea to solve this brittleness by adding support for such sandboxing/separation to Cargo itself. Cargo could, for example, gain a --fetch-only option. Running cargo --fetch-only build would perform exactly the fetching that cargo build would otherwise perform, but stop before building. Rust-analyzer could gain a --fetch-only option as well.
Even without network during the build, compromise can be deferred to test time (which almost inevitably follows) where network is less likely to be completely disabled. I'm not saying this is nothing, but I think it needs to be part of a comprehensive design of how to allocate and enforce permissions to stages of the build pipeline. For example, I know that some database proc macros reach out to connect to a database to divine the schema during the build; how does one indicate that this permission is necessary? Can that permission be limited to that component of the build somehow?
Also, this sounds like either a Linux-specific or CI-specific thing as I'm not sure how to "disable network" for a specific process on macOS (probably through the sandboxing framework?), Windows (???), FreeBSD (jails?). Even on Linux, you're at the mercy of user namespaces being available in order to create a network namespace you can manipulate.
I was surprised that rust-analzyer fetches dependencies of the stdlib (and not just of the project being analyzed). I naively assumed that rustup already installs everything that is needed for using the stdlib. It turns out that it installs everything that is needed for building with the stdlib, but not for rust-analyzing it. (Or at least this is my understanding.)
Since I was getting warnings about the wrapped rust-analyzer not being able to fetch what it needs, I added code that tries to emulate what rust-analyzer itself does. I took the above from rust-analyzer-master/crates/project-model/src/sysroot.rs:369
let mut cargo_config = cargo_config.clone();
// the sysroot uses `public-dependency`, so we make cargo think it's a nightly
cargo_config.extra_env.insert(
"__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS".to_owned(),
Some("nightly".to_owned()),
);
I do not claim to have a deep understanding of the involved mechanisms, but it seems to work at least in my testing.
In any way, whether __CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS=nightly should be set or not is not my main point.
Sure, I fully agree with the different parts of your assessment. I still hope that these wrappers provide a meaningful contribution to defense in depth.
By the way, many programs do not need network for tests. When launched with skn-cargo test, tests will not have network, unless the user opts-in explicitly with skn-cargo +N test.
Perhaps, in an ideal world, Cargo could expose a stable API with a more comprehensive internal model of operations and required effects. The Cargo subcommands would then be just one way to use it, and tools such as rust-analyzer would not have to approximate this by shelling out to CLI Cargo.
The --fetch-only idea is just one possible step towards this general goal. It won’t magically make Cargo safe, but I believe that it would be a useful building block whenever sandboxing primitives are available.
Is cargo vendor not sufficient for this? It's how we make sure that all jobs get the same set of dependencies even if rerun hours later (all cargo commands use --offline --locked after the initial cargo update && cargo vendor job)
A simple cargo fetch often works, but I am not aware of a way to limit it to a particular set of Cargo features. In particular, I believe that there is no way to make it aware of the features needed for a particular cargo build. So it tends to fetch too much, which may or may not be a problem.
But cargo fetch can also fail to fetch enough for a particular cargo build. For example, suppose a crate has a direct dependency named foo, and foo has
which activates a feature of the direct dependency. But cargo fetch has no corresponding --features option, so a wrapper using cargo fetch alone cannot say “fetch exactly as that build would fetch”. If rustls is not already cached, the later --frozen build can fail.
Even if one disregards such cases and if one does not worry about fetching more than necessary, making a wrapper cover most of Cargo’s command line interface (e.g. +nightly, --config, -C, etc.) is quite complicated.
In the two-phase fetch-then-build-workflow, --frozen is already close to the second phase: run the requested command without allowing network access or lockfile changes. The missing first phase is a way to ask Cargo to perform exactly the fetching that the command would need, then stop.
In this way --fetch-only would be a natural addition to the existing --locked, --offline, and --frozen options of Cargo. It would make it easy for wrappers like yours or mine to separate fetching from building cleanly and robustly.
According to my understanding cargo vendor suffers from the same problem as cargo fetch with regard to features (see above).
It also requires specific config and creates a separate vendor/ tree, making it as far as I can see not fit as a building block for a two-phase cargo build replacement that otherwise behaves as the original.
Hmm, I've never noticed cargo build not fetching the entire Cargo.lock before, but then I guess I never normally try to do a smaller build as the first step in a new project, I would start with a complete cargo check and so I wouldn't ever see it pulling extras.
Ah, right, I have run into such a situation before; I just disregard that as feeling like a bug in Cargo's CLI. It shouldn't be possible to activate a feature on a non-workspace crate during a particular build since the Cargo.lock doesn't contain the info necessary to do so. I assume it will just pull the latest of the unactivated optional dependencies each time?
cargo vendor will vendor all crates included in the lockfile by default, even if no combination of target and features can actually build a given crate.
If you have a custom registry (and a security minded focus) you can probably manage everything in your custom registry and then block internet access completely. Or you can use a tls terminating sandbox and limit access to specific repositories on e.g. github.com.
I'm not sure if this is still an issue or if it was blown out of proportion, but I recall an article making the rounds last year claiming that cargo vendor can execute arbitrary code.
That article is about Cargo loading a config file from a repo. If your concern is not with the repo in front of you but dependencies, then that article is not relevant. Cargo does not load config files from dependencies.