Skip to content

test(core): tamper, truncation, downgrade and property tests - #21

Merged
ericliswan merged 7 commits into
mainfrom
test/core-security-suite
Oct 27, 2026
Merged

ericliswan merged 7 commits into
mainfrom
test/core-security-suite

Conversation

@ericliswan

Copy link
Copy Markdown
Owner

Summary

Adds the security test suite in crates/pwv-core/tests/. Every test drives real vault files through the public storage API: wrong password, any flipped byte, every truncated length, an extended file, a memory-cost downgrade, a property-based round trip, and nonce/ciphertext freshness. Two storage unit tests are removed because the new tests strictly supersede them.

Closes #20

Type of change

  • feat - [ ] fix - [ ] docs - [x] test - [ ] refactor - [ ] ci/chore - [ ] security

How was this tested?

  • cargo fmt --all --check, cargo clippy --workspace --all-targets [--all-features] -- -D warnings, cargo test --workspace [--all-features], cargo deny check: all pass locally.
  • New suite runtime about 3 s locally (tamper.rs 4 tests, roundtrip.rs 2 tests, 64 proptest cases).
  • Mutation checks:
    • Making both seal and open use an empty AAD fails memory_cost_downgrade_is_rejected at the crypto::open(&key, &downgraded) assertion.
    • The byte-flip test still passed under that mutation. Through unlock, the salt and parameters change the derived key, the nonce changes the keystream, and the ids fail parsing. So the flip test proves the end-to-end claim, and the downgrade test is the one that guards the AAD.

Security checklist

  • No secret in args, env vars, logs, Debug output, errors or panics
  • Crypto is called only from pwv-core::kdf / pwv-core::crypto (tests use those modules, never a crypto crate)
  • Any new header/file-format field is inside the AAD and validated on load (no format change)
  • Failure paths have tests (wrong password, tampering, bad input)
  • Docs (README / DESIGN / THREAT_MODEL) updated if behaviour changed (no behaviour change)

Notes for the reviewer

  • Feature gating: both files are #![cfg(feature = "insecure-test-kdf")], so they run in the --all-features CI step and compile to nothing in the featureless step.
  • "README claim:" lines: each doc comment names its claim this way. The README is still a stub, so PR perf(kdf): benchmark and calibrate default parameters #23 (v1 README) will quote these claims and link each to its test.
  • Byte flips use XOR 0xFF. Flipping m_cost byte 7 yields 64 MiB, which is in range, so one real Argon2 run at that size happens (under 1 s in debug).
  • Below-floor downgrade: under the test profile the floor (8 KiB) equals Argon2's own minimum, so this test can't show that our validation runs before Argon2. The header/kdf unit tests cover that with production values in the featureless run.
  • Clippy: allow-unwrap-in-tests doesn't apply to helper functions in integration-test crates, hence a file-level #![allow(clippy::unwrap_used)] with a comment.
  • New dev-dependency: proptest 1.12.0 (MIT OR Apache-2.0); cargo deny is clean.

Drives storage::unlock on a real file rather than crypto::open, so the
test covers the whole path a user hits. It also pins which error each
header region produces.

Refs #20
Files shorter than header + tag must fail as Format before any key is
derived. Longer ones that are cut or padded must fail authentication.

Refs #20
Builds a vault above the floor, then rewrites m_cost in the header. The
original key must still fail to open it. That assertion is the one that
breaks if the header ever drops out of the associated data. Salt,
parameters and nonce all change the key or keystream anyway.

Refs #20
Random entries go through create, save and unlock on disk. Fields hold
arbitrary Unicode, including quotes, backslashes and control
characters. Timestamps span the full RFC 3339 range at nanosecond
precision. The test catches anything the JSON or file layers fail to
preserve exactly. Adds proptest as a dev-dependency.

Refs #20
Saves the same plaintext twice and checks that the salt is kept while
the nonce and ciphertext both change. This supersedes the storage unit
test that checked only the nonce, so that test is removed.

Refs #20
Snapshots every file in the vault directory, names and bytes, before
and after a failed unlock. A rewritten vault or .bak fails the test, and
so does a stray temp file. The test also pins the exact error message,
so a wrong password and a corrupted file stay indistinguishable. This
supersedes the storage unit test that checked only the error variant.

Refs #20
Without this baseline, the flip and truncation tests would pass
vacuously if unlock always failed.

Refs #20
@ericliswan

Copy link
Copy Markdown
Owner Author

Self-review: I read the full diff (git diff main...HEAD, excluding the lockfile, about 300 lines).

  • Fixed in review: the flip and truncation tests didn't check that the untampered vault unlocks, so they could pass vacuously if unlock always failed. Added a baseline unlock in new_vault_bytes() (087515d). The downgrade test already checked its baseline.
  • Mutation-checked: an empty AAD in seal/open fails the downgrade test at the fixed-key crypto::open assertion.
  • Secrets: no real secrets. Proptest failure output would show randomly generated test strings only.
  • Crypto calls: tests call pwv_core::kdf / pwv_core::crypto, never a crypto crate directly.
  • Removed tests: the two storage unit tests are strictly covered by wrong_password_fails_and_leaves_files_unchanged and two_saves_produce_different_nonces_and_ciphertexts.
  • For the owner: the "README claim:" wording in each doc comment is what PR perf(kdf): benchmark and calibrate default parameters #23 will quote and link. Change any you'd phrase differently.

No 🛑 checkpoint on this PR; CI is green on all four required checks.

@ericliswan
ericliswan merged commit 818c788 into main Oct 27, 2026
4 checks passed
@ericliswan
ericliswan deleted the test/core-security-suite branch October 27, 2026 15:29
@ericliswan ericliswan mentioned this pull request Oct 27, 2026
5 of 6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(core): tamper, truncation, downgrade and property tests

1 participant