search

deltachat / deltachat-core-rust

Delta Chat Rust Core library, used by Android/iOS/desktop apps, bindings and bots 📧
https://delta.chat/en/contribute
Other
683 stars 87 forks source link

Add `cargo deny` to deltachat-core-rust #1521

Closed lupine closed 4 years ago

lupine commented 4 years ago

As discussed in the IRC channel.

I don't know a lot about the Rust ecosystem yet, so I'm throwing linters at my own code excessively, to teach myself what is good practice and what is not. One of those linters is https://github.com/EmbarkStudios/cargo-deny - this one is mostly about dependencies, and my main dependency is deltachat-core-rust, so rather than fixing the lints myself, I thought it might be a good idea to add a cargo deny step to deltachat-core-rust.

Here's the output on current master, with this deny.toml file (mostly boilerplate, but note I assume some licenses are fine to get that section of the output down):

```toml # This template contains all of the possible sections and their default values # Note that all fields that take a lint level have these possible values: # * deny - An error will be produced and the check will fail # * warn - A warning will be produced, but the check will not fail # * allow - No warning or error will be produced, though in some cases a note # will be # The values provided in this template are the default values that will be used # when any section or field is not specified in your own configuration # If 1 or more target triples (and optionally, target_features) are specified, # only the specified targets will be checked when running `cargo deny check`. # This means, if a particular package is only ever used as a target specific # dependency, such as, for example, the `nix` crate only being used via the # `target_family = "unix"` configuration, that only having windows targets in # this list would mean the nix crate, as well as any of its exclusive # dependencies not shared by any other crates, would be ignored, as the target # list here is effectively saying which targets you are building for. targets = [ # The triple can be any string, but only the target triples built in to # rustc (as of 1.40) can be checked against actual config expressions #{ triple = "x86_64-unknown-linux-musl" }, # You can also specify which target_features you promise are enabled for a # particular target. target_features are currently not validated against # the actual valid features supported by the target architecture. #{ triple = "wasm32-unknown-unknown", features = ["atomics"] }, ] # This section is considered when running `cargo deny check advisories` # More documentation for the advisories section can be found here: # https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html [advisories] # The path where the advisory database is cloned/fetched into db-path = "~/.cargo/advisory-db" # The url of the advisory database to use db-url = "https://github.com/rustsec/advisory-db" # The lint level for security vulnerabilities vulnerability = "deny" # The lint level for unmaintained crates unmaintained = "warn" # The lint level for crates that have been yanked from their source registry yanked = "warn" # The lint level for crates with security notices. Note that as of # 2019-12-17 there are no security notice advisories in # https://github.com/rustsec/advisory-db notice = "warn" # A list of advisory IDs to ignore. Note that ignored advisories will still # output a note when they are encountered. ignore = [ #"RUSTSEC-0000-0000", ] # Threshold for security vulnerabilities, any vulnerability with a CVSS score # lower than the range specified will be ignored. Note that ignored advisories # will still output a note when they are encountered. # * None - CVSS Score 0.0 # * Low - CVSS Score 0.1 - 3.9 # * Medium - CVSS Score 4.0 - 6.9 # * High - CVSS Score 7.0 - 8.9 # * Critical - CVSS Score 9.0 - 10.0 #severity-threshold = # This section is considered when running `cargo deny check licenses` # More documentation for the licenses section can be found here: # https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html [licenses] # The lint level for crates which do not have a detectable license unlicensed = "deny" # List of explictly allowed licenses # See https://spdx.org/licenses/ for list of possible licenses # [possible values: any SPDX 3.7 short identifier (+ optional exception)]. allow = [ "0BSD", "BSD-2-Clause", "CC0-1.0", "MPL-2.0", # accepted: license is considered copyleft ] # List of explictly disallowed licenses # See https://spdx.org/licenses/ for list of possible licenses # [possible values: any SPDX 3.7 short identifier (+ optional exception)]. deny = [ #"Nokia", ] # Lint level for licenses considered copyleft copyleft = "warn" # Blanket approval or denial for OSI-approved or FSF Free/Libre licenses # * both - The license will be approved if it is both OSI-approved *AND* FSF # * either - The license will be approved if it is either OSI-approved *OR* FSF # * osi-only - The license will be approved if is OSI-approved *AND NOT* FSF # * fsf-only - The license will be approved if is FSF *AND NOT* OSI-approved # * neither - This predicate is ignored and the default lint level is used allow-osi-fsf-free = "both" # Lint level used when no other predicates are matched # 1. License isn't in the allow or deny lists # 2. License isn't copyleft # 3. License isn't OSI/FSF, or allow-osi-fsf-free = "neither" default = "deny" # The confidence threshold for detecting a license from license text. # The higher the value, the more closely the license text must be to the # canonical license text of a valid SPDX license file. # [possible values: any between 0.0 and 1.0]. confidence-threshold = 0.8 # Allow 1 or more licenses on a per-crate basis, so that particular licenses # aren't accepted for every possible crate as with the normal allow list exceptions = [ # Each entry is the crate and version constraint, and its specific allow # list #{ allow = ["Zlib"], name = "adler32", version = "*" }, ] # Some crates don't have (easily) machine readable licensing information, # adding a clarification entry for it allows you to manually specify the # licensing information #[[licenses.clarify]] # The name of the crate the clarification applies to #name = "ring" # THe optional version constraint for the crate #version = "*" # The SPDX expression for the license requirements of the crate #expression = "MIT AND ISC AND OpenSSL" # One or more files in the crate's source used as the "source of truth" for # the license expression. If the contents match, the clarification will be used # when running the license check, otherwise the clarification will be ignored # and the crate will be checked normally, which may produce warnings or errors # depending on the rest of your configuration #license-files = [ # Each entry is a crate relative path, and the (opaque) hash of its contents #{ path = "LICENSE", hash = 0xbd0eed23 } #] [licenses.private] # If true, ignores workspace crates that aren't published, or are only # published to private registries ignore = false # One or more private registries that you might publish crates to, if a crate # is only published to private registries, and ignore is true, the crate will # not have its license(s) checked registries = [ #"https://sekretz.com/registry ] # This section is considered when running `cargo deny check bans`. # More documentation about the 'bans' section can be found here: # https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html [bans] # Lint level for when multiple versions of the same crate are detected multiple-versions = "warn" # The graph highlighting used when creating dotgraphs for crates # with multiple versions # * lowest-version - The path to the lowest versioned duplicate is highlighted # * simplest-path - The path to the version with the fewest edges is highlighted # * all - Both lowest-version and simplest-path are used highlight = "all" # List of crates that are allowed. Use with care! allow = [ #{ name = "ansi_term", version = "=0.11.0" }, ] # List of crates to deny deny = [ # Each entry the name of a crate and a version range. If version is # not specified, all versions will be matched. #{ name = "ansi_term", version = "=0.11.0" }, ] # Certain crates/versions that will be skipped when doing duplicate detection. skip = [ #{ name = "ansi_term", version = "=0.11.0" }, ] # Similarly to `skip` allows you to skip certain crates during duplicate # detection. Unlike skip, it also includes the entire tree of transitive # dependencies starting at the specified crate, up to a certain depth, which is # by default infinite skip-tree = [ #{ name = "ansi_term", version = "=0.11.0", depth = 20 }, ] # This section is considered when running `cargo deny check sources`. # More documentation about the 'sources' section can be found here: # https://embarkstudios.github.io/cargo-deny/checks/sources/cfg.html [sources] # Lint level for what to happen when a crate from a crate registry that is not # in the allow list is encountered unknown-registry = "warn" # Lint level for what to happen when a crate from a git repository that is not # in the allow list is encountered unknown-git = "warn" # List of URLs for allowed crate registries. Defaults to the crates.io index # if not specified. If it is specified but empty, no registries are allowed. allow-registry = ["https://github.com/rust-lang/crates.io-index"] # List of URLs for allowed Git repositories allow-git = [] ```

