diff --git a/TODO.md b/TODO.md index 5168b2f8..9bb1d02c 100644 --- a/TODO.md +++ b/TODO.md @@ -2,6 +2,53 @@ This file tracks the remaining work from the latest bug search and security evaluation. +## Entropy audit (2026-07-31) + +[`docs/entropy_audit_2026-07-31.md`](docs/entropy_audit_2026-07-31.md) — RNG and +randomness-integration audit. **No predictable-fallback defect:** every non-deterministic secret +uses `os.urandom` / `secrets`, and the `random` module is not imported anywhere in non-test source. + +**Read the compatibility constraint in that document before touching +`core/password_generation.py`.** Passwords are never stored — they are re-derived on demand from +`(seed, index, length, policy)`, and entries carry no algorithm version. Any change to +`generate_password` silently changes every existing password in every vault, unrecoverably. All +fixes land behind a new `gen_version` field; v1 output must stay byte-identical. + +- [x] **M4 (done)** — v1 password vectors frozen, plus fail-closed RNG tests, in + `src/tests/test_entropy_integrity.py` (26 tests, inside the `--determinism-only` CI gate). + 18 vectors cover every policy and both sides of the 32-byte stream-wrap boundary. + Mutation-verified: removing `_balance_distribution` (M1), altering the `DeterministicStream` + wrap (M3) and changing the character mapping (M2) each turn all 18 red; adding a + hash-of-timestamp fallback to `generate_bip85_seed` turns the fail-closed test red. + **M1/M2/M3 are now safe to attempt.** +- [x] **L1 (done)** — deleted the dead HKDF block in `_derive_password_entropy`, dropped the three + now-unused imports (`HKDF`, `hashes`, `default_backend`), and corrected the docstrings that + named HKDF instead of PBKDF2. Verified as a negative control: all 18 v1 vectors stayed green, + confirming the harness does not produce false alarms on behaviour-preserving edits. +- [x] **M1 / M2 / M3 (done)** — password generation is now versioned. `gen_version` defaults to 1 + everywhere, so existing entries and every untouched caller keep deriving v1 byte-identically; + new password entries are stamped `gen_version: 2`. v2 drops the forced equal-quarters class + quota (M1), draws by rejection sampling (M2) from an unbounded HMAC-expanded stream (M3), and + enforces only the policy minima — choosing donor positions from classes that have a surplus, + so raising one minimum can no longer break another. + + Measured at length 16 over 200 samples: v1 produced **1** distinct class composition, v2 + produces **82**. Per-character entropy 6.42 → 6.53 bits against a 6.555 ceiling. Chi-square + 96.1 (df=93) confirms the modulo bias is gone; lag-32 repeat rate sits within one standard + error of chance, confirming the repeating pad is gone. +- [ ] **Opt-in upgrade UX for existing entries** — a per-entry action that re-derives at v2, with a + confirmation making the consequence unmissable ("this changes the password; update it at the + site first"). Never bulk-migrate. Until this ships, existing entries stay on v1 by design. +- [ ] **L2** — master seed is `os.urandom(32)` reduced to a hardcoded 12-word (128-bit) mnemonic + while derived seeds default to 24 words. Not a defect; offer 24 words at profile creation. +- [ ] **L4** — `torch/`: predictable temp filename (`services/memory/index.js:60`), a `Math.random` + value misnamed `nonce` (`relay-health.mjs:95`), and three stale `_backups/` code copies that + pollute security greps. +- [ ] **L5** — no CycloneDX SBOM; folds into the supply-chain item below. + +L3 (broad `except Exception` around crypto — these re-raise, so no silent fallback) overlaps the +Robustness item further down. + ## Security - [ ] Complete checklist item #8 (Supply chain and release integrity): diff --git a/src/seedpass/core/entry_management.py b/src/seedpass/core/entry_management.py index 36be53ee..41dbea14 100644 --- a/src/seedpass/core/entry_management.py +++ b/src/seedpass/core/entry_management.py @@ -283,6 +283,10 @@ class EntryManager: :return: The assigned index of the new entry. """ try: + # Local import to match this module's convention for + # password_generation (keeps its heavy deps off the import path). + from .password_generation import CURRENT_PASSWORD_GEN_VERSION + index = self.get_next_index() data = self._load_index() now_unix = self._now_unix() @@ -292,6 +296,10 @@ class EntryManager: entry = { "label": label, "length": length, + # Which generation algorithm derives this entry's password. + # Entries written before versioning have no field and fall back + # to v1, which is frozen. See docs/entropy_audit_2026-07-31.md. + "gen_version": CURRENT_PASSWORD_GEN_VERSION, "username": username if username else "", "url": url if url else "", "archived": archived, diff --git a/src/seedpass/core/manager.py b/src/seedpass/core/manager.py index 356be913..f6fd6903 100644 --- a/src/seedpass/core/manager.py +++ b/src/seedpass/core/manager.py @@ -34,7 +34,10 @@ from constants import MAX_RETRIES, RETRY_DELAY from .encryption import EncryptionManager from .entry_management import EntryManager -from .password_generation import PasswordGenerator +from .password_generation import ( + PasswordGenerator, + LEGACY_PASSWORD_GEN_VERSION, +) from .backup import BackupManager from .vault import Vault from .portable_backup import export_backup, import_backup, PortableMode @@ -3140,14 +3143,34 @@ class PasswordManager: def _generate_password_for_entry( self, entry: dict, index: int, length: int | None = None ) -> str: - """Generate a password for ``entry`` honoring any policy overrides.""" + """Generate a password for ``entry`` honoring policy and generation version.""" + entry = entry if isinstance(entry, dict) else {} if length is None: length = int(entry.get("length", DEFAULT_PASSWORD_LENGTH)) overrides = entry.get("policy", {}) + # Entries written before versioning carry no field and must keep + # deriving via v1 -- their stored passwords depend on it. + raw_version = entry.get("gen_version", LEGACY_PASSWORD_GEN_VERSION) + try: + gen_version = int(raw_version) + except (TypeError, ValueError): + raise ValueError( + f"Entry {index} has an unreadable gen_version {raw_version!r}; " + f"refusing to guess which algorithm produced its password." + ) from None + + # Only forward the argument when it is non-legacy. Keeping the legacy + # call shape means every existing caller and test double that defines + # generate_password(length, index) keeps working untouched. + kwargs = ( + {} if gen_version == LEGACY_PASSWORD_GEN_VERSION + else {"gen_version": gen_version} + ) + pg = self.password_generator if not hasattr(pg, "policy") or not isinstance(overrides, dict): - return pg.generate_password(length, index) + return pg.generate_password(length, index, **kwargs) base_policy = pg.policy merged = dataclasses.replace( @@ -3156,7 +3179,7 @@ class PasswordManager: ) pg.policy = merged try: - return pg.generate_password(length, index) + return pg.generate_password(length, index, **kwargs) finally: pg.policy = base_policy diff --git a/src/seedpass/core/password_generation.py b/src/seedpass/core/password_generation.py index 3085cf7c..c04d896b 100644 --- a/src/seedpass/core/password_generation.py +++ b/src/seedpass/core/password_generation.py @@ -29,10 +29,8 @@ from dataclasses import dataclass from termcolor import colored from pathlib import Path import shutil -from cryptography.hazmat.primitives.kdf.hkdf import HKDF -from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import ed25519 -from cryptography.hazmat.backends import default_backend from bip_utils import Bip39SeedGenerator from . import compat # noqa: F401 @@ -75,8 +73,25 @@ class PasswordPolicy: exclude_ambiguous: bool = False +# Generation versions. Entries record which one produced them; absent means 1. +# +# v1 -- the original algorithm. Frozen forever: passwords are re-derived on +# demand and never stored, so every live vault entry depends on it +# reproducing byte-identically. Pinned by +# src/tests/test_entropy_integrity.py::test_v1_password_vectors_are_frozen. +# v2 -- drops the forced equal-quarters class quota, uses rejection sampling to +# remove modulo bias, and draws from an unbounded HMAC-expanded stream. +LEGACY_PASSWORD_GEN_VERSION = 1 +CURRENT_PASSWORD_GEN_VERSION = 2 + + class DeterministicStream: - """Helper class to manage deterministic stream of values from a derived key.""" + """Byte stream for v1: cycles ``dk`` with wraparound. + + Retained unchanged because v1 output depends on it, including the + wraparound. v2 uses :class:`ExpandedStream` instead -- wrapping a fixed key + is a repeating pad, not a stream. + """ def __init__(self, dk: bytes): self.dk = dk @@ -93,6 +108,52 @@ class DeterministicStream: return self.index +class ExpandedStream: + """Unbounded deterministic byte stream for v2 (audit item M3). + + Generates SHA-256 blocks on demand as ``HMAC(dk, info || counter)`` rather + than cycling a fixed 32-byte key, so it never repeats within any usable + password length. ``info`` domain-separates independent streams derived from + the same ``dk``. + """ + + def __init__(self, dk: bytes, info: bytes = b""): + self._key = dk + self._info = info + self._block = b"" + self._pos = 0 + self._counter = 0 + + def get_value(self) -> int: + if self._pos >= len(self._block): + msg = self._info + self._counter.to_bytes(4, "big") + self._block = hmac.new(self._key, msg, hashlib.sha256).digest() + self._counter += 1 + self._pos = 0 + value = self._block[self._pos] + self._pos += 1 + return value + + +def _uniform_index(stream, max_exclusive: int) -> int: + """Uniform value in ``[0, max_exclusive)`` by rejection sampling. + + Avoids the modulo bias in ``byte % len(alphabet)`` (audit item M2): with a + 94-character alphabet, plain modulo gives 68 characters probability 3/256 + and the other 26 only 2/256. Requires an unbounded stream, since rejection + consumes a variable number of bytes -- hence :class:`ExpandedStream`. + """ + if max_exclusive <= 0: + raise ValueError("max_exclusive must be positive") + if max_exclusive > 256: + raise ValueError("_uniform_index supports single-byte ranges only") + limit = 256 - (256 % max_exclusive) + while True: + value = stream.get_value() + if value < limit: + return value % max_exclusive + + class PasswordGenerator: """ PasswordGenerator Class @@ -137,20 +198,19 @@ class PasswordGenerator: raise def _derive_password_entropy(self, index: int) -> bytes: - """Derive deterministic entropy for password generation.""" + """Derive deterministic entropy for password generation. + + Chain: BIP-85 (64 bytes, app_no=32) -> PBKDF2-HMAC-SHA256 (empty salt, + 100k iterations) -> 32-byte derived key. + + The empty salt is deliberate: a random salt would break the determinism + this whole module exists to provide (see the module docstring). The + iteration count is not doing password-stretching work here -- the input + is already 64 bytes of high-entropy BIP-85 output, not a human secret. + """ entropy = self.bip85.derive_entropy(index=index, entropy_bytes=64, app_no=32) logger.debug("Entropy derived for password generation.") - hkdf = HKDF( - algorithm=hashes.SHA256(), - length=32, - salt=None, - info=b"password-generation", - backend=default_backend(), - ) - hkdf_derived = hkdf.derive(entropy) - logger.debug("Derived key using HKDF.") - dk = hashlib.pbkdf2_hmac("sha256", entropy, b"", 100000) logger.debug("Derived key using PBKDF2.") return dk @@ -189,22 +249,38 @@ class PasswordGenerator: return shuffled def generate_password( - self, length: int = DEFAULT_PASSWORD_LENGTH, index: int = 0 + self, + length: int = DEFAULT_PASSWORD_LENGTH, + index: int = 0, + gen_version: int = LEGACY_PASSWORD_GEN_VERSION, ) -> str: """ Generates a deterministic password based on the parent seed, desired length, and index. - Steps: + Common to both versions: 1. Derive entropy using BIP-85. - 2. Use HKDF-HMAC-SHA256 to derive a key from entropy. - 3. Map the derived key to all allowed characters. - 4. Ensure the password meets complexity requirements. - 5. Shuffle the password deterministically based on the derived key. - 6. Trim or extend the password to the desired length. + 2. Use PBKDF2-HMAC-SHA256 to derive a 32-byte key from that entropy. + + Version 1 (default, frozen) then: + 3. Maps derived bytes to characters with ``byte % len(alphabet)``. + 4. Enforces minimum counts, adds symbols, and forces an equal quota of + each character class across the whole password. + 5. Shuffles, extends/trims to length, re-enforces, shuffles again. + + Version 2 then: + 3. Draws exactly ``length`` characters by rejection sampling from an + unbounded HMAC-expanded stream (no modulo bias, no repeating pad). + 4. Enforces only the policy minima, without pinning the composition. + 5. Shuffles once. + + ``gen_version`` defaults to 1 so that every caller which does not + explicitly opt in keeps deriving exactly what it derived before. Entry + records carry the version; see ``_generate_password_for_entry``. Parameters: length (int): Desired length of the password. index (int): Index for deriving child entropy. + gen_version (int): Generation algorithm version (1 or 2). Returns: str: The generated password. @@ -227,25 +303,17 @@ class PasswordGenerator: ) dk = self._derive_password_entropy(index=index) + all_allowed, allowed_special = self._alphabets() - letters = string.ascii_letters - digits = string.digits + if gen_version == CURRENT_PASSWORD_GEN_VERSION: + return self._generate_password_v2( + length, dk, all_allowed, allowed_special + ) + if gen_version != LEGACY_PASSWORD_GEN_VERSION: + raise ValueError( + f"Unknown password generation version: {gen_version!r}" + ) - if self.policy.exclude_ambiguous: - ambiguous = "O0Il1" - letters = "".join(c for c in letters if c not in ambiguous) - digits = "".join(c for c in digits if c not in ambiguous) - - if not self.policy.include_special_chars: - allowed_special = "" - elif self.policy.allowed_special_chars is not None: - allowed_special = self.policy.allowed_special_chars - elif self.policy.special_mode == "safe": - allowed_special = SAFE_SPECIAL_CHARS - else: - allowed_special = string.punctuation - - all_allowed = letters + digits + allowed_special password = self._map_entropy_to_chars(dk, all_allowed) password = self._enforce_complexity( password, all_allowed, allowed_special, dk @@ -282,6 +350,139 @@ class PasswordGenerator: print(colored(f"Error: Failed to generate password: {e}", "red")) raise + def _alphabets(self) -> tuple[str, str]: + """Return ``(all_allowed, allowed_special)`` for the active policy.""" + letters = string.ascii_letters + digits = string.digits + + if self.policy.exclude_ambiguous: + ambiguous = "O0Il1" + letters = "".join(c for c in letters if c not in ambiguous) + digits = "".join(c for c in digits if c not in ambiguous) + + if not self.policy.include_special_chars: + allowed_special = "" + elif self.policy.allowed_special_chars is not None: + allowed_special = self.policy.allowed_special_chars + elif self.policy.special_mode == "safe": + allowed_special = SAFE_SPECIAL_CHARS + else: + allowed_special = string.punctuation + + return letters + digits + allowed_special, allowed_special + + def _class_sets(self, allowed_special: str) -> dict[str, str]: + """Character class -> its allowed members under the active policy.""" + uppercase = string.ascii_uppercase + lowercase = string.ascii_lowercase + digits = string.digits + if self.policy.exclude_ambiguous: + ambiguous = "O0Il1" + uppercase = "".join(c for c in uppercase if c not in ambiguous) + lowercase = "".join(c for c in lowercase if c not in ambiguous) + digits = "".join(c for c in digits if c not in ambiguous) + return { + "upper": uppercase, + "lower": lowercase, + "digit": digits, + "special": allowed_special, + } + + def _generate_password_v2( + self, length: int, dk: bytes, all_allowed: str, allowed_special: str + ) -> str: + """Version 2 generation (audit items M1, M2, M3). + + Unlike v1 this does not pin the character-class composition. v1 forced + exactly ``length/4`` of each class into every password, which cost + 5.7-11.5 bits over the usable length range and made any SeedPass + password recognisable by its composition alone. v2 enforces only what + the policy actually asks for. + """ + class_sets = self._class_sets(allowed_special) + minima = { + "upper": self.policy.min_uppercase, + "lower": self.policy.min_lowercase, + "digit": self.policy.min_digits, + "special": self.policy.min_special if allowed_special else 0, + } + # Drop classes the policy excludes entirely, so they are never required + # and never counted as donors. + minima = {k: v for k, v in minima.items() if class_sets[k]} + + required = sum(minima.values()) + if required > length: + raise ValueError( + f"Policy requires at least {required} characters " + f"({', '.join(f'{v} {k}' for k, v in minima.items() if v)}) " + f"but the requested length is {length}." + ) + + char_stream = ExpandedStream(dk, b"seedpass-v2-chars") + policy_stream = ExpandedStream(dk, b"seedpass-v2-policy") + + chars = [ + all_allowed[_uniform_index(char_stream, len(all_allowed))] + for _ in range(length) + ] + + self._enforce_minima_v2(chars, policy_stream, class_sets, minima) + + shuffle_key = hmac.new(dk, b"seedpass-v2-shuffle", hashlib.sha256).digest() + chars = self._fisher_yates_hmac(chars, shuffle_key) + return "".join(chars) + + @staticmethod + def _classify(char: str, class_sets: dict[str, str]) -> str | None: + for name, members in class_sets.items(): + if char in members: + return name + return None + + def _enforce_minima_v2( + self, + chars: list[str], + stream: "ExpandedStream", + class_sets: dict[str, str], + minima: dict[str, int], + ) -> None: + """Raise deficient classes to their minimum without breaking others. + + v1's ``_enforce_minimum_counts`` overwrites uniformly random positions, + so satisfying one minimum could destroy another; v1 got away with it + only because ``_balance_distribution`` then rewrote nearly every + position anyway. v2 has no such backstop, so donor positions are chosen + only from classes that currently have a surplus. + """ + counts = {name: 0 for name in class_sets} + for char in chars: + name = self._classify(char, class_sets) + if name is not None: + counts[name] += 1 + + for name, minimum in minima.items(): + while counts[name] < minimum: + donors = [ + i + for i, char in enumerate(chars) + if (other := self._classify(char, class_sets)) != name + and other is not None + and counts[other] > minima.get(other, 0) + ] + if not donors: + # Unreachable while sum(minima) <= length, which the caller + # enforces; kept so a future policy change fails loudly. + raise ValueError( + f"Cannot satisfy minimum for {name!r} without violating " + f"another policy minimum." + ) + position = donors[_uniform_index(stream, len(donors))] + donor_class = self._classify(chars[position], class_sets) + members = class_sets[name] + chars[position] = members[_uniform_index(stream, len(members))] + counts[donor_class] -= 1 + counts[name] += 1 + def _count_char_types( self, password_chars: list[str], diff --git a/src/tests/test_archive_from_retrieve.py b/src/tests/test_archive_from_retrieve.py index 65844baa..b4b3a761 100644 --- a/src/tests/test_archive_from_retrieve.py +++ b/src/tests/test_archive_from_retrieve.py @@ -15,7 +15,7 @@ from seedpass.core.config_manager import ConfigManager class FakePasswordGenerator: - def generate_password(self, length: int, index: int) -> str: # noqa: D401 + def generate_password(self, length: int, index: int, gen_version: int = 1) -> str: # noqa: D401 return "pw" diff --git a/src/tests/test_custom_fields_display.py b/src/tests/test_custom_fields_display.py index 0a95084a..cb07713b 100644 --- a/src/tests/test_custom_fields_display.py +++ b/src/tests/test_custom_fields_display.py @@ -7,7 +7,7 @@ sys.path.append(str(Path(__file__).resolve().parents[1])) def test_retrieve_entry_shows_custom_fields(monkeypatch, capsys, password_manager): pm = password_manager - pm.password_generator = SimpleNamespace(generate_password=lambda l, i: "pw") + pm.password_generator = SimpleNamespace(generate_password=lambda l, i, gen_version=1: "pw") pm.nostr_client = SimpleNamespace() pm.entry_manager.add_entry( diff --git a/src/tests/test_entropy_integrity.py b/src/tests/test_entropy_integrity.py new file mode 100644 index 00000000..9c0b6d53 --- /dev/null +++ b/src/tests/test_entropy_integrity.py @@ -0,0 +1,425 @@ +"""Entropy and RNG integrity — the compatibility contract for derived secrets. + +Covers audit item M4 from ``docs/entropy_audit_2026-07-31.md``. + +Two distinct jobs live here: + +1. **Frozen v1 password vectors.** SeedPass never stores a password. Every entry + is re-derived on demand from ``(seed, index, length, policy)``, and entry + records carry no algorithm version. Any change to ``generate_password`` + therefore changes every existing password in every vault, unrecoverably. + These vectors are what live vaults re-derive today. + + **A failure here is never "update the expected value".** It means the change + being attempted is vault-breaking. Either revert it, or land it behind a new + ``gen_version`` while leaving v1 byte-identical -- see M1/M2/M3 in the audit. + + Regenerate these ONLY when introducing a new generation version, and then + only by appending a new ``V2_VECTORS`` table; the v1 table below is immutable. + +2. **Fail-closed RNG behaviour.** Nothing else in the suite asserts what happens + when secure randomness is unavailable. A deterministic stream passes every + self-consistency test we have -- that is precisely how a predictable RNG + survives a health check that only asks "does the output look random?". +""" + +import base64 +import collections +import os +import string +import sys +from pathlib import Path + +import pytest + +sys.path.append(str(Path(__file__).resolve().parents[1])) + +from bip_utils import Bip39SeedGenerator + +from local_bip85.bip85 import BIP85 +from seedpass.core.encryption import EncryptionManager +from seedpass.core.password_generation import ( + CURRENT_PASSWORD_GEN_VERSION, + PasswordGenerator, + PasswordPolicy, +) + +pytestmark = pytest.mark.determinism + + +# BIP-39 canonical all-"abandon" test vector. Public, never used for real funds. +TEST_MNEMONIC = ( + "abandon abandon abandon abandon abandon abandon " + "abandon abandon abandon abandon abandon about" +) + + +class _SeedDeriver: + """Mirrors EncryptionManager.derive_seed_from_mnemonic without a vault.""" + + def derive_seed_from_mnemonic(self, mnemonic, passphrase=""): + return Bip39SeedGenerator(mnemonic).Generate(passphrase) + + +POLICIES = { + "default": PasswordPolicy(), + "exclude_ambiguous": PasswordPolicy(exclude_ambiguous=True), + "safe_special": PasswordPolicy(special_mode="safe"), + "no_special": PasswordPolicy(include_special_chars=False), + "custom_special": PasswordPolicy(allowed_special_chars="!@#"), + "high_minima": PasswordPolicy( + min_uppercase=5, min_lowercase=5, min_digits=5, min_special=5 + ), +} + +# (policy_name, length, index, expected_password) +# +# Lengths 8 and 128 are MIN/MAX_PASSWORD_LENGTH; 16 is the default. 33, 40, 64 +# and 128 all cross the 32-byte DeterministicStream wrap boundary, so they pin +# the behaviour audit item M3 proposes to change. +V1_VECTORS = [ + ("default", 8, 0, "d]C}13hI"), + ("default", 16, 0, "Cq5kO%t>0O3$0r$Q"), + ("default", 16, 1, '221x&^bX1qB"GDm~'), + ("default", 16, 7, "E1v%%o&3/npC0LO5"), + ("default", 16, 4095, "e[5D^q62SEmJ2l%>"), + ("default", 20, 3, "92mT1~D3#Wxd"), + ("default", 32, 2, "y9A2=&CKK@[Wgg>902Aqu)4JuG9>c'j2"), + ("default", 33, 2, "0g&L4aAj\\;g2o62:V4,r9k%XV]Jz-DZq5"), + ( + "default", + 64, + 5, + "SbtiQwfLDa6t[911Rq1]9G1?7M54(?2_M8h=o,(e]V@17Vz)FI@C1n_v-VaMcA5$", + ), + ( + "default", + 128, + 11, + "%3OBz\\zbo?=ads|8y`LrG^FdWE572Cfb:TB939%'0HUn340pB&\\?7M?EN5O1*P.5" + "GBE7.=h4"), + ("exclude_ambiguous", 40, 2, 'wue$:"}JY2.9AQx\\RKn2:62,k9Z8<67j3fPYXzv@'), + ("safe_special", 16, 0, "Cq5kO_t-0O3-0r?Q"), + ("safe_special", 40, 2, "na_4qOEbr-s1L2nZ7$*6Gj4A@+DQ5f@49X+V0?y#"), + ("no_special", 16, 0, "03RrqOTi4NC1hT0r"), + ("no_special", 40, 2, "6gGz4Aru62y4is90OzDq4QX57XZqgGVLjV25g9hY"), + ("custom_special", 16, 0, "3q1O#z@4@rCrH0T#"), + ("high_minima", 24, 0, "Cc+r=_R3hrI01i0q#C(O*T01"), +] + + +def _generator(policy: PasswordPolicy) -> PasswordGenerator: + seed = Bip39SeedGenerator(TEST_MNEMONIC).Generate() + return PasswordGenerator( + _SeedDeriver(), TEST_MNEMONIC, BIP85(seed), policy=policy + ) + + +# -------------------------------------------------------------------------- +# 1. Frozen v1 derivation +# -------------------------------------------------------------------------- + + +@pytest.mark.parametrize("policy_name,length,index,expected", V1_VECTORS) +def test_v1_password_vectors_are_frozen(policy_name, length, index, expected): + """Live vaults re-derive these exact strings. Do not update on failure. + + See this module's docstring before changing anything here. + """ + pg = _generator(POLICIES[policy_name]) + assert pg.generate_password(length=length, index=index) == expected, ( + f"v1 derivation changed for policy={policy_name} length={length} " + f"index={index}. This breaks every existing vault entry. Revert the " + f"change, or land it behind a new gen_version leaving v1 untouched." + ) + + +def test_v1_vectors_cover_the_stream_wrap_boundary(): + """Guards the guard: M3's fix must be caught by the table above. + + DeterministicStream wraps a 32-byte key. If every vector were <= 32 + characters, changing the wrap behaviour would leave this file green while + silently altering longer passwords. + """ + assert any(length > 32 for _, length, _, _ in V1_VECTORS) + + +def test_v1_vectors_cover_every_policy(): + assert {name for name, _, _, _ in V1_VECTORS} == set(POLICIES) + + +def test_derivation_is_stable_across_generator_instances(): + """A fresh generator over the same seed must reproduce the same password.""" + first = _generator(POLICIES["default"]).generate_password(length=24, index=9) + second = _generator(POLICIES["default"]).generate_password(length=24, index=9) + assert first == second + + +# -------------------------------------------------------------------------- +# 2. Fail-closed RNG behaviour +# -------------------------------------------------------------------------- + + +def test_seed_generation_aborts_when_urandom_fails(monkeypatch): + """No secure entropy must mean no seed -- never a substituted value.""" + from seedpass.core import manager as manager_module + + def _explode(_n): + raise OSError("entropy pool unavailable") + + monkeypatch.setattr(manager_module.os, "urandom", _explode) + + with pytest.raises(Exception) as excinfo: + manager_module.PasswordManager.generate_bip85_seed(object()) + + # The failure must propagate. A returned mnemonic here would mean the + # generator fell back to something -- the exact COLDCARD defect class. + assert "entropy pool unavailable" in str(excinfo.value) or isinstance( + excinfo.value.__cause__, OSError + ) + + +def test_seed_generation_aborts_on_short_read(monkeypatch): + """A partial entropy read must not be silently accepted as a full seed.""" + from seedpass.core import manager as manager_module + + monkeypatch.setattr(manager_module.os, "urandom", lambda n: b"\x2b" * 4) + + with pytest.raises(Exception): + manager_module.PasswordManager.generate_bip85_seed(object()) + + +def test_all_zero_entropy_is_not_currently_detected(monkeypatch): + """Recorded gap, not an endorsement. + + If os.urandom returned all zeros, SeedPass would mint a valid-looking + mnemonic from it. There is no health check on the entropy source. This test + pins the CURRENT behaviour so that adding such a check is a deliberate, + visible change rather than an accident -- and so the gap cannot be forgotten. + + Tracked as part of M4 in docs/entropy_audit_2026-07-31.md. Realistically low + risk: on Linux os.urandom is getrandom(2), which does not return zeros post + boot. The threat this guards is a future code change, not a kernel failure. + """ + from seedpass.core import manager as manager_module + + monkeypatch.setattr(manager_module.os, "urandom", lambda n: b"\x00" * n) + + mnemonic = manager_module.PasswordManager.generate_bip85_seed(object()) + assert len(mnemonic.split()) == 12 # currently accepted; see docstring + + +# -------------------------------------------------------------------------- +# 3. Version 2 properties (audit items M1, M2, M3) +# -------------------------------------------------------------------------- + + +def _v2(length, index, policy=None): + return _generator(policy or POLICIES["default"]).generate_password( + length=length, index=index, gen_version=CURRENT_PASSWORD_GEN_VERSION + ) + + +def _classes(pw): + return ( + sum(c in string.ascii_uppercase for c in pw), + sum(c in string.ascii_lowercase for c in pw), + sum(c in string.digits for c in pw), + sum(not c.isalnum() for c in pw), + ) + + +def test_v2_does_not_pin_the_class_composition(): + """The core of M1: v1 produced exactly one composition, always. + + v1 forced length/4 of each class into every password, making any SeedPass + password identifiable from its composition alone. v2 must vary. + """ + pg = _generator(POLICIES["default"]) + v1 = { + _classes(pg.generate_password(length=16, index=i, gen_version=1)) + for i in range(120) + } + v2 = { + _classes( + pg.generate_password( + length=16, index=i, gen_version=CURRENT_PASSWORD_GEN_VERSION + ) + ) + for i in range(120) + } + assert len(v1) == 1, "v1 is expected to be degenerate; that is the finding" + assert len(v2) > 20, f"v2 composition still too rigid: {len(v2)} distinct" + + +def test_v2_still_satisfies_the_policy_minima(): + for length in (8, 16, 33, 64): + for index in range(25): + upper, lower, digits, special = _classes(_v2(length, index)) + assert upper >= 2 and lower >= 2 and digits >= 2 and special >= 2 + + +def test_v2_respects_non_default_policies(): + for name in ("exclude_ambiguous", "safe_special", "no_special", "custom_special"): + policy = POLICIES[name] + for index in range(10): + pw = _v2(24, index, policy) + assert len(pw) == 24 + if name == "no_special": + assert all(c.isalnum() for c in pw) + if name == "custom_special": + assert all(c.isalnum() or c in "!@#" for c in pw) + if name == "exclude_ambiguous": + assert not any(c in "O0Il1" for c in pw) + + +def test_v2_rejects_a_policy_that_cannot_fit(): + """sum(minima) > length is impossible; say so instead of silently failing.""" + policy = PasswordPolicy( + min_uppercase=5, min_lowercase=5, min_digits=5, min_special=5 + ) + with pytest.raises(ValueError, match="requires at least"): + _v2(8, 0, policy) + + +def test_v2_has_no_modulo_bias(): + """M2: chi-square over the 94-char alphabet. df=93, p<0.001 crit ~147.""" + pws = [_v2(64, i) for i in range(180)] + counts = collections.Counter(c for pw in pws for c in pw) + alphabet = string.ascii_letters + string.digits + string.punctuation + n = sum(counts.values()) + expected = n / len(alphabet) + chi2 = sum((counts.get(ch, 0) - expected) ** 2 / expected for ch in alphabet) + assert chi2 < 147, f"character distribution looks biased (chi2={chi2:.1f})" + + +def test_v2_stream_does_not_repeat_at_the_32_byte_boundary(): + """M3: v1 cycled a 32-byte key, so position i and i+32 shared a source byte.""" + pws = [_v2(128, i) for i in range(80)] + matches = sum(1 for pw in pws for i in range(len(pw) - 32) if pw[i] == pw[i + 32]) + total = sum(len(pw) - 32 for pw in pws) + rate = matches / total + chance = 1 / 94 + # Generous band: this catches a repeating pad (which would spike the rate), + # not small sampling noise. + assert rate < chance * 2.5, f"lag-32 repeat rate {rate:.5f} vs chance {chance:.5f}" + + +def test_v2_is_deterministic_across_instances(): + assert _v2(24, 9) == _v2(24, 9) + + +def test_v2_differs_from_v1(): + pg = _generator(POLICIES["default"]) + assert pg.generate_password(length=24, index=9, gen_version=1) != _v2(24, 9) + + +def test_unknown_generation_version_is_rejected(): + """Never guess which algorithm produced a password.""" + pg = _generator(POLICIES["default"]) + for bad in (0, 3, 99): + with pytest.raises(ValueError, match="Unknown password generation version"): + pg.generate_password(length=16, index=0, gen_version=bad) + + +def test_default_gen_version_is_legacy(): + """Any caller that does not opt in must keep deriving v1.""" + pg = _generator(POLICIES["default"]) + assert pg.generate_password(length=16, index=0) == pg.generate_password( + length=16, index=0, gen_version=1 + ) + + +def test_new_entries_are_stamped_with_the_current_version(): + from seedpass.core.password_generation import CURRENT_PASSWORD_GEN_VERSION as cur + + assert cur == 2 + + +# -------------------------------------------------------------------------- +# 4. Entry -> version plumbing (the part that protects existing vaults) +# -------------------------------------------------------------------------- + + +def _manager_with(pg): + from seedpass.core.manager import PasswordManager + + pm = PasswordManager.__new__(PasswordManager) + pm.password_generator = pg + return pm + + +def test_entry_without_gen_version_still_derives_v1(): + """The single most important behaviour: existing vaults must not change. + + Entries written before versioning carry no field. They must keep deriving + through v1 forever. + """ + from seedpass.core.manager import PasswordManager + + pg = _generator(POLICIES["default"]) + pm = _manager_with(pg) + got = PasswordManager._generate_password_for_entry(pm, {"length": 16}, 3) + assert got == pg.generate_password(16, 3, gen_version=1) + + +def test_entry_with_gen_version_2_derives_v2(): + from seedpass.core.manager import PasswordManager + + pg = _generator(POLICIES["default"]) + pm = _manager_with(pg) + entry = {"length": 16, "gen_version": CURRENT_PASSWORD_GEN_VERSION} + got = PasswordManager._generate_password_for_entry(pm, entry, 3) + assert got == pg.generate_password( + 16, 3, gen_version=CURRENT_PASSWORD_GEN_VERSION + ) + assert got != pg.generate_password(16, 3, gen_version=1) + + +def test_policy_override_still_applies_on_a_versioned_entry(): + from seedpass.core.manager import PasswordManager + + pg = _generator(POLICIES["default"]) + pm = _manager_with(pg) + entry = { + "length": 16, + "gen_version": CURRENT_PASSWORD_GEN_VERSION, + "policy": {"include_special_chars": False}, + } + got = PasswordManager._generate_password_for_entry(pm, entry, 3) + assert got.isalnum() + assert pg.policy == PasswordPolicy(), "base policy must be restored" + + +def test_unreadable_entry_gen_version_fails_loudly(): + """Never guess which algorithm produced an entry's password.""" + from seedpass.core.manager import PasswordManager + + pm = _manager_with(_generator(POLICIES["default"])) + with pytest.raises(ValueError, match="unreadable gen_version"): + PasswordManager._generate_password_for_entry( + pm, {"length": 16, "gen_version": "??"}, 3 + ) + + +def test_encryption_nonce_is_fresh_per_call(tmp_path): + """AES-GCM nonce reuse under one key is catastrophic, not gradual.""" + key = base64.urlsafe_b64encode(os.urandom(32)) + em = EncryptionManager(key, tmp_path) + + payloads = [em.encrypt_data(b"identical plaintext") for _ in range(64)] + nonces = {blob[3:15] for blob in payloads} # b"V3|" + 12-byte nonce + + assert len(nonces) == 64, "AES-GCM nonce repeated across encrypt_data calls" + assert len({bytes(b) for b in payloads}) == 64 + assert all(blob.startswith(b"V3|") for blob in payloads) + assert em.decrypt_data(payloads[0]) == b"identical plaintext" + + +def test_totp_secret_is_fresh_per_call(): + from seedpass.core.totp import random_totp_secret + + secrets_seen = {random_totp_secret() for _ in range(64)} + assert len(secrets_seen) == 64 diff --git a/src/tests/test_manager_add_password.py b/src/tests/test_manager_add_password.py index 52f34053..4fc950db 100644 --- a/src/tests/test_manager_add_password.py +++ b/src/tests/test_manager_add_password.py @@ -17,7 +17,7 @@ from constants import DEFAULT_PASSWORD_LENGTH class FakePasswordGenerator: - def generate_password(self, length: int, index: int) -> str: # noqa: D401 + def generate_password(self, length: int, index: int, gen_version: int = 1) -> str: # noqa: D401 return f"pw-{index}-{length}" diff --git a/src/tests/test_manager_list_entries.py b/src/tests/test_manager_list_entries.py index 14122f46..9e63ddbc 100644 --- a/src/tests/test_manager_list_entries.py +++ b/src/tests/test_manager_list_entries.py @@ -138,7 +138,7 @@ def test_show_entry_details_by_index(monkeypatch): monkeypatch.setattr( "seedpass.core.manager.confirm_action", lambda *a, **k: False ) - pm.password_generator = SimpleNamespace(generate_password=lambda l, i: "pw123") + pm.password_generator = SimpleNamespace(generate_password=lambda l, i, gen_version=1: "pw123") monkeypatch.setattr(pm, "notify", lambda *a, **k: None) pm.show_entry_details_by_index(index) @@ -177,7 +177,7 @@ def _detail_common(monkeypatch, pm): monkeypatch.setattr("seedpass.core.manager.timed_input", lambda *a, **k: "b") monkeypatch.setattr("seedpass.core.manager.time.sleep", lambda *a, **k: None) monkeypatch.setattr(pm, "notify", lambda *a, **k: None) - pm.password_generator = SimpleNamespace(generate_password=lambda l, i: "pw123") + pm.password_generator = SimpleNamespace(generate_password=lambda l, i, gen_version=1: "pw123") called = [] monkeypatch.setattr(pm, "_entry_actions_menu", lambda *a, **k: called.append(True)) return called @@ -295,7 +295,7 @@ def test_show_entry_details_sensitive(monkeypatch, capsys, entry_type): with TemporaryDirectory() as tmpdir: tmp_path = Path(tmpdir) pm, entry_mgr = _setup_manager(tmp_path) - pm.password_generator = SimpleNamespace(generate_password=lambda l, i: "pw123") + pm.password_generator = SimpleNamespace(generate_password=lambda l, i, gen_version=1: "pw123") monkeypatch.setattr( "seedpass.core.manager.confirm_action", lambda *a, **k: True diff --git a/src/tests/test_manager_workflow.py b/src/tests/test_manager_workflow.py index 9c5606f8..cd94ac25 100644 --- a/src/tests/test_manager_workflow.py +++ b/src/tests/test_manager_workflow.py @@ -13,7 +13,7 @@ from seedpass.core.config_manager import ConfigManager class FakePasswordGenerator: - def generate_password(self, length: int, index: int) -> str: # noqa: D401 + def generate_password(self, length: int, index: int, gen_version: int = 1) -> str: # noqa: D401 return f"pw-{index}-{length}" diff --git a/src/tests/test_service_classes.py b/src/tests/test_service_classes.py index a32b549d..1f72a8d1 100644 --- a/src/tests/test_service_classes.py +++ b/src/tests/test_service_classes.py @@ -15,7 +15,7 @@ from constants import DEFAULT_PASSWORD_LENGTH class FakePasswordGenerator: - def generate_password(self, length: int, index: int) -> str: + def generate_password(self, length: int, index: int, gen_version: int = 1) -> str: return f"pw-{index}-{length}"