Offer a 24-word master seed, and put the entropy draw beyond reach of any handler

L2: generate_bip85_seed and create_profile_from_generated_seed now take
words_num, accepting 12 or 24 and rejecting anything else with a clear
message. The default stays 12 -- every existing profile uses it, 128 bits is
entirely adequate, and moving the default would change nothing about
existing vaults while surprising anyone creating a new one. 24 is offered
because the master seed protects the whole vault and was the one secret
shorter than the seeds derived beneath it, which already default to 24.

L3: fixed structurally rather than by narrowing exception types.
os.urandom(32) now sits OUTSIDE the try block, so no handler can stand
between the entropy draw and the caller -- an entropy failure is unreachable
by anything that could substitute a value. Also removes a redundant
`except Exception` whose body was identical to the `Bip85Error` branch above
it.

The structural form is stronger than an exception list, and the test says so
precisely: it asserts the OSError reaching the caller is the SAME OBJECT
os.urandom threw, which holds only if nothing caught and repackaged it.
Listing exception types would still leave a handler in the path for someone
to widen later.

Adds five tests: the identity assertion above, 12- and 24-word generation,
the default staying 12, and rejection of 0/11/18/25/-12.

Determinism gate green (89 tests, up from 84). The seed/profile regression
subset is 143 passed with one failure --
test_profile_export_import_round_trip -- confirmed against the pre-change
baseline as pre-existing.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01HtNMsKzzFTPURemX7tT19u
This commit is contained in:
thePR0M3TH3ANandClaude Opus 5 committed 2026-07-31 17:43:24 -04:00
1 parent 6ac40d2e15
commit 3192c9f306
5 files changed
+138 -17

No files matched your search