Running cargo deny check --hide-inclusion-graph gets me this output:

``` warning: found 2 duplicate entries for crate 'arrayvec' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:9:1 │ 9 │ ╭ arrayvec 0.4.12 registry+https://github.com/rust-lang/crates.io-index 10 │ │ arrayvec 0.5.1 registry+https://github.com/rust-lang/crates.io-index │ ╰────────────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'autocfg' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:20:1 │ 20 │ ╭ autocfg 0.1.7 registry+https://github.com/rust-lang/crates.io-index 21 │ │ autocfg 1.0.0 registry+https://github.com/rust-lang/crates.io-index │ ╰───────────────────────────────────────────────────────────────────^ lock entries warning: detected 'git' source not specifically allowed ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:86:14 │ 86 │ email 0.0.21 git+https://github.com/deltachat/rust-email#ace12ee6f8e054dd890589f588d0311604fc25f0 │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ source warning: found 2 duplicate entries for crate 'base64' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:24:1 │ 24 │ ╭ base64 0.10.1 registry+https://github.com/rust-lang/crates.io-index 25 │ │ base64 0.11.0 registry+https://github.com/rust-lang/crates.io-index │ ╰───────────────────────────────────────────────────────────────────^ lock entries warning: detected 'git' source not specifically allowed ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:87:21 │ 87 │ encoded-words 0.1.0 git+https://github.com/async-email/encoded-words#2631c258183620f6d976abffabbfc2dcc697d793 │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ source warning: detected 'git' source not specifically allowed ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:154:14 │ 154 │ lettre 0.9.2 git+https://github.com/deltachat/lettre#12e4f22d8d691fb5d1606e4c9034f5d4d9ba635b │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ source warning: detected 'git' source not specifically allowed ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:155:20 │ 155 │ lettre_email 0.9.2 git+https://github.com/deltachat/lettre#12e4f22d8d691fb5d1606e4c9034f5d4d9ba635b │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ source warning: found 2 duplicate entries for crate 'nom' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:181:1 │ 181 │ ╭ nom 4.2.3 registry+https://github.com/rust-lang/crates.io-index 182 │ │ nom 5.1.1 registry+https://github.com/rust-lang/crates.io-index │ ╰───────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'quote' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:218:1 │ 218 │ ╭ quote 0.3.15 registry+https://github.com/rust-lang/crates.io-index 219 │ │ quote 1.0.2 registry+https://github.com/rust-lang/crates.io-index │ ╰─────────────────────────────────────────────────────────────────^ lock entries warning: found 3 duplicate entries for crate 'rand' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:223:1 │ 223 │ ╭ rand 0.4.6 registry+https://github.com/rust-lang/crates.io-index 224 │ │ rand 0.6.5 registry+https://github.com/rust-lang/crates.io-index 225 │ │ rand 0.7.3 registry+https://github.com/rust-lang/crates.io-index │ ╰────────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'rand_chacha' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:226:1 │ 226 │ ╭ rand_chacha 0.1.1 registry+https://github.com/rust-lang/crates.io-index 227 │ │ rand_chacha 0.2.1 registry+https://github.com/rust-lang/crates.io-index │ ╰───────────────────────────────────────────────────────────────────────^ lock entries warning: found 3 duplicate entries for crate 'rand_core' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:228:1 │ 228 │ ╭ rand_core 0.3.1 registry+https://github.com/rust-lang/crates.io-index 229 │ │ rand_core 0.4.2 registry+https://github.com/rust-lang/crates.io-index 230 │ │ rand_core 0.5.1 registry+https://github.com/rust-lang/crates.io-index │ ╰─────────────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'rand_hc' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:231:1 │ 231 │ ╭ rand_hc 0.1.0 registry+https://github.com/rust-lang/crates.io-index 232 │ │ rand_hc 0.2.0 registry+https://github.com/rust-lang/crates.io-index │ ╰───────────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'syn' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:287:1 │ 287 │ ╭ syn 0.11.11 registry+https://github.com/rust-lang/crates.io-index 288 │ │ syn 1.0.16 registry+https://github.com/rust-lang/crates.io-index │ ╰────────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'unicode-xid' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:313:1 │ 313 │ ╭ unicode-xid 0.0.4 registry+https://github.com/rust-lang/crates.io-index 314 │ │ unicode-xid 0.2.0 registry+https://github.com/rust-lang/crates.io-index │ ╰───────────────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'version_check' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:320:1 │ 320 │ ╭ version_check 0.1.5 registry+https://github.com/rust-lang/crates.io-index 321 │ │ version_check 0.9.1 registry+https://github.com/rust-lang/crates.io-index │ ╰─────────────────────────────────────────────────────────────────────────^ lock entries warning: found 2 duplicate entries for crate 'winapi' ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:334:1 │ 334 │ ╭ winapi 0.2.8 registry+https://github.com/rust-lang/crates.io-index 335 │ │ winapi 0.3.8 registry+https://github.com/rust-lang/crates.io-index │ ╰──────────────────────────────────────────────────────────────────^ lock entries error[RUSTSEC-2020-0006]: Flaw in `realloc` allows reading unknown memory ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:39:1 │ 39 │ bumpalo 3.2.0 registry+https://github.com/rust-lang/crates.io-index │ ------------------------------------------------------------------- security vulnerability detected │ = When `realloc`ing, if we allocate new space, we need to copy the old allocation's bytes into the new space. There are `old_size` number of bytes in the old allocation, but we were accidentally copying `new_size` number of bytes, which could lead to copying bytes into the realloc'd space from past the chunk that we're bump allocating out of, from unknown memory. If an attacker can cause `realloc`s, and can read the `realoc`ed data back, this could allow them to read things from other regions of memory that they shouldn't be able to. For example, if some crypto keys happened to live in memory right after a chunk we were bump allocating out of, this could allow the attacker to read the crypto keys. Beyond just fixing the bug and adding a regression test, I've also taken two additional steps: 1. While we were already running the testsuite under `valgrind` in CI, because `valgrind` exits with the same code that the program did, if there are invalid reads/writes that happen not to trigger a segfault, the program can still exit OK and we will be none the wiser. I've enabled the `--error-exitcode=1` flag for `valgrind` in CI so that tests eagerly fail in these scenarios. 2. I've written a quickcheck test to exercise `realloc`. Without the bug fix in this patch, this quickcheck immediately triggers invalid reads when run under `valgrind`. We didn't previously have quickchecks that exercised `realloc` beacuse `realloc` isn't publicly exposed directly, and instead can only be indirectly called. This new quickcheck test exercises `realloc` via `bumpalo::collections::Vec::resize` and `bumpalo::collections::Vec::shrink_to_fit` calls. = URL: https://github.com/fitzgen/bumpalo/issues/69 error[RUSTSEC-2020-0015]: Crash causing Denial of Service attack ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:194:1 │ 194 │ openssl-src 111.6.1+1.1.1d registry+https://github.com/rust-lang/crates.io-index │ -------------------------------------------------------------------------------- security vulnerability detected │ = Server or client applications that call the SSL_check_chain() function during or after a TLS 1.3 handshake may crash due to a NULL pointer dereference as a result of incorrect handling of the "signature_algorithms_cert" TLS extension. The crash occurs if an invalid or unrecognised signature algorithm is received from the peer. This could be exploited by a malicious peer in a Denial of Service attack. = URL: https://www.openssl.org/news/secadv/20200421.txt error[RUSTSEC-2020-0014]: Various memory safety issues ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:249:1 │ 249 │ rusqlite 0.21.0 registry+https://github.com/rust-lang/crates.io-index │ --------------------------------------------------------------------- security vulnerability detected │ = Several memory safety issues have been uncovered in an audit of rusqlite. See https://github.com/rusqlite/rusqlite/releases/tag/0.23.0 for a complete list. = URL: https://github.com/rusqlite/rusqlite/releases/tag/0.23.0 warning[RUSTSEC-2020-0016]: `net2` crate has been deprecated; use `socket2` instead ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:178:1 │ 178 │ net2 0.2.33 registry+https://github.com/rust-lang/crates.io-index │ ----------------------------------------------------------------- unmaintained advisory detected │ = The [`net2`](https://crates.io/crates/net2) crate has been deprecated and users are encouraged to considered [`socket2`](https://crates.io/crates/socket2) instead. = URL: https://github.com/deprecrated/net2-rs/commit/3350e3819adf151709047e93f25583a5df681091 warning[RUSTSEC-2019-0031]: spin is no longer actively maintained ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:278:1 │ 278 │ spin 0.5.2 registry+https://github.com/rust-lang/crates.io-index │ ---------------------------------------------------------------- unmaintained advisory detected │ = The author of the `spin` crate does not have time or interest to maintain it. Consider the following alternatives (both of which support `no_std`): - [`conquer-once`](https://github.com/oliver-giersch/conquer-once) - [`lock_api`](https://crates.io/crates/lock_api) (a subproject of `parking_lot`) - [`spinning_top`](https://github.com/rust-osdev/spinning_top) spinlock crate built on `lock_api` = URL: https://github.com/mvdnes/spin-rs/commit/7516c80 warning[RUSTSEC-2018-0017]: `tempdir` crate has been deprecated; use `tempfile` instead ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:291:1 │ 291 │ tempdir 0.3.7 registry+https://github.com/rust-lang/crates.io-index │ ------------------------------------------------------------------- unmaintained advisory detected │ = The [`tempdir`](https://crates.io/crates/tempdir) crate has been deprecated and the functionality is merged into [`tempfile`](https://crates.io/crates/tempfile). = URL: https://github.com/rust-lang-deprecated/tempdir/pull/46 warning: detected yanked crate ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:39:1 │ 39 │ bumpalo 3.2.0 registry+https://github.com/rust-lang/crates.io-index │ ------------------------------------------------------------------- yanked version warning: detected yanked crate ┌─ /home/lupine/dev/github.com/deltachat/deltachat-core-rust/Cargo.lock:219:1 │ 219 │ quote 1.0.2 registry+https://github.com/rust-lang/crates.io-index │ ----------------------------------------------------------------- yanked version 2020-05-23 17:54:29 [ERROR] encountered 3 errors ```

