Skip to content

Security audit: pen-test EncryptionUtility (the existing 'v1:' secret format) #212

Description

@ChrisonSimtian

Context

We ship a basic secret-encryption feature in src/Fallout.Utilities/Security/EncryptionUtility.cs. It's what backs fallout :secrets and the v1:-prefixed encrypted values consumers commit into .fallout/parameters.json.

The current docs page (docs/06-global-tool/02-secrets.md) already carries a "provided AS IS, please review and open an issue for flaws" disclaimer. We never actually did that review under Fallout maintenance. This issue tracks doing it.

Concrete weaknesses worth checking carefully

Inherited from NUKE; not yet validated as still acceptable post-takeover.

  1. Static salt. GetCryptoStream hardcodes byte[] salt = { 0x49, 0x76, 0x61, 0x6e, ... } (the bytes spell "Ivan Medvedev"). Same salt across every consumer of every Fallout version → a single rainbow table of common passwords compromises every encrypted secret on the public internet. Should be per-secret random salt prepended to ciphertext.
  2. 10,000 PBKDF2 iterations. OWASP's 2023 PBKDF2-SHA256 recommendation is 600,000 iterations. 10K is severely underprovisioned for 2026 hardware. Cracking budget for a single password is on the order of seconds at GPU rates.
  3. IV derived from the same KDF as the key. pdb.GetBytes(16) after pdb.GetBytes(32) — deterministic from password. Same password → same IV. For CBC mode (which AES.Create() defaults to) this leaks "two ciphertexts encrypt the same plaintext" and breaks semantic security. Should be fresh random IV per encrypt, prepended to ciphertext.
  4. No authentication tag. AES-CBC is malleable — an attacker can flip ciphertext bits to flip corresponding plaintext bits without Decrypt detecting tampering. If a secret value is later fed into a tool invocation, this could enable poisoning. Should be AES-GCM (or AES-CBC + HMAC-SHA256), with the auth tag stored alongside the ciphertext.
  5. No KDF version inside the encrypted blob. The v1: prefix marks the format version, which is good forethought — but only one version exists today. Need to confirm the v2: migration story works before changing anything (Decrypt looks at the prefix, but the logic doesn't read it).

What "pen-test this with Claude" looks like

Walk through the four weakness areas above with a fresh threat model. For each:

  • What's the realistic attacker capability? (Stolen parameters.json? Disgruntled ex-developer? Network MITM?)
  • What does the current implementation actually leak/permit?
  • How big is the gap to industry-standard defaults (AES-GCM, random salt + IV, modern PBKDF2 iterations, or migrate to Argon2id)?
  • What's the cleanest migration path — bump to v2: with new shape, keep v1: decryption working through the 10.x line, force re-encrypt during fallout :secrets in 11.x?

The output is a security-grade write-up of the current state and a concrete remediation plan, tracked here as comments or linked PRs.

Milestone

Maintainer pick — feels like v11 to me (the docs already invite the audit; weaknesses #1#4 are concrete enough to act on now). Could land as v2: format with backward-compat read of v1: during the 10.x deprecation window.

Refs

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions