Skip to content

Commit

Permalink
Add diagnostics for unmatched build configuration (#556)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jake-Shadle committed Sep 2, 2023
1 parent 3a20d37 commit ad72615
Show file tree
Hide file tree
Showing 17 changed files with 702 additions and 177 deletions.
31 changes: 31 additions & 0 deletions docs/src/checks/advisories/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,34 @@ Similar to cargo's [net.git-fetch-with-cli](https://doc.rust-lang.org/cargo/refe

* `false` (default) - Fetches advisory databases via `gix`
* `true` - Fetches advisory databases using `git`. Git must be installed and in `PATH`.

### The `maximum-db-staleness` field (optional)

A duration in RFC3339 format that specifies the maximum amount of time that can pass before the database is considered stale and an error is emitted. This is only checked when advisory database fetching has been disabled via the `--offline` or `check --disable-fetch` flags, as otherwise the database is always cloned or fetched to be up to date with the remote git repository.

The default if not specified is the same value that `cargo-audit` uses, and `cargo-deny` has been using, which is `P90D`, or 90 days.

The RFC3339 duration format is...not well documented. The official grammar is as follows:

```txt
dur-second = 1*DIGIT "S"
dur-minute = 1*DIGIT "M" [dur-second]
dur-hour = 1*DIGIT "H" [dur-minute]
dur-time = "T" (dur-hour / dur-minute / dur-second)
dur-day = 1*DIGIT "D"
dur-week = 1*DIGIT "W"
dur-month = 1*DIGIT "M" [dur-day]
dur-year = 1*DIGIT "Y" [dur-month]
dur-date = (dur-day / dur-month / dur-year) [dur-time]
duration = "P" (dur-date / dur-time / dur-week)
```

However, as far as I can tell, there are no official spec compliance tests one can run for the duration formation, and several parsers I found written in other languages seemed to...not actually properly follow the grammar, so the implementation in cargo-deny _may_ be wrong according to the spec, but at least it will be consistently wrong.

Note that while the spec supports `,` as a decimal separator, for simplicity cargo-deny only supports `.` as a decimal separator.

One final note, there are 2 units available in the format that are not exact, namely, year 'Y' and month 'M'. It's not recommended to use either of them for that reason, but if you do they are calculated as follows.

* 1 year = 365 days
* 1 month = 30.43 days
129 changes: 127 additions & 2 deletions docs/src/checks/bans/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ When multiple versions of the same crate are encountered and `multiple-versions`

### Crate specifier

The `allow`, `deny`, `features`, `skip`, and `skip-tree` fields all use a crate identifier to specify what crate(s) they want to match against.
The `allow`, `deny`, `features`, `skip`, `skip-tree`, and `build.allow` fields all use a crate identifier to specify what crate(s) they want to match against.

```ini
{ name = "some-crate-name-here", version = "<= 0.7.0" }
Expand Down Expand Up @@ -158,6 +158,131 @@ Note that by default, the `depth` is infinite.

**NOTE:** `skip-tree` is a very big hammer at the moment, and should be used with care.

### The `allow-build-scripts` field (optional)
### The `build` field (optional)

The `build` field contains configuration for raising diagnostics for crates that execute at compile time, either because they have a [build script](https://doc.rust-lang.org/cargo/reference/build-scripts.html), or they are a [procedural macro](https://doc.rust-lang.org/reference/procedural-macros.html). The configuration is (currently) focused on diagnostics around specific file types, as configured via extension glob patterns, as well as executables, either native or in the form of [interpreted shebang scripts](https://en.wikipedia.org/wiki/Shebang_(Unix)).

While the intention of this configuration is to raise awareness of crates that have or use precompiled binaries or scripts, or otherwise contain file types that you want to be aware of, the compile time crate linting supplied by cargo-deny does **NOT** protect you from actively malicious code.

A quick run down of things that cargo-deny **WILL NOT DETECT**.

* The crate just straight up does bad things like uploading your SSH keys to a remote server using vanilla rust code
* The crate contains compressed, or otherwise obfuscated executable binaries
* The build script uses `include!()` for code that is benign in one version, then replaces it with something malicious without triggering a checksum mismatch on the build script contents itself.
* A build time dependency of a non-malicious crate does any of the above.
* Tons of other stuff I haven't thought of because I am not a security person

So all this is to say, `cargo-deny` (currently) is only really useful for analyzing when crates have native executables, and/or the crate maintainers have either forgotten or purposefully left helper scripts for their CI/release management/etc in the crate source that are not actually ever executed automatically.

#### The `allow-build-scripts` field (optional)

Specifies all the crates that are allowed to have a build script. If this option is omitted, all crates are allowed to have a build script, and if this option is set to an empty list, no crate is allowed to have a build script.

#### The `executables` field (optional)

This controls how native executables are handled. Note this check is done by actually reading the file headers from disk so that this check works on Windows as well, ie the executable bit is irrelevant.

* `deny` (default) - Emits an error when native executables are detected.
* `warn` - Prints a warning when native executables are detected, but does not fail the check.
* `allow` - Prints a note when native executables are detected, but does not fail the check.

This check currently only handles the major executable formats.

* [ELF](https://en.wikipedia.org/wiki/Executable_and_Linkable_Format)
* [PE](https://en.wikipedia.org/wiki/Portable_Executable)
* [Mach-O](https://en.wikipedia.org/wiki/Mach-O)

#### The `interpreted` field (optional)

This controls how interpreted scripts are handled. Note this check is done by actually reading the file header from disk so that this check works on Windows as well, ie the executable bit is irrelevant.

* `deny` - Emits an error when interpreted scripts are detected.
* `warn` - Prints a warning when interpreted scripts are detected, but does not fail the check.
* `allow` (default) - Prints a note when interpreted scripts are detected, but does not fail the check.

#### The `script-extensions` field (optional)

If supplied scans crates that execute at compile time for any files with the specified extension(s), emitting an error for every one that matches.

#### The `enable-builtin-globs` field (optional)

If `true`, enables the builtin glob patterns for common languages that tend to by installed on most developer machines, such as python.

```ini
{{#include ../../../../src/bans/builtin_globs.toml}}
```

#### The `include-dependencies` field (optional)

By default, only the crate that executes at compile time is scanned, but if set to `true`, this field will check this crate as well as all of its dependencies. This option is disabled by default, as this will tend to only find CI scripts that people leave in their published crates.

#### The `include-workspace` field (optional)

If `true`, workspace crates will also be scanned. This defaults to false as you presumably have some degree of trust for your own code.

#### The `include-archives` field (optional)

If `true`, archive files (eg. Windows .lib, Unix .a, C++ .o object files etc) are also counted as native code. This defaults to false, as these tend to need to be linked before they can be executed.

#### The `bypass` field (optional)

While all the previous configuration is about configuration the global checks that run on compile time crates, the `allow` field is how one can suppress those lints on a crate-by-crate basis.

Each entry uses the same [Crate specifier](#crate-specifier) as other parts of cargo-deny's configuration.

```ini
[build.bypass]
name = "crate-name"
```

##### The `build-script` and `required-features` field (optional)

If set to a valid, 64-character hexadecimal [SHA-256](https://en.wikipedia.org/wiki/SHA-2), the `build-script` field will cause the rest of the scanning to be bypassed _if_ the crate's build script's checksum matches the user specified checksum **AND** none of the features specified in the `required-features` field are enabled. If the checksum does not match, the calculated checksum will be emitted as a warning, and the crate will be scanned as if a checksum was not supplied.

**NOTE:** These options only applies to crate with build scripts, not proc macros, as proc macros do not have a single entry point that can be easily checksummed.

```ini
[[build.bypass]]
name = "crate-name"
build-script = "5392f0e58ad06e089462d93304dfe82337acbbefb87a0749a7dc2ed32af04af7"
```

##### The `allow-globs` field (optional)

Bypasses scanning of files that match one or more of the glob patterns specified. Note that unlike the [`script-extensions`](#the-script-extensions-field-optional) field that applies to all crates, these globs can match anything, not just extensions.

```ini
[build]
script-extensions = ["cs"]

[[build.bypass]]
name = "crate-name"
allow-globs = [
"scripts/*.cs",
]
```

##### The `bypass.allow` field (optional)

Bypasses scanning a single file.

```ini
[build]
executables = "deny"

[[build.bypass]]
name = "crate-name"
allow = [
{ path = "bin/x86_64-linux", checksum = "5392f0e58ad06e089462d93304dfe82337acbbefb87a0749a7dc2ed32af04af7" }
]
```

###### The `path` field

The path, relative to the crate root, of the file to bypass scanning.

###### The `checksum` field (optional)

The 64-character hexadecimal [SHA-256](https://en.wikipedia.org/wiki/SHA-2) checksum of the file. If the checksum does not match, an error is emitted.

[](
48 changes: 48 additions & 0 deletions docs/src/checks/bans/diags.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,51 @@ A feature in either [`bans.features.deny`](cfg.md#the-features-deny-field-option
### `default-feature-enabled`

The `default` feature was enabled on a crate, and the [`bans.external-default-features`](cfg.md#the-external-default-features-field-optional) or [`bans.workspace-default-features`](cfg.md#the-workspace-default-features-field-optional) was configured.

### `path-bypassed`

A path specified by [`bans.build.bypass.allow.path`](cfg.md#the-path-field) was bypassed, optionally ensuring its contents matched a SHA-256 checksum.

### `path-bypassed-by-glob`

A path was bypassed due to matching one or more [glob patterns](cfg.md#the-allow-globs-field-optional).

### `checksum-match`

The SHA-256 checksum calculated for the contents of a file matched the checksum in the configuration.

### `checksum-mismatch`

The SHA-256 checksum calculated for the contents of a file did not match the checksum in the configuration.

### `denied-by-extension`

The file extension matched either a [user specified](cfg.md#the-script-extensions-field-optional) or [builtin](cfg.md#the-enable-builtin-globs-field-optional) extension.

### `detected-executable`

A [native executable](cfg.md#the-executables-field-optional) was detected.

### `detected-executable-script`

An [interpreted script](cfg.md#the-interpreted-field-optional) was detected.

### `unable-to-check-path`

An I/O error occured when opening or reading a file from disk.

### `features-enabled`

One or more [`required-features`](cfg.md#the-build-script-and-required-features-field-optional) were enabled, causing the [`build-script`](cfg.md#the-build-script-and-required-features-field-optional) bypass to be ignored.

### `unmatched-bypass`

A [crate bypass](cfg.md#the-bypass-field-optional) did not match any crate in the graph.

### `unmatched-path-bypass`

A [path bypass](cfg.md#the-bypassallow-field-optional) did not match a file in the crate.

### `unmatched-glob`

A [glob bypass](cfg.md#the-allow-globs-field-optional) did not match any files in the crate.
4 changes: 3 additions & 1 deletion docs/src/cli/check.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Disable fetching of the advisory database

When running the `advisories` check, the configured advisory database will be fetched and opened. If this flag is passed, the database won't be fetched, but an error will occur if it doesn't already exist locally.

This option is also set if the `--offline` flag is used in the global options.

### `-D, --deny <DENY>`

Set lint denied
Expand Down Expand Up @@ -77,7 +79,7 @@ Set lint warnings

As of [0.14.1](https://github.com/EmbarkStudios/cargo-deny/releases/tag/0.14.1), the exit code for the check command is a bitset of the checks that were executed and had 1 or more errors.

A script or program can use the following values to determine exactly which check failed.
A script or program can use the following values to determine exactly which check(s) failed.

* `advisories` - `0x1`
* `bans` - `0x2`
Expand Down
24 changes: 15 additions & 9 deletions docs/src/cli/common.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,37 @@

The subcommands share some common options that can be used before the subcommand.

#### `--manifest-path`
## Options

### `--manifest-path`

The path to a `Cargo.toml` file which is used as the context for operations.

#### `--all-features` (single crate or workspace)
### `--all-features` (single crate or workspace)

Enables all features when determining which crates to consider. Works for both single crates and workspaces.

#### `--no-default-features` (single crate only)
### `--no-default-features` (single crate only)

Disables the `default` feature for a crate when determing which crates to consider.
Disables the `default` feature for a crate when determining which crates to consider.

#### `--features` (single crate only)
### `--features` (single crate only)

Space-separated list of features to enable when determining which crates to consider.

#### `--workspace`
### `--workspace`

Forces all workspace crates to be used as roots in the crate graph that we operate on, unless they are excluded by other means. By default, if you specify a [virtual manifest](https://doc.rust-lang.org/cargo/reference/manifest.html#virtual-manifest), all crates in the workspace will be used as roots. However, if you specify a normal package manifest somewhere inside a workspace, only that crate will be used as a graph root, and only other workspaces crates it depends on will be included in the graph. If you want to specify a sub-crate in a workspace, but still include all other crates in the workspace, you can use this flag.

#### `--exclude`
### `--exclude`

Exclude the specified package(s) from the crate graph. Unlike other cargo subcommands, it doesn't have to be used in conjunction with the `--workspace` flag. This flag may be specified multiple times.

This uses a similar (though slightly more strict) [Package ID specification](https://doc.rust-lang.org/cargo/commands/cargo-pkgid.html) to other cargo subcommands.

Packages can also be excluded in your [configuration](../checks/cfg.md#the-exclude-field-optional) files, specifying this on the command line will append the package ID to the list that may exist in your configuration.

#### `-L, --log-level`
### `-L, --log-level`

The log level for messages, only log messages at or above the level will be emitted.

Expand Down Expand Up @@ -62,6 +64,10 @@ Possible values:
* `always` - Coloring is always applied
* `never` - No coloring is applied for any output

#### `-t, --target`
### `-t, --target`

One or more platforms to filter crates with. If a dependency is target specific, it will be ignored if it does not match at least 1 of the specified targets. This overrides the top-level [`targets = []`](../checks/cfg.md) configuration value.

### `--offline`

Disables network I/O.
16 changes: 8 additions & 8 deletions src/advisories/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ fn parse_rfc3339_duration(value: &str) -> anyhow::Result<time::Duration> {
#[derive(Copy, Clone, PartialEq, PartialOrd)]
enum Unit {
Empty,
Day,
Month,
Year,
Month,
Day,
Time,
Hour,
Minute,
Expand Down Expand Up @@ -406,7 +406,7 @@ mod test {
"P", "PT", // Empty duration, must specify at least _one_ unit
"P1H3", "P2TH3", // Number without unit specified
"PT1HM", // Unit without number specified
"PT1M3H", "P3Y1M", "P2W1Y", "PT2W1H", // Units in an invalid order
"PT1M3H", "P1M3Y", "P2W1Y", "PT2W1H", // Units in an invalid order
"P5H", "P5S", // Time units must be preceded by T
// We don't accept ',' as a decimal separator even though it is allowed in the spec
"PT1,5S",
Expand Down Expand Up @@ -438,12 +438,12 @@ mod test {
("PT2M", 2. * 60.),
("PT8S", 8.),
("PT8.5S", 8.5),
("P3M1Y", 3. * MONTH + 365. * DAY),
("P5D1Y", 5. * DAY + 365. * DAY),
("P3D4M1Y", 3. * DAY + 4. * MONTH + 365. * DAY),
("P1Y3M", 365. * DAY + 3. * MONTH),
("P1Y5D", 365. * DAY + 5. * DAY),
("P1Y4M3D", 365. * DAY + 4. * MONTH + 3. * DAY),
(
"P2D3M1YT3H2M1S",
2. * DAY + 3. * MONTH + 365. * DAY + 3. * 60. * 60. + 2. * 60. + 1.,
"P1Y3M2DT3H2M1S",
365. * DAY + 3. * MONTH + 2. * DAY + 3. * 60. * 60. + 2. * 60. + 1.,
),
("P2DT4H", 2. * DAY + 4. * 60. * 60.),
("P2MT0.5M", 2. * MONTH + 0.5 * 60.),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Err('H' must be preceded with 'T')
Err(unit not specified for value '2')
Err(value not specified for 'M')
Err(unit 'H' cannot follow 'M')
Err(unit 'M' cannot follow 'Y')
Err(unit 'Y' cannot follow 'M')
Err(unit 'Y' cannot follow 'W')
Err(unit 'H' cannot follow 'W')
Err('H' must be preceded with 'T')
Expand Down
Loading

0 comments on commit ad72615

Please sign in to comment.