escrow: a recovery code can no longer contain a hyphenated word (v0.93.0)

The EFF large list has exactly 4 entries containing the join separator (drop-down, felt-tip, t-shirt, yo-yo). Drawing one made a code read as 11 words instead of 10 - ambiguous to transcribe in precisely the situation R exists for. Filter them at init; the draw space goes 7776 -> 7772 and the 10-word code goes 129.248 -> 129.241 bits, still well over the 128 floor. Generation-only: already-issued codes stay valid, R is verified as a whole passphrase and never re-split. Also fixes the ~1/5 flake this same defect caused: the test counted words by splitting the joined string. It now counts what the generator drew and asserts segmentation separately, plus a deterministic red-proof fixture.
2026-07-21 14:46:51 +02:00
parent 935904fa4e
commit a452dc3314
5 changed files with 232 additions and 23 deletions
@@ -11,8 +11,16 @@ import (
 )

 func TestWordlistLoaded(t *testing.T) {
-	if WordlistSize() != 7776 {
-		t.Fatalf("EFF large wordlist should be 7776 words, got %d", WordlistSize())
+	// The EFF large list is 7776 entries; joinSafe removes the 4 that contain RecoveryCodeSep
+	// (drop-down, felt-tip, t-shirt, yo-yo), leaving 7772 as the effective draw space.
+	if got := WordlistSize(); got != 7772 {
+		t.Fatalf("effective wordlist should be 7772 words (7776 EFF - 4 hyphenated), got %d", got)
+	}
+	if got := WordlistFilteredOut(); got != 4 {
+		t.Fatalf("joinSafe should have removed exactly 4 hyphenated entries, removed %d", got)
+	}
+	if got := WordlistSize() + WordlistFilteredOut(); got != 7776 {
+		t.Fatalf("filtered + removed should reconstitute the 7776-word EFF list, got %d", got)
 	}
 }

@@ -25,19 +33,27 @@ func TestGenerateRecoveryCode_EntropyAndFormat(t *testing.T) {
 		inList[w] = true
 	}
 	for i := 0; i < 50; i++ {
-		r, err := GenerateRecoveryCode()
+		// Count words by GENERATION count, not by re-splitting the joined string: the two agree
+		// only because joinSafe holds, and conflating them is what made this test flake ~1/5.
+		words, err := generateWords(wordlist)
 		if err != nil {
-			t.Fatalf("GenerateRecoveryCode: %v", err)
+			t.Fatalf("generateWords: %v", err)
 		}
-		words := strings.Split(r, "-")
 		if len(words) != RecoveryCodeWords {
-			t.Fatalf("recovery code must be %d words, got %d (%q)", RecoveryCodeWords, len(words), r)
+			t.Fatalf("generator must draw %d words, drew %d", RecoveryCodeWords, len(words))
 		}
 		for _, w := range words {
 			if !inList[w] {
 				t.Errorf("recovery-code word %q is not from the EFF wordlist", w)
 			}
 		}
+		// Separately assert the property joinSafe buys: the joined code segments back to the same
+		// count. Never print r — it is a live-shaped secret.
+		r := strings.Join(words, RecoveryCodeSep)
+		if got := len(strings.Split(r, RecoveryCodeSep)); got != RecoveryCodeWords {
+			t.Fatalf("joined code must segment into %d words, got %d (a drawn word contained %q)",
+				RecoveryCodeWords, got, RecoveryCodeSep)
+		}
 	}
 }

