Skip to content

no-rsa-private-key.sh misses cli/tests, and its failure mode is passing #335

Description

@LKSNDRTMLKV

Two defects in scripts/no-rsa-private-key.sh, introduced in #322. The script enforces the claim that suppresses RUSTSEC-2023-0071 — that this workspace holds no RSA private key and performs no RSA private-key operation.

1. cli/tests is not scanned

The source scan covers crates/*/src crates/*/tests cli/src. cli/tests/ exists in this workspace and is absent from that list, so RSA private-key material in the CLI's integration tests would not void the suppression.

One-word fix: add cli/tests to the path list.

2. A path error turns the whole check green, silently

grep exits 2 on a path error, 1 on no match, 0 on a match. The script tests it with if grep …; then, which treats 2 the same as 1 — no match — and 2>/dev/null hides the reason:

if grep -rn --include="*.rs" -e 'RsaPrivateKey' …      crates/*/src crates/*/tests cli/src 2>/dev/null; then
    echo "ERROR: …" >&2
    STATUS=1
fi

Bash expands crates/*/tests only to directories that exist. Today that is 8 of them, so the check runs correctly — this is latent, not live. If it ever expands to nothing (integration tests moved under src, a crate layout change, the glob edited), bash passes the literal string crates/*/tests, grep errors on it, and the entire second check goes green while printing its matches to stdout.

Observed, not reasoned

Running the script against a fixture tree whose crates had no tests/ directory:

crates/foo/src/lib.rs:1:use rsa::RsaPrivateKey;
no-rsa-private-key: rsa is transitive and verification-only.
exit=0

It found the thing it exists to find, printed it, and reported success. Adding a tests/ directory to the fixture made the same input fail correctly with exit 1, which is what confines this to latent.

The first check has the same shape but a fixed path list (crates cli Cargo.toml), so it is not exposed the same way.

Fix

shopt -s nullglob so an empty glob contributes nothing rather than a literal, or test grep's status explicitly:

grep … ; rc=$?
case $rc in
  0) echo "ERROR: …" >&2; STATUS=1 ;;
  1) ;;
  *) echo "ERROR: the scan could not run (grep exit $rc)" >&2; STATUS=1 ;;
esac

The second is better: a gate that cannot run should fail, not pass.

Worth doing at the same time

Confirm the gate fails, as part of the gate. Every check here would have been caught by one negative case run in CI — a fixture with a planted RsaPrivateKey that the script must reject. The same applies to the other scripts/*.sh gates.

Found reviewing #322.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity-relevant issuetype/defectSomething published or encoded here is wrong or unbackable nowurgency/nextBlocks work already scheduled

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions