xous-core
xous-core copied to clipboard
Can't verify that packages from crates.io have a correct Cargo.toml
We've been splitting crates into separate API crates and implementation crates to facilitate better portability for Xous to new platforms.
One result of this is that the API crates, along with the kernel and key things like the IPC crate, are now sourced from crates.io during a build.
There's a check for project consistency that's been added (currently runs on every build, but this may be changed) to confirm that what we have in our local tree matches what's on crates.io:
https://github.com/betrusted-io/xous-core/blob/5c0154f730cb4acbe328397bd4e03ef3793df728/xtask/src/verifier.rs#L10-L23
The main purpose of this is to make sure that we've pushed any local changes to crates.io, so that other builds dependent on the clean state of Xous don't break (at the moment, xous-core itself refers to the crates on crates.io, so Xous would be counted as among the programs that have this dependency). In other words, what we want to avoid is me forgetting to push a change to crates.io after making a local change and testing it.
This works fine for source files, but, there is a huge caveat. Cargo.toml files are put through a lot of grooming before they are uploaded to crates.io:
https://github.com/rust-lang/cargo/blob/928561a0c539c1f96cbe72ca5f8c52d026221e98/src/cargo/util/toml/mod.rs#L1349
For example, log = 0.4.14
is converted to log = {version = "0.4.14"}
by the crate packing code. This fundamentally alters the structure of the Cargo.toml file. It also defeats any trivial attempt to just read in the Cargo.toml file as a syntax tree and compare the leaf nodes, because the comparison routine needs to be aware of these kinds of conversions and allow them as equivalent.
The amount of grooming that happens is much more than just normalizing the version numbers -- there's about a thousand lines of fairly complicated Rust code in the packager that does a lot of heavy lifting around many corner cases inside the crate packaging code.
Fundamentally, this means it's very hard to verify that the Cargo.toml file is really equivalent from crates.io to the one you're using to do your build.
In the worst case scenario, it means that someone with control of crates.io could do something like substitute
xous-ipc = "0.9.10"
with
xous-ipc = {package = "xous-lpc", version = "0.9.10"}
.
This would effectively take the IPC communications crate and substitute it for some other crate called xous-lpc
, which would be controlled by the attacker, thus allowing an attacker to instrument anything into any inter-process message (as an example). To detect this, you'd have to notice that xous-lpc
was compiled in the compilation list as the build runs (presuming you're watching that list carefully), or you'd need to go into your .cargo/registry/src/...
directory and manually inspect the Cargo.toml file for such deficiencies. It would also result in a change in the Cargo.lock file, but it would not be a breaking change -- if you're not checking that the Cargo.lock file did not change after a build, then, you would still not notice.
In reality, the chance that someone compromise crates.io is quite small (I hope!), but on the other hand, a targeted compromise of just one crate in the Xous tree could quite possibly go unnoticed for a while, since we don't have an automated way to check for this (we would notice immediately if there is a compromise in the source code files, because those are verified every time xtask
runs; the compromise would have to happen on Cargo.toml, but because that file controls dependencies it can effectively bypass a wide range of source file checks).
There's now an extra check at the end of the CI step where the release images are made to confirm that the Cargo.lock file did not change somehow during the build.
It never does, but presumably if someone tampered with a Cargo.toml file (either accidentally or purposefully) it "should" leave a mark in the Cargo.lock file???