+10 -4
View File
@@ -41,8 +41,13 @@ wild.** v1 is frozen; a fix that changes derivation goes into a new version, nev
- [ ] **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.
- [x] **L2 (done)** — `generate_bip85_seed(words_num=...)` accepts 12 or 24 and rejects anything
else; threaded through `create_profile_from_generated_seed`. Default stays 12 so existing
profiles and callers are unaffected.
- [x] **L3 (done for the RNG path)** — `os.urandom(32)` moved outside the try block in
`generate_bip85_seed`, so no handler can stand between the entropy draw and the caller, and
the redundant duplicate `except Exception` is gone. A test asserts the `OSError` arrives as
the same object that was thrown, which is only true if nothing repackaged it.
- [x] **L4 (code fixes done)** — `torch/` temp filename now uses `randomBytes(8)`; the
`Math.random` value misnamed `nonce` in `relay-health.mjs` is now `probeSuffix` from
`randomBytes(6)`.
@@ -56,8 +61,9 @@ wild.** v1 is frozen; a fix that changes derivation goes into a new version, nev
`requirements.lock`. Verified with the real tool: CycloneDX 1.6, 96 components, crypto chain
recorded at exact versions.
L3 (broad `except Exception` around crypto — these re-raise, so no silent fallback) overlaps the
Robustness item further down.
The remaining broad `except Exception` handlers in `password_generation.py` wrap deterministic
derivation with no RNG in it and re-raise rather than substituting; they belong to the Robustness
item further down, not to this audit.
## Security
+35 -2
View File
@@ -320,7 +320,19 @@ anyway, to prove that.
---
### L2 — Master seed entropy accounting
### L2 — Master seed entropy accounting — **DONE 2026-07-31**
`generate_bip85_seed` and `create_profile_from_generated_seed` now take
`words_num`, accepting 12 or 24 and rejecting anything else. **The default stays
12**: every existing profile uses it, it is entirely adequate, and moving the
default would surprise without improving any existing vault. 24 is now available
for a 256-bit master seed, which removes the oddity that the secret protecting
the whole vault was shorter than the seeds derived beneath it (those default to
24).
The original note follows.
### L2 (original) — Master seed entropy accounting
`generate_bip85_seed` (`core/manager.py:1810-1812`) draws `os.urandom(32)` —
256 bits — then derives a **12-word** mnemonic, which encodes 128. The word
@@ -338,7 +350,28 @@ unaffected — this only changes what new seeds look like.
---
### L3 — Broad `except Exception` around crypto
### L3 — Broad `except Exception` around crypto — **DONE 2026-07-31** (for the RNG path)
Fixed structurally rather than by narrowing exception types. `os.urandom(32)`
now sits **outside** the try block in `generate_bip85_seed`, so no handler can
stand between the entropy draw and the caller. The redundant duplicate
`except Exception` — whose body was identical to the `Bip85Error` branch above
it — is gone.
This is stronger than listing which exceptions to catch: an entropy failure is
now unreachable by any handler that could substitute a value, and
`test_entropy_draw_sits_outside_the_try_block` asserts the raised `OSError`
arrives as the *same object* that `os.urandom` threw, which is only true if
nothing caught and repackaged it.
The remaining broad handlers in `password_generation.py` (lines 280, 485) wrap
deterministic derivation with no RNG in it, and they log and re-raise rather
than substituting. They are covered by the general "replace broad
`except Exception`" item in `TODO.md` rather than being an entropy concern.
The original note follows.
### L3 (original) — Broad `except Exception` around crypto
`password_generation.py:280`, `:485`, `core/manager.py:1817`. All of these
log-and-re-raise, so **there is no silent fallback** — the operation aborts, as
+11
View File
@@ -49,6 +49,17 @@ MAX_PASSWORD_LENGTH = 128 # Maximum allowed password length
# Characters considered safe for passwords when limiting punctuation
SAFE_SPECIAL_CHARS = "!@#$%^*-_+=?"
# Master seed word count.
# -----------------------------------
# 12 words = 128 bits, 24 words = 256 bits. The default stays 12: it is
# entirely adequate and every existing profile uses it, so changing the default
# would alter nothing about existing vaults but would surprise. 24 is offered
# because the master seed protects the whole vault and was previously the one
# secret shorter than the seeds derived beneath it, which default to 24
# (entropy audit L2, docs/entropy_audit_2026-07-31.md).
DEFAULT_SEED_WORD_COUNT = 12
SUPPORTED_SEED_WORD_COUNTS = frozenset({12, 24})
# Timeout in seconds before the vault locks due to inactivity
INACTIVITY_TIMEOUT = 15 * 60 # 15 minutes
+32 -11
View File
@@ -89,6 +89,8 @@ from constants import (
DEFAULT_PASSWORD_LENGTH,
INACTIVITY_TIMEOUT,
DEFAULT_SEED_BACKUP_FILENAME,
DEFAULT_SEED_WORD_COUNT,
SUPPORTED_SEED_WORD_COUNTS,
NOTIFICATION_DURATION,
initialize_app,
)
@@ -1802,35 +1804,54 @@ class PasswordManager:
logging.error(f"Error validating BIP-85 seed: {e}")
return False
def generate_bip85_seed(self) -> str:
def generate_bip85_seed(self, words_num: int = DEFAULT_SEED_WORD_COUNT) -> str:
"""
Generates a new BIP-85 seed phrase.
Parameters:
words_num (int): 12 (128-bit, default) or 24 (256-bit). The default
stays 12 so existing callers are unaffected; 24 is offered
because the master seed protects the whole vault and was
previously shorter than the seeds derived beneath it, which
default to 24 (entropy audit L2).
Returns:
str: The generated 12-word mnemonic seed phrase.
str: The generated mnemonic seed phrase.
"""
if words_num not in SUPPORTED_SEED_WORD_COUNTS:
raise SeedPassError(
f"Seed word count must be one of {sorted(SUPPORTED_SEED_WORD_COUNTS)}, "
f"got {words_num!r}."
)
# Deliberately OUTSIDE the try block (entropy audit L3). Nothing below
# may catch an entropy failure and substitute a value -- that is the
# COLDCARD defect class. If os.urandom raises, it propagates unchanged
# and the operation aborts, which is the only acceptable outcome.
master_seed = os.urandom(32)
try:
master_seed = os.urandom(32) # Generate a random 32-byte seed
bip85 = BIP85(master_seed)
mnemonic = bip85.derive_mnemonic(index=0, words_num=12)
return mnemonic
return bip85.derive_mnemonic(index=0, words_num=words_num)
except Bip85Error as e:
logging.error(f"Failed to generate BIP-85 seed: {e}", exc_info=True)
print(colored(f"Error: Failed to generate BIP-85 seed: {e}", "red"))
raise SeedPassError(f"Failed to generate BIP-85 seed: {e}") from e
except Exception as e:
logging.error(f"Failed to generate BIP-85 seed: {e}", exc_info=True)
print(colored(f"Error: Failed to generate BIP-85 seed: {e}", "red"))
raise SeedPassError(f"Failed to generate BIP-85 seed: {e}") from e
def create_profile_from_generated_seed(
self,
*,
password: str,
seed: Optional[str] = None,
words_num: int = DEFAULT_SEED_WORD_COUNT,
) -> tuple[str, str]:
"""Create a new profile from a generated seed and return ``(fingerprint, seed)``."""
new_seed = seed or self.generate_bip85_seed()
"""Create a new profile from a generated seed and return ``(fingerprint, seed)``.
``words_num`` selects the master seed length (12 or 24) and is ignored
when ``seed`` is supplied, since that phrase already fixes its own
length.
"""
new_seed = seed or self.generate_bip85_seed(words_num=words_num)
fingerprint = self._finalize_existing_seed(new_seed, password=password)
if not fingerprint:
raise SeedPassError("Failed to create profile from generated seed.")
+50
View File
@@ -191,6 +191,56 @@ def test_seed_generation_aborts_on_short_read(monkeypatch):
manager_module.PasswordManager.generate_bip85_seed(object())
def test_entropy_draw_sits_outside_the_try_block(monkeypatch):
"""L3: no except clause may stand between os.urandom and the caller.
The generator's exception handling wraps BIP-85 failures in SeedPassError,
which is fine -- but an entropy failure must never be reachable by any
handler that could substitute a value. Keeping the draw outside the try is
the structural version of that guarantee, stronger than narrowing which
exception types get caught.
"""
from seedpass.core import manager as manager_module
sentinel = OSError("entropy pool unavailable")
def _explode(_n):
raise sentinel
monkeypatch.setattr(manager_module.os, "urandom", _explode)
with pytest.raises(OSError) as excinfo:
manager_module.PasswordManager.generate_bip85_seed(object())
# Propagates as itself, not repackaged -- proof nothing caught it.
assert excinfo.value is sentinel
@pytest.mark.parametrize("words_num,expected", [(12, 12), (24, 24)])
def test_master_seed_word_count_is_selectable(words_num, expected):
"""L2: the master seed can now be 256-bit, not only 128-bit."""
from seedpass.core.manager import PasswordManager
mnemonic = PasswordManager.generate_bip85_seed(object(), words_num=words_num)
assert len(mnemonic.split()) == expected
def test_master_seed_default_is_unchanged():
"""Existing profiles are 12-word; the default must not move under them."""
from seedpass.core.manager import PasswordManager
assert len(PasswordManager.generate_bip85_seed(object()).split()) == 12
def test_unsupported_word_count_is_rejected():
from seedpass.core.errors import SeedPassError
from seedpass.core.manager import PasswordManager
for bad in (0, 11, 18, 25, -12):
with pytest.raises(SeedPassError, match="Seed word count"):
PasswordManager.generate_bip85_seed(object(), words_num=bad)
def test_all_zero_entropy_is_not_currently_detected(monkeypatch):
"""Recorded gap, not an endorsement.