@@ -29,9 +29,20 @@ import (
 //go:embed eff_large_wordlist.txt
 var wordlistRaw []byte

-// wordlist is the EFF large wordlist (7776 words, 12.92 bits/word) — the diceware standard for
-// human-transcribed passphrases. Parsed once at init.
-var wordlist = parseWordlist(wordlistRaw)
+// RecoveryCodeSep joins the words of a recovery code R. It is ALSO the reason for the
+// joinSafe filter below: a word that itself contains the separator makes the joined code
+// ambiguous to segment by eye, which is unaffordable in the one situation R exists for — a
+// customer transcribing it during a disaster. Do not change it: R is consumed as a whole
+// passphrase (see Wrap/Unwrap), so the separator is a transcription aid, not a parsed delimiter.
+const RecoveryCodeSep = "-"
+
+// wordlist is the EFF large wordlist (the diceware standard for human-transcribed passphrases),
+// minus the handful of entries that contain RecoveryCodeSep. Parsed and filtered once at init.
+// Sizes are asserted in wordlist_test.go so a wordlist swap cannot silently move the entropy floor.
+var wordlist = joinSafe(parseWordlist(wordlistRaw))
+
+// wordlistRawSize is the unfiltered parse length, kept for audit (see WordlistFilteredOut).
+var wordlistRawSize = len(parseWordlist(wordlistRaw))

 func parseWordlist(raw []byte) []string {
 	var w []string
@@ -44,28 +55,55 @@ func parseWordlist(raw []byte) []string {
 	return w
 }

-// RecoveryCodeWords is the number of words in a recovery code R. 10 words from the 7776-word EFF
-// list ≈ 129.2 bits (≥128) — the chosen entropy floor (slice7-escrow-spike-findings.md §4).
+// joinSafe drops every word containing RecoveryCodeSep, so that a generated code always segments
+// back into exactly RecoveryCodeWords words. In the EFF large list this removes exactly 4 entries
+// (drop-down, felt-tip, t-shirt, yo-yo) of 7776, costing ~0.0007 bits/word — the floor still holds
+// (asserted in the tests). Generation-time only: codes already issued remain valid, because R is
+// verified as a whole passphrase and is never re-split.
+func joinSafe(words []string) []string {
+	out := make([]string, 0, len(words))
+	for _, w := range words {
+		if strings.Contains(w, RecoveryCodeSep) {
+			continue
+		}
+		out = append(out, w)
+	}
+	return out
+}
+
+// RecoveryCodeWords is the number of words in a recovery code R. 10 words from the filtered EFF
+// list (7772 words) ≈ 129.2 bits (≥128) — the chosen entropy floor (slice7-escrow-spike-findings.md §4).
 const RecoveryCodeWords = 10

-// GenerateRecoveryCode returns a fresh recovery code R: RecoveryCodeWords words chosen uniformly
-// (crypto/rand via big.Int — no modulo bias) from the EFF large wordlist, hyphen-joined.
-//
-// SECRET: the returned string is R. Surface it to the customer exactly once; never log or persist it.
-func GenerateRecoveryCode() (string, error) {
-	if len(wordlist) < 2 {
-		return "", fmt.Errorf("escrow: wordlist not loaded (%d words)", len(wordlist))
+// generateWords draws RecoveryCodeWords words uniformly (crypto/rand via big.Int — no modulo bias)
+// from list. Split out from GenerateRecoveryCode so tests can drive an unfiltered list and prove
+// the filter is what keeps a code segmentable.
+func generateWords(list []string) ([]string, error) {
+	if len(list) < 2 {
+		return nil, fmt.Errorf("escrow: wordlist not loaded (%d words)", len(list))
 	}
-	n := big.NewInt(int64(len(wordlist)))
+	n := big.NewInt(int64(len(list)))
 	words := make([]string, RecoveryCodeWords)
 	for i := range words {
 		idx, err := rand.Int(rand.Reader, n)
 		if err != nil {
-			return "", fmt.Errorf("escrow: recovery-code rng: %w", err)
+			return nil, fmt.Errorf("escrow: recovery-code rng: %w", err)
 		}
-		words[i] = wordlist[idx.Int64()]
+		words[i] = list[idx.Int64()]
 	}
-	return strings.Join(words, "-"), nil
+	return words, nil
+}
+
+// GenerateRecoveryCode returns a fresh recovery code R: RecoveryCodeWords words chosen uniformly
+// from the filtered EFF large wordlist, joined with RecoveryCodeSep.
+//
+// SECRET: the returned string is R. Surface it to the customer exactly once; never log or persist it.
+func GenerateRecoveryCode() (string, error) {
+	words, err := generateWords(wordlist)
+	if err != nil {
+		return "", err
+	}
+	return strings.Join(words, RecoveryCodeSep), nil
 }

 // RecoveryCodeEntropyBits is the approximate entropy of a generated code, for display/audit only
@@ -77,5 +115,8 @@ func RecoveryCodeEntropyBits() float64 {
 	return float64(RecoveryCodeWords) * math.Log2(float64(len(wordlist)))
 }

-// WordlistSize is the loaded wordlist length (for audit/tests).
+// WordlistSize is the effective (filtered) wordlist length — the draw space. For audit/tests.
 func WordlistSize() int { return len(wordlist) }
+
+// WordlistFilteredOut is how many parsed entries joinSafe removed. For audit/tests.
+func WordlistFilteredOut() int { return wordlistRawSize - len(wordlist) }
@@ -0,0 +1,126 @@
+package escrow
+
+import (
+	"math"
+	"strings"
+	"testing"
+)
+
+// The four EFF large-list entries that contain RecoveryCodeSep. Named here so a wordlist swap that
+// changes the set fails loudly rather than silently re-opening the ambiguity.
+var hyphenatedEFFWords = []string{"drop-down", "felt-tip", "t-shirt", "yo-yo"}
+
+func TestJoinSafe_RemovesExactlyTheHyphenatedEFFWords(t *testing.T) {
+	raw := parseWordlist(wordlistRaw)
+	rawSet := make(map[string]bool, len(raw))
+	for _, w := range raw {
+		rawSet[w] = true
+	}
+	for _, w := range hyphenatedEFFWords {
+		if !rawSet[w] {
+			t.Fatalf("fixture drift: %q is no longer in the embedded EFF list", w)
+		}
+	}
+
+	filtered := joinSafe(raw)
+	if len(raw)-len(filtered) != len(hyphenatedEFFWords) {
+		t.Fatalf("joinSafe removed %d entries, expected exactly %d",
+			len(raw)-len(filtered), len(hyphenatedEFFWords))
+	}
+	got := make(map[string]bool, len(filtered))
+	for _, w := range filtered {
+		if strings.Contains(w, RecoveryCodeSep) {
+			t.Errorf("filtered wordlist still contains a separator-bearing word %q", w)
+		}
+		got[w] = true
+	}
+	for _, w := range hyphenatedEFFWords {
+		if got[w] {
+			t.Errorf("joinSafe kept %q, which contains %q", w, RecoveryCodeSep)
+		}
+	}
+}
+
+// TestEntropyFloorSurvivesFiltering states the numbers explicitly: dropping 4 of 7776 words costs
+// ~0.0007 bits/word, so the 10-word code stays above the 128-bit floor with room to spare.
+func TestEntropyFloorSurvivesFiltering(t *testing.T) {
+	const floorBits = 128.0
+	before := float64(RecoveryCodeWords) * math.Log2(7776)
+	after := RecoveryCodeEntropyBits()
+
+	if after < floorBits {
+		t.Fatalf("filtered entropy %.3f bits is below the %.0f-bit floor", after, floorBits)
+	}
+	if want := float64(RecoveryCodeWords) * math.Log2(float64(WordlistSize())); math.Abs(after-want) > 1e-9 {
+		t.Fatalf("RecoveryCodeEntropyBits() = %.6f, want %.6f (10 * log2(%d))", after, want, WordlistSize())
+	}
+	// Concrete expectations, so a wordlist change that quietly erodes the margin is visible:
+	// 10*log2(7776) = 129.248 bits before, 10*log2(7772) = 129.241 bits after — a 0.007-bit cost.
+	if math.Abs(before-129.248) > 0.001 {
+		t.Fatalf("unfiltered entropy baseline moved: %.3f, expected 129.248", before)
+	}
+	if math.Abs(after-129.241) > 0.001 {
+		t.Fatalf("filtered entropy moved: %.3f, expected 129.241", after)
+	}
+	if cost := before - after; cost > 0.01 {
+		t.Fatalf("filtering cost %.4f bits, expected well under 0.01", cost)
+	}
+}
+
+// TestGeneratedCodeSegments_FilteredVsUnfiltered is the deterministic red-proof companion.
+//
+// Against a list where EVERY word contains the separator, a 10-word draw MUST segment into more
+// than 10 parts — that is the pre-fix behaviour, reproduced with probability 1 instead of the ~1/5
+// flake the real list produced. Against the same list run through joinSafe, generation must refuse
+// (nothing is left to draw from), proving joinSafe — not luck — is what makes a code segmentable.
+func TestGeneratedCodeSegments_FilteredVsUnfiltered(t *testing.T) {
+	unfiltered := hyphenatedEFFWords
+
+	words, err := generateWords(unfiltered)
+	if err != nil {
+		t.Fatalf("generateWords(unfiltered): %v", err)
+	}
+	if len(words) != RecoveryCodeWords {
+		t.Fatalf("generator drew %d words, want %d", len(words), RecoveryCodeWords)
+	}
+	joined := strings.Join(words, RecoveryCodeSep)
+	segs := len(strings.Split(joined, RecoveryCodeSep))
+	if segs <= RecoveryCodeWords {
+		t.Fatalf("unfiltered draw segmented into %d parts; the pre-fix defect should yield more than %d",
+			segs, RecoveryCodeWords)
+	}
+	if segs != 2*RecoveryCodeWords {
+		t.Fatalf("every fixture word has exactly one separator, so 10 words must segment into 20 parts, got %d", segs)
+	}
+
+	// Same fixture, filtered: the draw space is empty, so generation must error rather than
+	// silently fall back to something ambiguous.
+	if _, err := generateWords(joinSafe(unfiltered)); err == nil {
+		t.Fatal("generateWords on a fully-filtered list must fail, not return a code")
+	}
+}
+
+// TestGenerateRecoveryCode_NeverContainsAmbiguousWord is the production-wiring test: it asserts the
+// exported entry point (not just the helper) draws from the filtered list.
+func TestGenerateRecoveryCode_NeverContainsAmbiguousWord(t *testing.T) {
+	inFiltered := make(map[string]bool, len(wordlist))
+	for _, w := range wordlist {
+		inFiltered[w] = true
+	}
+	for i := 0; i < 500; i++ {
+		r, err := GenerateRecoveryCode()
+		if err != nil {
+			t.Fatalf("GenerateRecoveryCode: %v", err)
+		}
+		parts := strings.Split(r, RecoveryCodeSep)
+		if len(parts) != RecoveryCodeWords {
+			// Do not print r: it is a live-shaped secret.
+			t.Fatalf("code %d segmented into %d parts, want %d", i, len(parts), RecoveryCodeWords)
+		}
+		for _, p := range parts {
+			if !inFiltered[p] {
+				t.Fatalf("segment %q is not a filtered-wordlist word", p)
+			}
+		}
+	}
+}