BIP93: Fix checksum selection bounds and restrict ms sizes - #2258
BIP93: Fix checksum selection bounds and restrict ms sizes#2258BenWestgate wants to merge 2 commits into
ms sizes#2258Conversation
This comment was marked as low quality.
This comment was marked as low quality.
I added "reproducible boundary vectors." Edit: Unsure if I should have done this to satisfy a review comment that was later marked "low quality". For vectors, we need at minimum:
Willing to revert the "reproducible" vectors added in a9d8246 if reviews also find it overkill or open to another suggestion. What I would keep: Similarly ms32_create_regular_checksum creates the short checksum for data, while ms32_create_checksum selects the required checksum by expanded codewode length and creates it. It should be easier to generate test vectors and test them now, as well as to reuse our checksums for non-codex32 data. |
Account for the HRP expansion when selecting and slicing regular and long checksums. Enforce the invalid 94–95 value gap and update the affected test vectors. Aligns the reference implementation with bitcoin/bips#2258.
|
Based on my comment at #2040 (comment) which received some thumbs up by all parties here, I wonder if it everyone would find it preferable to simply restrict BIP-93 to seed lengths 128, 160, 192, 224, 256 and 512. I think I slightly prefer that, and if we are going to make such a change it would be better to do it from the get go. |
|
cACK
To prepare to generalize BIP-93 we still must also correct the checksum-period coverage or it leaves the unsafe checksum-selection question for every later application: For an Since restricting
|
Include the expanded human-readable part when selecting the codex32 checksum. This keeps regular codewords within the 93-symbol BCH period and rejects the 94- and 95-symbol gap before the long checksum starts at 96 symbols. Split regular and long checksum construction and verification so their periods can be tested independently. Add assertions for both selection boundaries and the long checksum upper limit. Refs: bitcoin#2258 (comment)
Include the expanded human-readable part when selecting the codex32 checksum. This keeps regular codewords within the 93-symbol BCH period and rejects the 94- and 95-symbol gap before the long checksum starts at 96 symbols. Split regular and long checksum construction and verification so their periods can be tested independently. Add assertions for both selection boundaries and the long checksum upper limit. Refs: bitcoin#2258 (comment)
53203a9 to
c201536
Compare
|
c201536 is ready for review. I drafted the next commit "BIP93: Restrict ms seed lengths", working on the vectors. Each commit is stand alone complete. |
Restrict codex32-encoded BIP32 master seeds to 16, 20, 24, 28, 32, or 64 bytes. These sizes correspond to the BIP39 entropy sizes and the 512-bit BIP32 seed produced by BIP39 recovery and provide minimum six-character length gaps to reduce target length ambiguity during optional insertion/deletion correction. Move payload decoding out of the generic codex32 secret definition and make it application-specific. Define the byte conversion and length requirements under the master seed format instead. Additionally: - Enforce the new valid encoded lengths (48, 54, 61, 67, 74, 127) in the Python `ms32_decode` in-line reference. - Remove obsolete short-checksum backward compatibility constraints and unsupported length test vectors. - Add test vectors for 160, 192, and 224-bit master seeds - Update rationale, compatibility, and reference implementation links.
ms sizes
|
ACK 5117f58 This looks good to me! Lol I think my hideous BIP39->codex32 conversion scheme is now permissible under "application-specific rules". |
It indeed is! I used your |
|
I had thought that SLIP-0039 supported just 128 and 256, but now I see that it supports lengths This PR for BIP-93 supports I think we should at give some consideration in our discussion here in supporting all SLIP-39 lengths. It would cause no problems for us regarding long and short codex32 checksums. It would make insertion and deletion recovery somewhat more difficult. With the full SLIP-93 lengths Thinking out loud, if you take a 160 bit, 54 length ms codex32 string and delete 3 random characters, can you actually able to recover that by trial and error inserting 3 characters? There are So we are considering a trade off here of enhanced SLIP-39 compatibility for secret lengths that probably no one ever uses versus allowing recovering an from up to 3 characters being omitted from a codex32 string, which feels like also an unlikely number of characters to forget to jot down. I'm genuinely torn here. What are the thoughts of other folks here? |
|
Maybe another angle to consider is, would users want to try 144 bit secrets with codex32? The argument is that Bitcoin has 128-bit security so you really want to debias your dice / entropy source when making a 128 bit secret. An alternative would be to make a longer secret and avoid debiasing your entropy with the idea that your longer secret would still contain 128-bits of entropy. This was my main motivation for considering 160 bit secrets (which was the next step up in BIP-39 secret sizes). But shorter is better because it is less work to create checksums and fewer characters means fewer errors to correct. The question is can users use regular biased dice to get 128 bits of entropy in 144 bits? The AI machine suggests an air bubble trapped in a cheap opaque die could bias one face up to 25% of the time. If the distribution were [25%, 16.75%, 16.75%, 16.75%, 16.75%, 8%], that would reduce the entropy of the dice from 2.58 bits per roll to 2.518 bits per roll. If the distribution were even more unreasonable at [33%, 16.66%, 16.66%, 16.66%, 16.66%, 0%] (this is also a fair die with one number repeated on a face). We'd have 2.25 bits per roll. Treating that die as fair would leave you with only than 125 bits of entropy in a 144 bit secret, but would give you 138 bits for a 160 bit secret. So I don't know. Maybe 144 bits is still to small to eliminate debiasing? |
|
As a point of information, as a child I had a PC game of monopoly where the RNG had an off by 1 error and 6s were never rolled. This gives a "five" sided dice with entropy of 2.32 per roll, which gives just over 128 bits of entropy for a 144 bit secret. We did eventually notice the missing 6, but it took quite a surprising amount of time to realize. I think I'd argue that 144 bit is inadequate for generating 128 bits of entropy using dice without debiasing. |
|
As counter argument for why 144 bits could be adequate, if you are building a 2 out of n secret share, you are xoring 2 biases secrets together which, generally speaking, reduces the bias of the resulting master secret. That is to say that while generating a 144 bit master secret form a plausibly biased die seems too close to borderline unsafe, generating a 144-bit master secret from 2 randomly generated shares feels a lot safer. (As a counter-counter argument, leaking k-1 shares reduces the security back down to a single share, though that involves the attacker both loading the dice and accessing k-1 shares). |
|
We could consider a compromise where ms secrets are limited to between 128 and 256 bits (i.e. 16-32 bytes) (plus the 512 bit long format), but generating bit sizes outside [128, 160, 192, 224, 256] is discouraged? I don't know if that is sensible. |
Adding 160, 192 and 224 already required my single threaded python indel correction to search 160, 192 and 224 one indel less deeply to stay under 10 seconds on my laptop. It can fill 4 omissions in 44 character observed in about 5 seconds. Since omissions are marked as erasures and algebraically solved, its only 149k combinations of 4 in 45 since deletions are marked as erasures and algebraically solved. 250k for 4 in 51 and 1.1 million for 4 in 71. (128, 160 and 256-bit respectively) The limit for filling omissions is false-correction. Intuitively it feels wrong to correct more deletions than substitutions so I prefer the symmetric "up to 4 extra or omitted characters, and mixtures" I.e. I+O <= 4. Removing inserted characters is faster and CPU limited rather than accuracy limited. I will simulate how much adding the 4 extra lengths increases ambiguity assuming non 128 and 256 remain searched to 3 indels. Lastly we say "we do not recommend interconversion and suggest users generate a fresh master seed" so this potentially harms an optional feature to better support a not recommended feature. We said wallets may assume the true length is 48 or 74. But if weird lengths get corrected to 48 or 74 they're not well supported so they need to be checked at some depth before "assuming 48 or 74". For avoiding transcription errors and having fast corrections of them, the best recommendation is avoid generating sizes besides 128-bit, the 4-indel correction is necessarily slower on 256-bit and they've more chances to make mistakes, we already say " shorter strings are better." However if not generating 128-bit, 256-bit is the next best size to reduce search ambiguity. SLIP 39 requires 128 and 256 and requires 16-bit multiples so if the argument is slip39 compatibility, its either 128 and 256 alone or every even byte size 16-64, not just those 16-32. |
It supports all even byte lengths 16-64 inclusive.
One of these lengths switched checksums in my first commit here. I will test indel recovery, if it's not meaningfully worse than the 6 lengths in my 2nd commit, I have no objection.
Only the positions need search, the characters are marked erasures and filled without brute force. 3 is very fast, 4 omitted is seconds on a laptop.
Its possible to recover from 5 omitted characters too if 0.1% chance of false corrections is acceptable. Limiting to 4 is safer. We dont recommend interconversion, indel recovery is optional. We could use a new profile SLIP39_EMS and let that one support every even byte length and also the passphrase feature from SLIP39, while using our SSS and checksums. I also don't oppose every even byte length if indel recovery is unharmed. |
I dont think a lower bound can be assumed on biased entropy. If the goal is less work, more efficient entropy extractors and 128-bit secrets are better.
Fewer lengths means fewer length errors to correct, fewer worksheets to print.
This would require modifying the "uniform random" part of share generation.
Biased shares means we can't say nothing is learned about the secret at K-1, if smallest strings are better, so is debasing. Debiasing doesn't take too long. I can generate 128-bits with a quarter in 1/2 hour. But I would not trust 160 or even 256 flips to be 128-bit entropy. |
My thought experiment was if my dice only give 1s and 2s treating that like log2(6) bits and xoring with another share produced with only 1s and 2s is not going to improve much. they're biased in the same way so entropy does not increase. |
Ah, my whole premise was mistaken then. Okay, I'm less inclined to support the full range of SLIP-39 seeds. The most common sizes of 128-bit and 256-bit seeds will still be interconvertable. And, as noted in this PR, all of the seed sizes supported by this amendment, are also interconvertable. Now I'm inclined to keep this aspect of the PR as is. |
| *** We do not define how to choose the identifier, beyond noting that it SHOULD be distinct for every master seed and master seed share set the user may need to disambiguate. | ||
| ** The share index "s". | ||
| ** A conversion of the 16-to-64-byte BIP-0032 HD master seed to bech32: | ||
| ** A conversion of a 16-, 20-, 24-, 28-, 32-, or 64-byte BIP-0032 HD master seed to bech32: |
There was a problem hiding this comment.
Would you be willing to rephrase this in terms of bits? I personally almost always think of seed sizes in terms of bits.
There was a problem hiding this comment.
Sure, the original used bytes to concisely imply multiples of 8-bits.
I think of bits when generating, but if encoding an existing, I think bytes as thats how the software moves seeds around.
Now that its noncontiguous the word savings is minimal.
| * Re-arrange those bits into groups of 8 bits. Any incomplete group at the end MUST be 4 bits or less, and is discarded. | ||
|
|
||
| Unlike the decoding process in BIP-0173, master-seed decoding does not require that the discarded incomplete group contain only zero bits. | ||
| The decoded master seed MUST be exactly 16, 20, 24, 28, 32, or 64 bytes. |
There was a problem hiding this comment.
I think this is fine to stay as bytes here since the decoding process is producing bytes as output.
| The 13 character checksum design only supports expanded codewords of up to 93 values. | ||
| After accounting for the expanded <code>ms</code> human-readable part, header, and checksum, this limits the payload of a regular codex32 string to 69 characters. | ||
| While this is enough to support the 32-byte advised size of BIP-0032 master seeds, BIP-0032 allows seeds to be up to 64 bytes in size. | ||
| We define a long codex32 format to support these longer seeds by defining an alternative checksum. |
There was a problem hiding this comment.
"While this is enough to support the 256-bit advised size of BIP-0032 master seeds, BIP-0032 allows seeds to be up to 512 bits in size. In particular, BIP-0039 derives 512 master seeds from its mnemonic.
We define a long codex32 format to support 512 bit seeds by defining an alternative checksum."
There was a problem hiding this comment.
Why don't we change bytes to bits in another PR?
It touches lines outside these two commits and is behavior neutral. I have a refactor for BIP93 queued based on this (with typos and reordering), a bit vs bytes commit could go there if you want to propose one over the full text.
I'm sure we made arbitrary choices and clarity could be improved.
|
|
||
| def ms32_encode(data): | ||
| combined = data + ms32_create_checksum(data) | ||
| return "ms" + "1" + ''.join([CHARSET[d] for d in combined]) |
There was a problem hiding this comment.
I recommend returning None if the constructed string it doesn't have a valid length.
There was a problem hiding this comment.
I copied the style of bech32_encode https://github.com/sipa/bech32/blob/master/ref%2Fpython%2Fsegwit_addr.py#L68-L71
Its intentional to not validate data on encode and create_checksum functions.
I sort of wanted to write encode from bytes for the "ms" secret, which would call decode to verify itself (like encode/decode, in segwit_addr.py) but I kept this since master seed shares also need an encode function but must not be encoded from bytes.
| A codex32 string is similar to a bech32 string defined in [https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki BIP-0173]. | ||
| It reuses the base-32 character set from BIP-0173, and consists of: | ||
|
|
||
| * A human-readable part, which is the string "ms" (or "MS"). |
There was a problem hiding this comment.
Do we want to rephrase this in this PR to say the HRP is application specific, but will be "ms" (or "MS") for the application defined in this document ... or something like that?
There was a problem hiding this comment.
To keep this focused I left HRP generalizing out of this PR. #2040, would say something like BIP173:
The human-readable part, which is intended to convey the type of data, or anything else that is relevant to the reader. This part MUST contain 1 to 500 US-ASCII characters, with each character having a value in the range [33-126]. HRP validity may be further restricted by specific applications.
The new vague sentence "string validity may be further restricted by specific applications" can then be moved to payload. "Payload validity may be further restricted by specific applications".
There was a problem hiding this comment.
Also before #2040 we should probably be conceptually clear on application vs profile vs HRP.
"cl" is one of each, "ms" is one of each. "bip39_" is an application with "12w" and "24w" profiles.
If I propose codex32 xpubs and prepend key origin info in their HRP, that's millions of HRP but one profile of a BIP32 key serialization application.
There was a problem hiding this comment.
Okay. I'm happy to postpone this until a future PR.
|
I got a bunch of emails from @BenWestgate last Wednesday on this PR but that do not appear here. I'm very confused. |
Motivation
BIP93 currently admits expanded codewords beyond the checksum’s stated period, so its detection guarantee no longer applies to the complete codeword. This is a problem for a generic
codex32_decodefunction in later PRs. Both regular and long checksums permitted codewords in excess of their period.Meanwhile, fixing the above will invalidate very uncommon
msstrings or leave an ugly disjoint checksum selection rule forlen(hrp) == 2. A better solution is immediately follow up with by restricting master seeds to the most commonly used seed and entropy sizes. This reduces target length ambiguity for optional insert/delete correcting wallets and my benchmarks show improvements in both false-correction risk and performance at the same indel search depths.Changes
ms32_verify_checksumandms32_create_checksumnow include the expanded "ms" HRP length when selecting regular vs. long checksums.ms32_create_checksumusesif 5 + len(data) > 80:as the switch toms32_create_long_checksum.ms32_create_regular_checksumandms32_verify_regular_checksum.ms32_decodeenforces the newmsstring lengths.codex32 lengths:
invalid codex32 lengths:
codex32-encoded master seed sizes:
unsupported
mssizes:Compatibility
This is a breaking change for old strings encoding deprecated byte-length seeds. However the authors do not see this is a problem due to the exceedingly rare possibility these have ever been created and used.
Testing
Thoroughly reviewed the complete diff.
Checked the new vectors.
Checked that expanded length 1023 verifies and 1024 fails.
Checked that the legacy short-checksum vectors should fail.
Checked that
ms32_decoderejects newly invalid lengths, even when they have valid header, incomplete group and checksum.Discussion
Proposal: #2040 (comment)
cACK #2040 (comment)
Reference impl issue: BlockstreamResearch/codex32#75
rust-codex32 PR: BlockstreamResearch/codex32#76
Restrict
mssizes suggestion: #2258 (comment)