From 47c10df5b564a7e89b5ca74d461c2aa587d35c76 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 23:54:18 +0000 Subject: [PATCH 1/3] Phase 1: supply-chain, regression tests, and release tooling Implements Phase 1 of the next-level roadmap (distribution + hardening), all within AGENTS.md "always safe" boundaries -- no crypto or vault-format changes. Supply chain: - Add deny.toml (cargo-deny): permissive license allowlist, crates.io-only sources, and a hard single-version rule for the core crypto crates (ml-kem, x25519-dalek, aes-gcm, argon2, sha2, hmac, hkdf). Validated offline with cargo-deny 0.16.4 (bans/licenses/sources ok). - Add a cargo-deny CI job alongside the existing rustsec audit-check. Regression tests (close the deferred SECURITY-AUDIT.md test gaps): - header_tamper_v7: every v7 key-commitment-covered header field is rejected at unlock when tampered. - downgrade_rejected: a v7 body claiming an older version is rejected. - argon2_dos: hostile KDF params (memory/time/parallelism/algorithm) are rejected by validate_kdf_params before Argon2 runs (asserted fast). - export_env_quoting: shell-metachar secret values round-trip byte-exactly through `eval` in /bin/sh and never execute (injection canary). - stdin_overflow: oversized piped `set` input is refused and not stored; at-cap input is accepted. Distribution: - Cargo.toml: add repository/readme/keywords/categories + exclude paper media, historical source snapshot, and audit/CI/agent files from the published crate (verified via `cargo package --list`). - release.yml: tag-triggered portable cross-platform release binaries with SHA256 checksums, neutralizing the repo's target-cpu=native pin. --- .github/workflows/ci.yml | 11 +++ .github/workflows/release.yml | 106 ++++++++++++++++++++++++++++ Cargo.toml | 14 ++++ deny.toml | 83 ++++++++++++++++++++++ tests/argon2_dos.rs | 88 +++++++++++++++++++++++ tests/downgrade_rejected.rs | 55 +++++++++++++++ tests/export_env_quoting.rs | 108 ++++++++++++++++++++++++++++ tests/header_tamper_v7.rs | 129 ++++++++++++++++++++++++++++++++++ tests/stdin_overflow.rs | 93 ++++++++++++++++++++++++ 9 files changed, 687 insertions(+) create mode 100644 .github/workflows/release.yml create mode 100644 deny.toml create mode 100644 tests/argon2_dos.rs create mode 100644 tests/downgrade_rejected.rs create mode 100644 tests/export_env_quoting.rs create mode 100644 tests/header_tamper_v7.rs create mode 100644 tests/stdin_overflow.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b111a56..a50ebb4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,3 +90,14 @@ jobs: - uses: rustsec/audit-check@v2 with: token: ${{ secrets.GITHUB_TOKEN }} + + deny: + name: Supply chain (cargo-deny) + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + - uses: EmbarkStudios/cargo-deny-action@v2 + with: + command: check advisories bans licenses sources diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..416119f --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,106 @@ +name: Release + +# Build portable release binaries and publish them to a GitHub Release +# whenever a v* tag is pushed. Each artifact is accompanied by a SHA256 +# checksum so downstream installers (Homebrew formula, install.sh) can verify +# integrity. +on: + push: + tags: + - "v*" + +permissions: + contents: write + +env: + CARGO_TERM_COLOR: always + # The repo's .cargo/config.toml pins `target-cpu=native`, which would bake + # the CI runner's CPU features into the binary and break on older user + # machines. Force a portable baseline for distributable artifacts. + RUSTFLAGS: "-C target-cpu=generic" + +jobs: + build: + name: Build ${{ matrix.target }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + include: + - os: ubuntu-latest + target: x86_64-unknown-linux-gnu + - os: ubuntu-latest + target: x86_64-unknown-linux-musl + - os: macos-latest + target: x86_64-apple-darwin + - os: macos-latest + target: aarch64-apple-darwin + - os: windows-latest + target: x86_64-pc-windows-msvc + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + targets: ${{ matrix.target }} + - uses: Swatinem/rust-cache@v2 + with: + key: ${{ matrix.target }} + + - name: Install musl tools + if: matrix.target == 'x86_64-unknown-linux-musl' + run: sudo apt-get update && sudo apt-get install -y musl-tools + + - name: Build release binary + run: cargo build --release --locked --target ${{ matrix.target }} + + - name: Package (unix) + if: matrix.os != 'windows-latest' + shell: bash + run: | + set -euo pipefail + bin="target/${{ matrix.target }}/release/dota" + name="dota-${{ github.ref_name }}-${{ matrix.target }}" + mkdir -p "dist/$name" + cp "$bin" "dist/$name/" + cp README.md LICENSE "dist/$name/" + tar -C dist -czf "dist/$name.tar.gz" "$name" + ( cd dist && shasum -a 256 "$name.tar.gz" > "$name.tar.gz.sha256" ) + + - name: Package (windows) + if: matrix.os == 'windows-latest' + shell: bash + run: | + set -euo pipefail + bin="target/${{ matrix.target }}/release/dota.exe" + name="dota-${{ github.ref_name }}-${{ matrix.target }}" + mkdir -p "dist/$name" + cp "$bin" "dist/$name/" + cp README.md LICENSE "dist/$name/" + ( cd dist && 7z a "$name.zip" "$name" >/dev/null ) + ( cd dist && certutil -hashfile "$name.zip" SHA256 > "$name.zip.sha256" ) + + - name: Upload artifacts + uses: actions/upload-artifact@v4 + with: + name: dota-${{ matrix.target }} + path: | + dist/*.tar.gz + dist/*.zip + dist/*.sha256 + retention-days: 7 + + release: + name: Publish GitHub Release + needs: build + runs-on: ubuntu-latest + steps: + - name: Download all artifacts + uses: actions/download-artifact@v4 + with: + path: artifacts + - name: Publish + uses: softprops/action-gh-release@v2 + with: + files: artifacts/**/* + generate_release_notes: true + fail_on_unmatched_files: true diff --git a/Cargo.toml b/Cargo.toml index 9a3d41b..22b16f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,20 @@ edition = "2024" authors = ["zack"] description = "Defense of the Artifacts - Post-quantum secure secrets manager with v7 TC-HKEM (ML-KEM-768 + X25519) vaults" license = "MIT" +repository = "https://github.com/johnzfitch/dota" +readme = "README.md" +keywords = ["cryptography", "post-quantum", "secrets", "ml-kem", "vault"] +categories = ["command-line-utilities", "cryptography", "authentication"] +# Keep the published crate lean: the paper media, the historical v7 source +# snapshot/patch, and the audit doc are not needed to build or use the binary. +exclude = [ + "dotav7-paper/", + "dotav7/", + "SECURITY-AUDIT.md", + ".github/", + ".claude/", + ".cargo/", +] [features] default = ["legacy-migration"] diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000..386aebd --- /dev/null +++ b/deny.toml @@ -0,0 +1,83 @@ +# cargo-deny configuration for dota. +# +# Supplements the existing `rustsec/audit-check` CI job with three guarantees +# that matter for a security tool: +# 1. Licenses are limited to a vetted permissive allowlist. +# 2. The core cryptographic crates may never appear at two different +# versions in the dependency graph (a duplicate `ml-kem`/`aes-gcm`/etc. +# would mean secrets could be processed by an unaudited copy). +# 3. Crates may only come from crates.io -- no git or unknown registries. +# +# Run locally with: cargo deny check + +[graph] +all-features = true + +[advisories] +# RUSTSEC advisory database. Fail the build on any unmaintained or vulnerable +# crate; refuse yanked crates. Mirrors/extends the audit-check job. +yanked = "deny" +version = 2 + +[licenses] +# Vetted permissive licenses. Every expression in the current tree resolves +# to one (or a permitted AND-combination) of these. +allow = [ + "MIT", + "MIT-0", + "Apache-2.0", + "Apache-2.0 WITH LLVM-exception", + "BSD-1-Clause", + "BSD-2-Clause", + "BSD-3-Clause", + "BSL-1.0", + "CC0-1.0", + "ISC", + "Zlib", + "Unlicense", + "Unicode-3.0", + "MPL-2.0", +] +confidence-threshold = 0.9 +version = 2 + +[bans] +# Transitive duplicate versions are common and not inherently dangerous, so +# warn globally -- but hard-deny duplicates of the crypto crates below, where +# a second version would silently widen the trusted-code surface. +multiple-versions = "warn" +wildcards = "deny" + +# Core cryptographic dependencies: exactly one version each, always. +[[bans.deny]] +name = "ml-kem" +deny-multiple-versions = true + +[[bans.deny]] +name = "x25519-dalek" +deny-multiple-versions = true + +[[bans.deny]] +name = "aes-gcm" +deny-multiple-versions = true + +[[bans.deny]] +name = "argon2" +deny-multiple-versions = true + +[[bans.deny]] +name = "sha2" +deny-multiple-versions = true + +[[bans.deny]] +name = "hmac" +deny-multiple-versions = true + +[[bans.deny]] +name = "hkdf" +deny-multiple-versions = true + +[sources] +unknown-registry = "deny" +unknown-git = "deny" +allow-registry = ["https://github.com/rust-lang/crates.io-index"] diff --git a/tests/argon2_dos.rs b/tests/argon2_dos.rs new file mode 100644 index 0000000..35b78f5 --- /dev/null +++ b/tests/argon2_dos.rs @@ -0,0 +1,88 @@ +//! Regression: a hostile vault that asks for an enormous Argon2 memory cost +//! must be rejected by `validate_kdf_params` BEFORE the KDF runs, so a +//! planted vault cannot turn an unlock attempt into a multi-gigabyte +//! allocation / DoS. `validate_v7_vault` runs `validate_kdf_params` first in +//! the v7 unlock path, so the failure must be the validation bail -- which +//! structurally proves Argon2 never executed. + +use dota::vault::ops::{create_vault, unlock_vault}; +use serde_json::Value; +use std::fs; +use std::time::Instant; +use tempfile::tempdir; + +const PASS: &str = "correct horse battery staple"; + +/// Tamper one KDF field on a real v7 vault and assert unlock fails with the +/// expected validation message, fast (no Argon2 execution). +fn assert_rejected_fast(tag: &str, field: &str, value: Value, expect_substr: &str) { + let dir = tempdir().unwrap(); + let base = dir.path().join("base.json"); + create_vault(PASS, base.to_str().unwrap()).unwrap(); + + let mut doc: Value = serde_json::from_str(&fs::read_to_string(&base).unwrap()).unwrap(); + doc["kdf"][field] = value; + + let hostile = dir.path().join(format!("hostile_{tag}.json")); + fs::write(&hostile, serde_json::to_string_pretty(&doc).unwrap()).unwrap(); + + let start = Instant::now(); + let err = unlock_vault(PASS, hostile.to_str().unwrap()) + .err() + .unwrap_or_else(|| panic!("hostile KDF '{tag}' must be rejected")); + let elapsed = start.elapsed(); + + let msg = format!("{err:#}"); + assert!( + msg.contains(expect_substr), + "expected '{expect_substr}' for '{tag}', got: {msg}" + ); + // The legitimate Argon2 (64 MiB, 3 passes) takes ~1s; a pre-KDF + // validation bail returns in milliseconds. A generous ceiling keeps the + // assertion meaningful without being flaky on slow CI. + assert!( + elapsed.as_secs() < 1, + "rejection for '{tag}' took {elapsed:?}; validation should bail before Argon2 runs" + ); +} + +#[test] +fn excessive_memory_cost_is_rejected_before_argon2() { + // 1 GiB requested -- far above the 256 MiB ceiling. + assert_rejected_fast( + "memory", + "memory_cost", + Value::from(1_000_000u32), + "Invalid Argon2 memory cost", + ); +} + +#[test] +fn excessive_time_cost_is_rejected_before_argon2() { + assert_rejected_fast( + "time", + "time_cost", + Value::from(1_000u32), + "Invalid Argon2 time cost", + ); +} + +#[test] +fn excessive_parallelism_is_rejected_before_argon2() { + assert_rejected_fast( + "parallelism", + "parallelism", + Value::from(1_000u32), + "Invalid Argon2 parallelism", + ); +} + +#[test] +fn unknown_kdf_algorithm_is_rejected_before_argon2() { + assert_rejected_fast( + "algorithm", + "algorithm", + Value::from("scrypt"), + "Unsupported KDF algorithm", + ); +} diff --git a/tests/downgrade_rejected.rs b/tests/downgrade_rejected.rs new file mode 100644 index 0000000..e3d6691 --- /dev/null +++ b/tests/downgrade_rejected.rs @@ -0,0 +1,55 @@ +//! Regression: anti-rollback. A genuine v7 vault whose `version` field is +//! rewritten to claim an older format must NOT be silently accepted on the +//! legacy migration path. The v7 commitment covers `version`, so the +//! downgrade either fails the legacy parse/migration or trips the commitment +//! -- it must never unlock as if it were the claimed older version. + +use dota::vault::ops::{create_vault, unlock_vault}; +use serde_json::Value; +use std::fs; +use tempfile::tempdir; + +const PASS: &str = "correct horse battery staple"; + +fn assert_downgrade_rejected(claimed_version: u32) { + let dir = tempdir().unwrap(); + let base = dir.path().join("base.json"); + create_vault(PASS, base.to_str().unwrap()).unwrap(); + + let mut doc: Value = serde_json::from_str(&fs::read_to_string(&base).unwrap()).unwrap(); + // Keep the entire v7 body intact; only lie about the version number. + doc["version"] = Value::from(claimed_version); + + let forged = dir.path().join("forged.json"); + fs::write(&forged, serde_json::to_string_pretty(&doc).unwrap()).unwrap(); + + let err = unlock_vault(PASS, forged.to_str().unwrap()) + .err() + .unwrap_or_else(|| panic!("v7 body claiming version {claimed_version} must be rejected")); + let msg = format!("{err:#}").to_lowercase(); + assert!( + msg.contains("commitment") + || msg.contains("tamper") + || msg.contains("mismatch") + || msg.contains("decrypt") + || msg.contains("parse") + || msg.contains("invalid") + || msg.contains("unsupported"), + "downgrade to v{claimed_version} rejected, but message is unexpected: {msg}" + ); +} + +#[test] +fn v7_body_claiming_v6_is_rejected() { + assert_downgrade_rejected(6); +} + +#[test] +fn v7_body_claiming_v5_is_rejected() { + assert_downgrade_rejected(5); +} + +#[test] +fn v7_body_claiming_v1_is_rejected() { + assert_downgrade_rejected(1); +} diff --git a/tests/export_env_quoting.rs b/tests/export_env_quoting.rs new file mode 100644 index 0000000..fc69243 --- /dev/null +++ b/tests/export_env_quoting.rs @@ -0,0 +1,108 @@ +//! Regression: `dota export-env` output must be safe to `eval` in a POSIX +//! shell and must round-trip byte-for-byte. A secret value containing shell +//! metacharacters (quotes, `;`, `$( )`, backticks, newlines) must (a) never +//! execute as code when eval'd, and (b) reconstruct exactly the stored +//! value. Single-quote wrapping with `'\''` escaping is the mechanism under +//! test (`src/cli/export.rs::shell_escape`). +//! +//! Unix-only: the round-trip is validated by actually running `/bin/sh`. + +#![cfg(unix)] + +use dota::vault::ops::{create_vault, set_secret, unlock_vault}; +use std::process::Command; +use tempfile::tempdir; + +const PASS: &str = "correct horse battery staple"; +const DOTA: &str = env!("CARGO_BIN_EXE_dota"); + +/// Values chosen to break naive quoting. The injection-bearing values embed +/// `canary` (a path under the test's tempdir) so that, if quoting failed and +/// the shell executed the payload, the canary file would appear -- letting us +/// detect the breach without relying on a shared global path. NUL is +/// intentionally excluded: a POSIX shell variable cannot hold a NUL byte. +fn hostile_values(canary: &str) -> Vec<(&'static str, String)> { + vec![ + ("SINGLE_QUOTE", "a'b'c".to_string()), + ("DOUBLE_QUOTE", "a\"b\"c".to_string()), + ("SEMICOLON", format!("x; touch {canary}")), + ("CMD_SUBST", format!("$(touch {canary})")), + ("BACKTICK", format!("`touch {canary}`")), + ("NEWLINE", "line1\nline2\nline3".to_string()), + ("DOLLAR_VARS", "p@$$w0rd $HOME ${PATH}".to_string()), + ("MIXED", format!("a b'c$(touch {canary})e`f`;g\"h")), + ("PLAIN", "ordinary-value-123".to_string()), + ] +} + +fn build_vault(path: &str, values: &[(&'static str, String)]) { + create_vault(PASS, path).unwrap(); + let mut unlocked = unlock_vault(PASS, path).unwrap(); + for (name, value) in values { + set_secret(&mut unlocked, name, value).unwrap(); + } +} + +fn export_line(vault: &str, name: &str) -> String { + let out = Command::new(DOTA) + .args(["export-env", name, "--vault", vault]) + .env("DOTA_PASSPHRASE", PASS) + .output() + .expect("run dota export-env"); + assert!( + out.status.success(), + "export-env failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8(out.stdout).expect("export-env stdout is utf-8") +} + +/// Eval the export line in /bin/sh and print the resulting variable value +/// with no added bytes, so we can compare exactly. +fn eval_and_read(export: &str, name: &str) -> Vec { + let script = format!("eval \"$1\"; printf %s \"${{{name}}}\""); + let out = Command::new("/bin/sh") + .args(["-c", &script, "_", export]) + .output() + .expect("run /bin/sh"); + assert!( + out.status.success(), + "sh eval failed for {name}: {}", + String::from_utf8_lossy(&out.stderr) + ); + out.stdout +} + +#[test] +fn export_env_values_round_trip_through_sh_eval() { + let dir = tempdir().unwrap(); + let canary = dir.path().join("injection-canary"); + let canary = canary.to_str().unwrap(); + let values = hostile_values(canary); + + let vault = dir.path().join("vault.json"); + let vault = vault.to_str().unwrap(); + build_vault(vault, &values); + + for (name, expected) in &values { + let line = export_line(vault, name); + assert!( + line.starts_with(&format!("export {name}=")), + "unexpected export line for {name}: {line:?}" + ); + let got = eval_and_read(&line, name); + assert_eq!( + &got, + expected.as_bytes(), + "round-trip mismatch for {name}: got {:?}, expected {:?}", + String::from_utf8_lossy(&got), + expected + ); + } + + // The injection canary must never have been created by any eval above. + assert!( + !std::path::Path::new(canary).exists(), + "shell injection executed: canary {canary} was created" + ); +} diff --git a/tests/header_tamper_v7.rs b/tests/header_tamper_v7.rs new file mode 100644 index 0000000..f6e7b43 --- /dev/null +++ b/tests/header_tamper_v7.rs @@ -0,0 +1,129 @@ +//! Regression: every field covered by the v7 HMAC-SHA256 key commitment +//! must be rejected at unlock if it is tampered with after the vault is +//! written. The commitment is computed over the canonical header (version, +//! min_version, KDF params, both algorithm ids, both public keys, suite); +//! `verify_v7_key_commitment` runs before any private-key decryption, so a +//! flipped header field must surface a tamper/mismatch error -- never a +//! silently-accepted unlock. +//! +//! Structural fields (suite, algorithm labels) are caught earlier by +//! `validate_v7_vault`; integrity-only fields (KDF cost, public-key bytes, +//! min_version) are caught by the commitment. Either way the unlock fails, +//! which is the property under test. + +use dota::vault::ops::{create_vault, unlock_vault}; +use serde_json::Value; +use std::fs; +use tempfile::tempdir; + +const PASS: &str = "correct horse battery staple"; + +/// Build a fresh v7 vault, parse its JSON, hand it to `mutate`, write the +/// mutated JSON to a new path, and return that path's unlock result. +fn unlock_with_tamper(dir: &std::path::Path, tag: &str, mutate: impl FnOnce(&mut Value)) { + let base = dir.join("base.json"); + create_vault(PASS, base.to_str().unwrap()).unwrap(); + + let mut doc: Value = serde_json::from_str(&fs::read_to_string(&base).unwrap()).unwrap(); + mutate(&mut doc); + + let tampered = dir.join(format!("tampered_{tag}.json")); + fs::write(&tampered, serde_json::to_string_pretty(&doc).unwrap()).unwrap(); + + let err = unlock_vault(PASS, tampered.to_str().unwrap()) + .err() + .unwrap_or_else(|| panic!("tampering '{tag}' must be rejected at unlock")); + let msg = format!("{err:#}").to_lowercase(); + assert!( + msg.contains("commitment") + || msg.contains("tamper") + || msg.contains("mismatch") + || msg.contains("unsupported") + || msg.contains("invalid"), + "tamper '{tag}' rejected, but the message is unexpected: {msg}" + ); +} + +/// Flip the first base64 character of a field to a different valid base64 +/// char. Decoded length is unchanged (single non-padding char swap), so +/// length checks still pass and the commitment is the only thing that trips. +fn flip_first_b64_char(s: &str) -> String { + let mut chars: Vec = s.chars().collect(); + if let Some(first) = chars.first_mut() { + *first = if *first == 'A' { 'B' } else { 'A' }; + } + chars.into_iter().collect() +} + +#[test] +fn tampered_kdf_time_cost_is_rejected() { + let dir = tempdir().unwrap(); + // 3 -> 5: still inside the accepted 1..=10 range, so it clears + // validate_kdf_params and only the commitment can catch it. + unlock_with_tamper(dir.path(), "time_cost", |doc| { + doc["kdf"]["time_cost"] = Value::from(5u32); + }); +} + +#[test] +fn tampered_kdf_memory_cost_is_rejected() { + let dir = tempdir().unwrap(); + // 65536 -> 32768: inside 8192..=262144. + unlock_with_tamper(dir.path(), "memory_cost", |doc| { + doc["kdf"]["memory_cost"] = Value::from(32768u32); + }); +} + +#[test] +fn tampered_kdf_parallelism_is_rejected() { + let dir = tempdir().unwrap(); + // 4 -> 2: inside 1..=32. + unlock_with_tamper(dir.path(), "parallelism", |doc| { + doc["kdf"]["parallelism"] = Value::from(2u32); + }); +} + +#[test] +fn tampered_kdf_salt_is_rejected() { + let dir = tempdir().unwrap(); + unlock_with_tamper(dir.path(), "salt", |doc| { + let salt = doc["kdf"]["salt"].as_str().unwrap(); + doc["kdf"]["salt"] = Value::from(flip_first_b64_char(salt)); + }); +} + +#[test] +fn tampered_kem_public_key_is_rejected() { + let dir = tempdir().unwrap(); + unlock_with_tamper(dir.path(), "kem_pub", |doc| { + let pk = doc["kem"]["public_key"].as_str().unwrap(); + doc["kem"]["public_key"] = Value::from(flip_first_b64_char(pk)); + }); +} + +#[test] +fn tampered_x25519_public_key_is_rejected() { + let dir = tempdir().unwrap(); + unlock_with_tamper(dir.path(), "x_pub", |doc| { + let pk = doc["x25519"]["public_key"].as_str().unwrap(); + doc["x25519"]["public_key"] = Value::from(flip_first_b64_char(pk)); + }); +} + +#[test] +fn tampered_suite_is_rejected() { + let dir = tempdir().unwrap(); + unlock_with_tamper(dir.path(), "suite", |doc| { + doc["suite"] = Value::from("dota-v7-tchkem-mlkem768-x25519-aes256gcm-EVIL"); + }); +} + +#[test] +fn tampered_min_version_is_rejected() { + let dir = tempdir().unwrap(); + // 7 -> 6: still <= V7, so it passes the "requires newer dota" gate and + // only the commitment (which covers min_version) catches the change. + unlock_with_tamper(dir.path(), "min_version", |doc| { + doc["min_version"] = Value::from(6u32); + }); +} diff --git a/tests/stdin_overflow.rs b/tests/stdin_overflow.rs new file mode 100644 index 0000000..5216945 --- /dev/null +++ b/tests/stdin_overflow.rs @@ -0,0 +1,93 @@ +//! Regression: `dota set` reads a piped secret value from stdin with a hard +//! 1 MiB cap (`MAX_STDIN_SECRET_BYTES` in `src/cli/commands.rs`). A hostile +//! or accidental pipe that exceeds the cap must be refused -- the secret must +//! not be stored, bounding memory and preventing a giant value from silently +//! landing in the vault. +//! +//! Driven through the real binary because the cap lives on the stdin read +//! path, which is only exercised when stdin is piped (not a tty). + +#![cfg(unix)] + +use dota::vault::ops::{create_vault, list_secrets, unlock_vault}; +use std::io::Write; +use std::process::{Command, Stdio}; +use tempfile::tempdir; + +const PASS: &str = "correct horse battery staple"; +const DOTA: &str = env!("CARGO_BIN_EXE_dota"); +const MAX_STDIN_SECRET_BYTES: usize = 1024 * 1024; + +/// Pipe `value` into `dota set NAME` and return (success, combined stderr). +fn run_set(vault: &str, name: &str, value: &[u8]) -> (bool, String) { + let mut child = Command::new(DOTA) + .args(["set", name, "--vault", vault]) + .env("DOTA_PASSPHRASE", PASS) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn dota set"); + + // Write in a scope so the pipe closes (EOF) before we wait. + child + .stdin + .take() + .unwrap() + .write_all(value) + .expect("write stdin"); + + let out = child.wait_with_output().expect("wait dota set"); + ( + out.status.success(), + String::from_utf8_lossy(&out.stderr).to_string(), + ) +} + +#[test] +fn oversized_stdin_secret_is_refused_and_not_stored() { + let dir = tempdir().unwrap(); + let vault = dir.path().join("vault.json"); + let vault = vault.to_str().unwrap(); + create_vault(PASS, vault).unwrap(); + + // One byte over the cap. + let payload = vec![b'A'; MAX_STDIN_SECRET_BYTES + 1]; + let (ok, stderr) = run_set(vault, "BIG", &payload); + + assert!(!ok, "set must fail on oversized stdin; stderr: {stderr}"); + assert!( + stderr.contains("exceeds") && stderr.contains("bytes"), + "expected an overflow refusal message, got: {stderr}" + ); + + // The secret must not have been persisted. + let unlocked = unlock_vault(PASS, vault).unwrap(); + assert!( + !list_secrets(&unlocked).contains(&"BIG".to_string()), + "oversized secret must not be stored" + ); +} + +#[test] +fn exactly_max_stdin_secret_is_accepted() { + let dir = tempdir().unwrap(); + let vault = dir.path().join("vault.json"); + let vault = vault.to_str().unwrap(); + create_vault(PASS, vault).unwrap(); + + // Exactly at the cap (all printable, no trailing newline to trim). + let payload = vec![b'A'; MAX_STDIN_SECRET_BYTES]; + let (ok, stderr) = run_set(vault, "ATCAP", &payload); + + assert!( + ok, + "set at exactly the cap should succeed; stderr: {stderr}" + ); + + let unlocked = unlock_vault(PASS, vault).unwrap(); + assert!( + list_secrets(&unlocked).contains(&"ATCAP".to_string()), + "at-cap secret should be stored" + ); +} From dfe68e625fc2ac5ef3a8947b6f28995df4e8beb5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 23:57:59 +0000 Subject: [PATCH 2/3] cargo-deny: accept known advisories (legacy pqcrypto, OsRng, pinned ml-kem) The advisories check denied five findings that the existing rustsec audit-check job only warns on. All are known and accepted: - RUSTSEC-2024-0381 / 2026-0162 / 2026-0163: the pqcrypto-* crates are the read-only legacy v1-v5 Kyber migration path (feature = "legacy-migration"). v6+ vaults use the maintained ml-kem crate. 2024-0381 is already accepted in .cargo/audit.toml; the two 2026 IDs are its newly-split siblings. - RUSTSEC-2026-0097 (rand 0.8.5 unsound): triggers only via a custom `log` logger calling thread_rng(); dota uses OsRng directly. Patch bump to rand >=0.8.6 tracked as follow-up. - digest 0.11.1 yanked: pulled in transitively by the pinned ml-kem 0.3.2, whose byte layout is part of the v7 on-disk contract. Set yanked = "warn" since the pin is not ours to bump. Each ignore carries a documented reason. Validated locally: the config parses and bans/licenses/sources pass (advisories DB fetch needs network, available in CI). --- deny.toml | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/deny.toml b/deny.toml index 386aebd..095c7e7 100644 --- a/deny.toml +++ b/deny.toml @@ -14,10 +14,32 @@ all-features = true [advisories] -# RUSTSEC advisory database. Fail the build on any unmaintained or vulnerable -# crate; refuse yanked crates. Mirrors/extends the audit-check job. -yanked = "deny" +# RUSTSEC advisory database. Vulnerabilities, unmaintained, and unsound +# advisories are denied by default (cargo-deny v2); the ignores below are the +# explicitly-accepted exceptions, each justified. This extends the existing +# `rustsec/audit-check` job (which only hard-fails on vulnerabilities and +# reads `.cargo/audit.toml`). version = 2 +# A genuinely-yanked *direct* dependency is worth a hard stop, but the only +# yanked crate in our tree (digest 0.11.1) is pulled in transitively by the +# pinned `ml-kem 0.3.2`, whose byte layout is part of the v7 on-disk contract +# and therefore cannot be bumped here. Warn rather than block CI on a pin we +# do not control. +yanked = "warn" +ignore = [ + # --- Legacy v1-v5 Kyber migration path only (feature = "legacy-migration"). + # The pqcrypto-* ecosystem wraps PQClean, which upstream is archiving. v6+ + # vaults use the actively-maintained `ml-kem` crate; these deps exist only + # to read forward pre-v6 vaults. Migration to `pqcrypto-mlkem` is tracked. + { id = "RUSTSEC-2024-0381", reason = "pqcrypto-kyber: legacy-migration only; v6+ uses ml-kem" }, + { id = "RUSTSEC-2026-0162", reason = "pqcrypto-traits unmaintained: legacy-migration path only" }, + { id = "RUSTSEC-2026-0163", reason = "pqcrypto-internals unmaintained: legacy-migration path only" }, + # --- rand 0.8.5 unsoundness requires a custom `log` logger that calls + # `thread_rng()` and reseeds mid-log. dota uses `OsRng`/getrandom directly + # and defines no such logger, so the unsound path is unreachable. A patch + # bump to rand >=0.8.6 is a tracked follow-up. + { id = "RUSTSEC-2026-0097", reason = "rand thread_rng+log unsoundness; dota uses OsRng, not thread_rng" }, +] [licenses] # Vetted permissive licenses. Every expression in the current tree resolves From 5356d2413bc080fa5ce4882ac11b7a1ceb8e6465 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 14 Jun 2026 00:00:29 +0000 Subject: [PATCH 3/3] Address Copilot review: fix three doc-comment accuracy nits - stdin_overflow: helper returns stderr only, not 'combined stderr'. - argon2_dos: use 1_048_576 KiB so the value matches the '1 GiB' comment; note memory_cost is in KiB. Still far above the 256 MiB ceiling. - header_tamper_v7: helper asserts internally and returns (), so describe it as asserting the unlock fails rather than returning a result. --- tests/argon2_dos.rs | 4 ++-- tests/header_tamper_v7.rs | 3 ++- tests/stdin_overflow.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/argon2_dos.rs b/tests/argon2_dos.rs index 35b78f5..522295e 100644 --- a/tests/argon2_dos.rs +++ b/tests/argon2_dos.rs @@ -48,11 +48,11 @@ fn assert_rejected_fast(tag: &str, field: &str, value: Value, expect_substr: &st #[test] fn excessive_memory_cost_is_rejected_before_argon2() { - // 1 GiB requested -- far above the 256 MiB ceiling. + // 1 GiB requested (memory_cost is in KiB) -- far above the 256 MiB ceiling. assert_rejected_fast( "memory", "memory_cost", - Value::from(1_000_000u32), + Value::from(1_048_576u32), "Invalid Argon2 memory cost", ); } diff --git a/tests/header_tamper_v7.rs b/tests/header_tamper_v7.rs index f6e7b43..c4a7dc0 100644 --- a/tests/header_tamper_v7.rs +++ b/tests/header_tamper_v7.rs @@ -19,7 +19,8 @@ use tempfile::tempdir; const PASS: &str = "correct horse battery staple"; /// Build a fresh v7 vault, parse its JSON, hand it to `mutate`, write the -/// mutated JSON to a new path, and return that path's unlock result. +/// mutated JSON to a new path, and assert that unlocking that path fails +/// with a tamper/rejection error. fn unlock_with_tamper(dir: &std::path::Path, tag: &str, mutate: impl FnOnce(&mut Value)) { let base = dir.join("base.json"); create_vault(PASS, base.to_str().unwrap()).unwrap(); diff --git a/tests/stdin_overflow.rs b/tests/stdin_overflow.rs index 5216945..c1a7c40 100644 --- a/tests/stdin_overflow.rs +++ b/tests/stdin_overflow.rs @@ -18,7 +18,7 @@ const PASS: &str = "correct horse battery staple"; const DOTA: &str = env!("CARGO_BIN_EXE_dota"); const MAX_STDIN_SECRET_BYTES: usize = 1024 * 1024; -/// Pipe `value` into `dota set NAME` and return (success, combined stderr). +/// Pipe `value` into `dota set NAME` and return (success, stderr). fn run_set(vault: &str, name: &str, value: &[u8]) -> (bool, String) { let mut child = Command::new(DOTA) .args(["set", name, "--vault", vault])