From 6567dba913a63d46105db967281d987063ea3287 Mon Sep 17 00:00:00 2001 From: Matt Norris Date: Tue, 6 Oct 2026 15:43:36 -0400 Subject: [PATCH] fix(ess-passwords): count entropy without replacement Words are drawn without replacement, so entropy is now the sum of log2(N - i) per draw. A word count above the filtered wordlist raises PasswordPolicyError, and a max_length too small for the word count raises the new PasswordLengthError. Document that entropy is an upper bound under max_length and that --json omits the stderr note. --- packages/python/ess-passwords/README.md | 6 ++- .../src/ess_passwords/__init__.py | 3 +- .../src/ess_passwords/__main__.py | 2 + .../src/ess_passwords/exceptions.py | 4 ++ .../src/ess_passwords/password.py | 50 +++++++++++++++---- .../src/ess_passwords/test_main.py | 1 + .../src/ess_passwords/test_password.py | 33 +++++++++++- 7 files changed, 84 insertions(+), 15 deletions(-) diff --git a/packages/python/ess-passwords/README.md b/packages/python/ess-passwords/README.md index 717ddbd..cc4525b 100644 --- a/packages/python/ess-passwords/README.md +++ b/packages/python/ess-passwords/README.md @@ -51,7 +51,7 @@ no `--dry-run` and no confirmation prompt. | `--capitalize-at` | random | Position of the upper-cased word, counting from 0 | | `--digits` | `0` | Digits to append after the last word | | `--count` / `-n` | `1` | Number of passwords to print | -| `--json` | off | Emit JSON: an object, or an array when `--count` is above 1 | +| `--json` | off | Emit JSON: an object, or an array when `--count` is above 1. Entropy is in the payload; the stderr note is omitted | ## API @@ -81,7 +81,9 @@ Frozen dataclass with `value`, `entropy_bits`, and `wordlist_size`. `entropy_bits` counts the random word and digit choices only. Which word is capitalised is not counted: it is worth just `log2(word_count)` against a scheme an attacker already knows, and understating strength is -the safe direction. +the safe direction. When `max_length` is set, the generator rejects +over-long candidates, so the reported figure is an upper bound on the +accepted distribution. ### `load_wordlist()` and `excluded_words()` diff --git a/packages/python/ess-passwords/src/ess_passwords/__init__.py b/packages/python/ess-passwords/src/ess_passwords/__init__.py index 4a42bb4..8cbd527 100644 --- a/packages/python/ess-passwords/src/ess_passwords/__init__.py +++ b/packages/python/ess-passwords/src/ess_passwords/__init__.py @@ -1,6 +1,6 @@ """Readable password generation from a filtered EFF wordlist.""" -from .exceptions import EssPasswordsError, PasswordPolicyError +from .exceptions import EssPasswordsError, PasswordLengthError, PasswordPolicyError from .password import ( DEFAULT_SEPARATOR, DEFAULT_WORD_COUNT, @@ -15,6 +15,7 @@ "DEFAULT_WORD_COUNT", "EssPasswordsError", "GeneratedPassword", + "PasswordLengthError", "PasswordOptions", "PasswordPolicyError", "excluded_words", diff --git a/packages/python/ess-passwords/src/ess_passwords/__main__.py b/packages/python/ess-passwords/src/ess_passwords/__main__.py index 063514c..3b7cf0f 100644 --- a/packages/python/ess-passwords/src/ess_passwords/__main__.py +++ b/packages/python/ess-passwords/src/ess_passwords/__main__.py @@ -91,6 +91,8 @@ def generate_command( # noqa: PLR0913 -- Click option surface; bundling hurts r The password goes to stdout and its entropy to stderr, so the output pipes straight into a secret store without a stray note tagging along. + `--json` puts entropy in the JSON object instead and does not write + the stderr note. """ options = PasswordOptions( word_count=words, diff --git a/packages/python/ess-passwords/src/ess_passwords/exceptions.py b/packages/python/ess-passwords/src/ess_passwords/exceptions.py index 0221a35..c3970f8 100644 --- a/packages/python/ess-passwords/src/ess_passwords/exceptions.py +++ b/packages/python/ess-passwords/src/ess_passwords/exceptions.py @@ -9,3 +9,7 @@ class EssPasswordsError(Exception): class PasswordPolicyError(EssPasswordsError): """The requested password options cannot produce a valid password.""" + + +class PasswordLengthError(PasswordPolicyError): + """`max_length` is too small for the requested word count.""" diff --git a/packages/python/ess-passwords/src/ess_passwords/password.py b/packages/python/ess-passwords/src/ess_passwords/password.py index 7d82660..5a15908 100644 --- a/packages/python/ess-passwords/src/ess_passwords/password.py +++ b/packages/python/ess-passwords/src/ess_passwords/password.py @@ -13,7 +13,7 @@ from xkcdpass import xkcd_password -from .exceptions import PasswordPolicyError +from .exceptions import PasswordLengthError, PasswordPolicyError from .wordlist import load_wordlist DEFAULT_WORD_COUNT = 5 @@ -55,9 +55,14 @@ class GeneratedPassword: Attributes: value: The password itself. entropy_bits: Entropy contributed by the random word and digit - choices. Which word is capitalised is not counted: it is worth - only log2(word_count) against a scheme an attacker already - knows, and understating strength is the safe direction. + choices. Words are drawn in order without replacement, so this + is the sum of log2 of the words still left at each draw, plus + log2(10) per digit. Which word is capitalised is not counted: + it is worth only log2(word_count) against a scheme an attacker + already knows, and understating strength is the safe direction. + When `max_length` is set, rejection sampling discards long + candidates, so this figure is an upper bound on the accepted + distribution rather than its exact entropy. wordlist_size: Number of candidate words after length filtering. """ @@ -95,7 +100,7 @@ def _require_valid(options: PasswordOptions) -> None: options ): msg = _impossible_length_message(options) - raise PasswordPolicyError(msg) + raise PasswordLengthError(msg) def _compose(words: list[str], options: PasswordOptions) -> str: @@ -149,19 +154,29 @@ def generate_password(options: PasswordOptions | None = None) -> GeneratedPasswo Returns: The password plus the entropy of the scheme that produced it. + With `max_length` set, `entropy_bits` is an upper bound: long + draws are rejected, so not every combination is equally reachable. Raises: PasswordPolicyError: The options cannot produce a password -- - `word_count` or `max_length` below 1, `capitalized_index` - outside the words, or a `max_length` the word count cannot fit. + `word_count` or `max_length` below 1, `word_count` larger + than the filtered wordlist, `capitalized_index` outside the + words, or a `max_length` the word count cannot fit. """ opts = options or PasswordOptions() _require_valid(opts) wordlist = load_wordlist() - entropy_bits = opts.word_count * math.log2(len(wordlist)) - if opts.digit_count: - entropy_bits += opts.digit_count * math.log2(len(_DIGITS)) + # xkcdpass samples without replacement, so a count past the filtered + # list raises ValueError. That is a policy failure, not a traceback. + if opts.word_count > len(wordlist): + msg = ( + f"word_count must be at most {len(wordlist)}, the size of the " + f"filtered wordlist, got {opts.word_count}." + ) + raise PasswordPolicyError(msg) + + entropy_bits = _entropy_bits(opts.word_count, len(wordlist), opts.digit_count) for _ in range(_MAX_GENERATION_ATTEMPTS): words = xkcd_password.generate_xkcdpassword( @@ -179,4 +194,17 @@ def generate_password(options: PasswordOptions | None = None) -> GeneratedPasswo ) msg = _impossible_length_message(opts) - raise PasswordPolicyError(msg) + raise PasswordLengthError(msg) + + +def _entropy_bits(word_count: int, wordlist_size: int, digit_count: int) -> float: + """Entropy of an ordered draw without replacement, plus the digits. + + Each word is chosen from the words still left, so the word contribution + is sum(log2(N - i)) for each draw. Digits are independent draws from 0-9. + + Callers that also set `max_length` reject over-long candidates, so this + value is an upper bound on the accepted distribution. + """ + word_bits = sum(math.log2(wordlist_size - index) for index in range(word_count)) + return word_bits + digit_count * math.log2(len(_DIGITS)) diff --git a/packages/python/ess-passwords/src/ess_passwords/test_main.py b/packages/python/ess-passwords/src/ess_passwords/test_main.py index 5f153c0..633bcfb 100644 --- a/packages/python/ess-passwords/src/ess_passwords/test_main.py +++ b/packages/python/ess-passwords/src/ess_passwords/test_main.py @@ -38,6 +38,7 @@ def test_json_carries_the_password_and_its_strength() -> None: assert result.exit_code == 0 assert payload.keys() == {"password", "entropy_bits", "wordlist_size"} assert payload["entropy_bits"] > 50 + assert "bits of entropy" not in result.stderr def test_json_with_count_is_a_list() -> None: diff --git a/packages/python/ess-passwords/src/ess_passwords/test_password.py b/packages/python/ess-passwords/src/ess_passwords/test_password.py index 698297a..1571dc3 100644 --- a/packages/python/ess-passwords/src/ess_passwords/test_password.py +++ b/packages/python/ess-passwords/src/ess_passwords/test_password.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math import re import pytest @@ -94,7 +95,7 @@ def test_digits_raise_entropy() -> None: with_digits = generate_password(PasswordOptions(digit_count=4)).entropy_bits # Four decimal digits are worth 4 * log2(10), a little over 13 bits. - assert with_digits > without + 13 + assert with_digits == pytest.approx(without + 4 * math.log2(10)) def test_negative_digit_count_raises() -> None: @@ -110,12 +111,29 @@ def test_passwords_are_not_repeated() -> None: def test_reports_entropy_from_actual_wordlist() -> None: result = generate_password() + expected = sum( + math.log2(result.wordlist_size - index) for index in range(DEFAULT_WORD_COUNT) + ) assert result.wordlist_size > 1000 # Five words from a wordlist of a few thousand clears 50 bits easily. + assert result.entropy_bits == pytest.approx(expected) assert result.entropy_bits > 50 +def test_entropy_counts_sampling_without_replacement( + monkeypatch: pytest.MonkeyPatch, +) -> None: + words = ["alpha", "bravo", "charlie"] + monkeypatch.setattr("ess_passwords.password.load_wordlist", lambda: words) + + result = generate_password(PasswordOptions(word_count=2, digit_count=1)) + expected = math.log2(3) + math.log2(2) + math.log2(10) + + assert result.entropy_bits == pytest.approx(expected) + assert result.entropy_bits < 2 * math.log2(3) + math.log2(10) + + def test_max_length_is_respected() -> None: result = generate_password(PasswordOptions(word_count=3, max_length=30)) @@ -141,6 +159,19 @@ def unreachable(*args: object, **kwargs: object) -> str: generate_password(PasswordOptions(word_count=8, max_length=20)) +def test_word_count_above_the_wordlist_raises( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Sampling without replacement cannot draw more words than the list. + monkeypatch.setattr( + "ess_passwords.password.load_wordlist", + lambda: ["alpha", "bravo", "charlie"], + ) + + with pytest.raises(PasswordPolicyError, match="at most 3"): + generate_password(PasswordOptions(word_count=4)) + + def test_zero_words_raises() -> None: with pytest.raises(PasswordPolicyError, match="at least 1"): generate_password(PasswordOptions(word_count=0))