Repository navigation
A share supplied twice counts once, one line rule for pasted text, and property tests across both readers - #275
Merged
404SecNotFound merged 5 commits intoOct 24, 2026
Conversation
…rs on generated inputs A set holding k genuine shares was refused when one of them was pasted or scanned twice, and the generic message sent the person to check strips that were all correct. §6 now says a share whose decoded record is identical to one already supplied is that share supplied twice, and is dropped before the other rules run. Two different shares at one index are still refused, and a repeat never stands in for a missing share. - docs/FORMAT-V2-DESIGN.md §6: the rule, ahead of the index rule it narrows. - reference/keym2.py combine_shares and src/lib/keym-v2-shamir.ts combineShares: drop a repeated record, compared in constant time. The TypeScript only skips repeats, so its finally still erases every value. - The key-ceremony rehearsal lists a strip pasted twice once. reference/properties_test.py (npm run test:properties, and a step in the conformance workflow) draws armor, shares, share sets, paper parts and Padme lengths from a printed seed and hands every case to keym2.py and to the shipping TypeScript in one textverdicts batch. Twelve properties; every verdict must match the other side's, and where the answer is known it must be that answer. KEYMAKER_PROPERTY_SEED replays a run. The bridge gains two verdict kinds, combine and padlen. No new dependency. Checks added to the self-test, shamir-adversarial.mts, crosstest2.py and shamir-ui.spec.ts. Each new check was shown to fail with its fix removed.
…asted text, and stop crashes passing as refusals An independent review of the previous commit found sixteen gaps, fourteen of them distinct, each confirmed by a reproduction or a negative control. This closes them. The rule - §6 said to drop a repeat "before the rules below", which put the drop ahead of the checksum and padding-bit rules. A reader written from the prose would have dropped a damaged copy of a share as a repeat. Both readers already check each share first; the spec now says so, and that a record differing in any one field is a different share. One line rule (§7) - keym2.py join split with str.splitlines(), which also breaks at U+000B, U+000C, U+000D, U+0085, U+2028 and U+2029, so a part holding one came apart in the recovery kit and reassembled in the app. The share box used trim(), which keeps U+0085, so a line holding only that character failed a set the recovery kit opened. §7 now says a text holding one item per line is split at U+000A only, each line stripped of IGNORABLE, blank and "#" lines skipped. keym2.py text_lines and keym-text.ts textLines implement it; join, --shares-from, splitPaperParts and the share box all use them. Tests - The self-test, shamir-adversarial.mts and crosstest2.py refuse a copy of a share that differs only in k, or only in set id, and a damaged copy (spare padding bit, wrong checksum), each in both orders. crosstest2 compares refusals through both combiners rather than "did not open", which a reader returning garbage also satisfies. - The bridge records a TypeError, RangeError, ReferenceError or SyntaxError as a crash, which can never match a refusal, and its combine kind reads lines with the share box's own parseShareLines. The harness accepts only the exceptions keym2.py raises on purpose for each kind. - properties_test.py draws coefficients from the seeded generator and checks that two builds from one seed are identical, so the printed seed replays a run. It adds properties for an ignorable character inside or just after a share prefix, characters outside the alphabet, non-ASCII letters that upper-case into it, every spare padding bit, a consistent extra share above the k lowest, a different share or near copy at a supplied index in either order, skipped lines between shares, and comment lines and CRLF between parts. Every enumerated set is covered whatever KEYMAKER_PROPERTY_CASES is. - Comments in shared.ts and shamir-ui.spec.ts corrected. Each new check was shown to fail with the thing it tests removed, and each control compiled.
…ew's test gaps A second independent review, of the previous commit, found nine more gaps. Seven were handed to a refuter and all survived; the other two were confirmed by negative controls. The line rule - §7 said a line ends at U+000A only. A browser's text box hands over CR and CRLF as LF, so the app's share and container boxes always ended a line at a lone CR, and keym2.py had before the previous commit. Splitting at LF alone made a CR-only shares or parts file, which opens in the app, fail in the recovery kit. §7 now ends a line at LF, CRLF and a lone CR; U+000B, U+000C, U+0085, U+2028 and U+2029 stay ignorable inside a line. text_lines and textLines both follow it. Tests - The self-test runs join and decrypt --shares-from on CR-only and CRLF files whose parts and shares hold U+2028 and U+0085, and asserts the bytes. - A copy of a share with only its index changed is a different share and is interpolated, in the self-test, shamir-adversarial.mts, crosstest2.py and the property suite. - A KMSHARE2 copy whose set id differs only past byte 4, and a KMSHARE2 copy with a wrong checksum, are refused in both orders (shamir-adversarial.mts, crosstest2.py, properties_test.py). - crosstest2.py takes the pinned secret as its oracle and guards every call into the reference in the new block and the v2 combine check, so a regression is reported by name instead of ending the run. - properties_test.py joins every multi-line case with each of the three line ends, inserts characters from outside the alphabet as well as swapping them in, adds the Kelvin sign to the fold traps, puts damaged copies among a set's near copies, and rebuilds its cases in two child processes with different hash seeds so the replay check sees a set's iteration order. Each new check was shown to fail with the thing it tests removed, and each control compiled.
docs/vectors/ holds the expected bytes for the text and binary forms of KEYM v2, v3 and v4 as plain JSON, for anyone writing a third reader. There are 94 vectors in seven families. - primitives.json has HKDF-SHA-256 (RFC 5869 A.1 to A.3, prk and okm), ChaCha20-Poly1305 (RFC 8439 §2.8.2 and A.5), AES-256-GCM (NIST CAVP) and Argon2id, transcribed from pyca's cryptography_vectors 50.0.2 and checked against the libraries each reader loads. - containers.json has whole containers from pinned inputs with kdf_input, slot_key, core_header, k_table and slot_table_mac, including v3 §8's and v4 §8's vectors, all three ciphers, a key file and an NFD password. - shares.json, parts.json and text.json have a 3-of-5 split from pinned coefficients with both share texts and the set code, the KMPART1 and KMPART2 strings, armor in both widths, the IGNORABLE set and padded_len. - reject.json has 50 inputs a reader must refuse with v2 §6's one generic failure, each tagged with its rule. The container rows that test a rule are sealed correctly around one violation, so a reader that skips the rule opens them. - accept.json has 21 inputs a reader must open although it may be tempted to refuse: a bad slot beside a good one, a stripped slot table (which must also be reported), armor full of ignorable characters, shares out of order or repeated, and parts out of order. reference/vectors.py generates the pack from keym2.py and checks it, including the inputs and outputs printed in v3 §8 and v4 §8 and the files' exact text. scripts/vectors-test.mts reproduces every vector with the app's code. The pack is append-only, a wrong vector is superseded by an erratum that both suites skip, and its count is asserted in both. v2 §7 now states how armor is wrapped. keym-v2.ts exports the three intermediate derivations for the suite. npm run test:vectors, a conformance workflow step and the README list run it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Bet 5 of the review report's plan. Three parts, because each fell out of the one before.
reference/properties_test.pygenerates armor, shares, share sets, paper parts and Padmé lengths from a printed seed and hands every case tokeym2.pyand to the shipping TypeScript in one batch. Each verdict must match the other implementation's exactly, and where the right answer is known it must be that answer.keym2.py joinand the app split a paste into lines differently, and so did--shares-fromand the app's share box. §7 now defines it once, and both languages implement it once.Stacked on #274 (the audio carrier reader), so this diff shows only its own change. GitHub retargets it to
mainwhen #274 merges.The rules, in the house order
#is skipped. The other characters an editor may treat as a line end (U+000B, U+000C, U+0085, U+2028, U+2029) are IGNORABLE, so inside a line they are ignored.combine_sharesdrops a repeated record, comparing set id and value in constant time because a share value is key material.text_linesimplements the §7 line rule.joinand--shares-fromboth use it.combineSharesdoes the same ascombine_shares. The repeats are only skipped, so the existingfinallystill erases every decoded value.textLinesinkeym-text.tsis the line rule.splitPaperPartsand the share box'sparseShareLinesboth call it.What changes for a person:
keym2.py join, as it already did in the app. A lone CR still ends a line in both, as it did before.keym2.py.Nothing that opened before stops opening. These stay refused, in both readers, in conformance and in the property suite:
The property suite
npm run test:properties, also a step in the conformance workflow. About 5,900 cases, in under 5 seconds, across 24 properties. Every multi-line case is joined with each of the three line ends in turn.ıfor 1,ſfor S, the Kelvin sign for K), or any spare padding bit setEverything that can be enumerated is enumerated instead of drawn, whatever
KEYMAKER_PROPERTY_CASESis set to:So the controls below bite on every seed, not only on a lucky one.
A crash on either side is its own verdict and can never pass as a refusal:
keym2.pyraises on purpose for each kind of text.No new dependency. The TypeScript side is two new verdict kinds (
combine,padlen) on the bridge's existingtextverdictscommand.combinereads lines with the share box's ownparseShareLines, so the box is what is tested.Review
The first commit went through three independent reviews (rule semantics, harness strength, the other tests). They returned 16 findings, 14 of them distinct. The 8 handed to a verifier told to refute them were all reproduced instead, and each of the other 6 was confirmed by a negative control or by reading the code. All are fixed in the second commit.
os.urandom.keym2.py joinand the app, and between--shares-fromand the share box.A second review of that commit found nine more, all fixed in the third commit. Seven were handed to a refuter and all survived; the other two were confirmed by negative controls.
joinor--shares-fromto its old line handling passed every suite. The self-test now runs both on CR-only and CRLF files holding U+2028 and U+0085, and asserts the bytes.upper()only.Tests
keym2.py selftestshamir-adversarial.mtscrosstest2.py(conformance)properties_test.pyshamir-ui.spec.tsNegative controls
Each was applied to a copy, confirmed to compile, and shown to fail the checks that name it without crashing the run.
shamir-adversarial.mts, conformance and the property suite as relevant. One TypeScript check first passed, because a reader that counted the repeat toward k returned garbage instead of refusing, and the check only asked whether the set opened. It now asserts a refusal..,_and/like a hyphen fails the inserted-character property. Python folding withlower()fails the fold-trap property on the Kelvin sign.upper(). That control first bit on 1 case in 200, so the fold traps are now placed where they matter, and it bites on all 363.text_linesusingsplitlines(), or ignoring a lone CR.textLinesusingtrim(), or ignoring a lone CR.trim()parser.joinreverting tosplitlines().--shares-fromsplitting at LF only, or withsplitlines().os.urandomfails the seed-replay property, and so does iterating the fold traps as a set.ValueErrorcrash fails 200 of 200 cases. WithValueErrorread as a refusal for every kind, it fails 52.Open question for the owner
Both readers accept a paper part with an ignorable character inside its
KMPART2:prefix, where armor and shares refuse the same placement. §7 says a reader ignores IGNORABLE characters "inside armor, share and part bodies" and does not say whether a part's prefix is its body. The property suite holds the two readers to agreement here without fixing an answer. Refusing would be stricter than today, and accepting is harmless because a part carries its own checksum and the set its fingerprint.Not in this PR
CHANGELOG.mdis left for the release that ships this.Verification on this tree
npm run typecheck,npm run build, and everytest:*script inpackage.jsonexcepttest:browser(44 scripts) pass on the head commit.--workers=2on the full suite, after a plainnpm run build, gives 360 passed, 5 skipped and 0 failed. Firefox and WebKit are left to CI.