I assume that rationalising the dependency graph so only one version of each dependency is used will reduce build times and binary size, which can only be a good thing. The other errors seem well worth fixing too.

Happy to create a PR for this if there's interest!

link2xt commented 4 years ago

Related issue wrt duplicate dependencies: #870

lupine commented 4 years ago

OK, I've created PRs for each of the current errors. I think once those three are merged, we could add a simple cargo deny configuration that will alert when new advisories are created, and that would be sufficient to close this issue if we have another for the duplicate dependencies.

lupine commented 4 years ago

If https://github.com/budziq/rust-skeptic/pull/119 is accepted, it will remove one of deltachat-core-rust's many dependencies once it filters down, since we already depend on tempfile v3.1.0 ^^.

It's not a big gain, but there are many like this, and the concrete goal of a green linter encourages contributions towards it.

dignifiedquire commented 4 years ago

@lupine a lot of deps have changed and updated with #1356 Could you run tests and PRs against that for the moment please? otherwise we will have to redo a lot of the work

lupine commented 4 years ago

OK, the async branch has been merged and it's down to a single RUSTSEC error:

``` error[RUSTSEC-2020-0014]: Various memory safety issues ┌─ /home/lupine/dev/code.ur.gs/lupine/telepathy-padfoot/Cargo.lock:226:1 │ 226 │ rusqlite 0.22.0 registry+https://github.com/rust-lang/crates.io-index │ --------------------------------------------------------------------- security vulnerability detected │ = Several memory safety issues have been uncovered in an audit of rusqlite. See https://github.com/rusqlite/rusqlite/releases/tag/0.23.0 for a complete list. = URL: https://github.com/rusqlite/rusqlite/releases/tag/0.23.0 = rusqlite v0.22.0 ├── deltachat v1.34.0 │ └── telepathy-padfoot v0.1.0 └── r2d2_sqlite v0.15.0 └── deltachat v1.34.0 (*) ```

It doesn't look like I can contribute effectively from a fork, so I can't create the PR for it, or for adding cargo deny more generally.

hpk42 commented 4 years ago

On Thu, May 28, 2020 at 01:49 -0700, Nick Thomas wrote:

226 │ rusqlite 0.22.0 registry+https://github.com/rust-lang/crates.io-index

this is probably going away when https://github.com/deltachat/deltachat-core-rust/pull/1534 is merged. No idea how "cargo deny" could best be integrated regularly --

It doesn't look like I can contribute from a fork, so I can't create the PR for it, or for adding cargo deny more generally.

@lupine, you are invited now to write-access, use carefully ;)

hpk42 commented 4 years ago

@lupine closing this, i think we have worked through most issues here apart from #1534 Maybe you can do a PR introducing Cargo-deny?