BIP39: mention hard limit of 1024 bytes

[afk11]

By using SHA256 as a checksum and requiring a 32 to 1 ratio of entropy to checksum, we bound ENT to 8192 bits (1024 bytes), because CS has reached a maximum of 256 bits. Although it seems unrealistic, it is a hard limit, and not preventing it could lead to mnemonics that can't be decoded or other implementations can't verify.

Libraries that expose encoding/decoding a BIP39 mnemonic should check:

• whilst encoding, that the entropy size does not exceed 8192 bits, and that entropyBits % 32 == 0
• whilst decoding, that the length of the encoded bit-string does not exceed 8448 bits

Context: bitcoinjs/bip39#37 @dcousens @rubensayshi @voisine @prusnak

[dabura667]

Not necessary. Anyone making phrases that long are already breaking the BIP, anyways.

-1

[prusnak]

-1 as well. BIP39 specifies valid sizes of 128, 160, 192, 224 and 256 bits. Anything else is not BIP39.

[afk11]

The BIP makes recommendations (feel free to correct me) Not being clear on this point is perhaps the reason for this PR in the first place?

[prusnak]

No, it does not make recommendations. If the wording is not strong enough, please change it. I don't want to have several BIP39 wallets floating around which are not compatible.

[afk11]

The recommended size of ENT is 128-256 bits.

I know if this is to be taken as written, wallets are already not compatible. (haskoin, greenaddress..) I guess when provided the formula and not told a max, people are going to experiment and see what they're comfortable with. Although there aren't many wallets with 768 word mnemonics (the max), I can't see why it's a problem?

https://github.com/greenaddress/GreenBits/blob/8d3057fd202ba61af9cab9a8bb7b0d27395a286c/app/libwally-core/src/bip39.c#L87
https://github.com/haskoin/haskoin/blob/6b66396eb18329727525577b56a58a8b13741dca/haskoin-core/tests/Network/Haskoin/Crypto/Mnemonic/Tests.hs#L33

[prusnak]

Then they are not BIP39 compatible. Loosing restrictions in BIP39 does not make the situation better (2 wallets which are not BIP39 compatible vs making all BIP39 wallets incompatible).

[dcousens]

The recommended size of ENT is 128-256 bits.

@prusnak this is definitely not clear.
It implies a recommendation, not a rule.

There is no indication that sizes outside of that range are not BIP39 compatible.

edit: I don't care either way, but as is, it is not clear what implementations should be doing.

[rubensayshi]

iirc most implementations that I have seen do not limit the size at all because the BIP only recommends certain sizes, it's never mentioned the BIP is limited to those sizes.

now properly defining that this BIP is restricted to 128, 160, 192, 224 and 256 would make it a BC break when libraries implement this limitation.

considering this has been "out in the wild" in this state for so long already and there's people using non "recommended" sizes it seem more sane to only limit the BIP to 1024 bytes, the point where things start to break horribly.

[prusnak]

I don't know how to put it any more cleaner. If there are some broken implementations of BIP39, that is their problem. Let's rephrase the BIP if it is not crystal clear to everybody. Anyone is free to start their own BIP if BIP39 is not enough for their usage. End of discussion. Please close this request. Thanks!

[rubensayshi]

@prusnak @luke-jr what the ...

the BIP until now has not being clear AT ALL on what the limits were, only the recommended size, not a single word about that being the LIMIT.

as a result there are implementations and usage in the wild that have gone over the recommended sizes and I think you should man up to the result of the poorly specified limits and adjust the BIP to match what has happened as a result of that instead of suddenly making a whole bunch of "BIP39 compatible wallets" become no longer BIP39 compatible because you are fixing the BIP to match what was in your head!

disclaimer: Blocktrail is not BIP39 compatible, I don't have any incentive for this to go down either way except that I feel this is clearly WRONG

[btcdrak]

I tend to agree with @rubensayshi - the written specification is the specification, you can't declare the specification is whatever you intended but didn't write down. Does the reference implementation test for these supposed limits?

[luke-jr]

I'm inclined to agree that existing implementations shouldn't be retroactively made incompatible with the BIP, especially if Haskoin is one of such violations to this recommendation yet was accepted as an "other implementation". But the author has the final word on what changes to include in the BIP, so the only recourse would be to submit a competing BIP.

[rubensayshi]

Reference Implementation
http://github.com/trezor/python-mnemonic

• no check for max length at all

Other Implementations
https://github.com/nybex/NYMnemonic

• no check

https://github.com/haskoin/haskoin

• max 64 bytes

https://github.com/Thashiznets/BIP39.NET

• max 1024 bytes

https://github.com/NicolasDorier/NBitcoin

https://github.com/bitpay/bitcore-mnemonic

• no check

https://github.com/bitcoinjs/bip39

• no check

https://github.com/sreekanthgs/bip_mnemonic

• no check

Other Other Implementations
BitcoinJ

• no check

[rubensayshi]

even the reference implementation itself does not limit itself to the recommended sizes.

[prusnak]

disclaimer: Blocktrail is not BIP39 compatible, I don't have any incentive for this to go down either way except that I feel this is clearly WRONG

@rubensayshi However you phrase it, the reality is - if you don't use 128-256 bits, but something out of this range, most of the BIP39 wallets won't be able to import this mnemonic you generated. I don't know what your incentive is, but if you are generating mnemonics out of this range, it is clear that your incentive is not interoperability for your users.

And my point is that easier way to fix this "mess" (if there is any, TBH, I don't know how many users Blocktrail or hascoin has, but I doubt these are significant numbers when compared to other BIP39 wallets using the 128-256 bit range ...), is to make the standard more tight/specific (=interoperability) not more loose (=another meaningless standard which does not guarantee anything).

[btcdrak]

@prusnak I think you have this backwards. Someone who wants to implement a BIP will read the text look at the reference implementation and test vectors in order to ensure they faithfully reproduce the specification. In this case certain limits are not specified, not in text, not in reference implementation and not in test vectors. As such, I dont understand how you can claim compatibility based on intent. The specification and even reference implementation clearly allows higher limits.

[prusnak]

@btcdrak Test vectors never included lengths different than mentioned in the BIP. Fixing of the BIP is in #472, I am pushing check to python-mnemonic now.

@rubensayshi if you want to be useful (and you already did by collecting implementations and their limits), please open issues/pull requests to implementations asking them to set limit between 128-256 bits

My rationale about why I am being so strict about this: If you are forcing user to write down more than 256 bits of entropy (24 words), then something is really wrong with your application design (hell, users often have problem writing 24 words down correctly ...). I won't change my mind unless someone comes up with a really solid argument, why storing more than 256 bits of entropy is needed. So far there was none.