Serialization to seed & subsequent re-serialization to shares breaks shamir recover result

[scgbckbone]

I just noticed that if I import string share(s) to my app and serialize it for storage where I'm storing just seed (aka data) and metadata (hrp, idx, threshold, id) separately. Next when I try to load storage data to Share object (via from_seed) and try to recover secret share s, shares before serialization provide different result compared to shares loaded from seed.

My guess is that this has something to do with padding. Any idea how to fix this issue ?

I added test case proving the point:

---- tests::my_vector stdout ----
thread 'tests::my_vector' panicked at src/lib.rs:501:9:
assertion `left == right` failed
  left: "10cbc41852b76438e5781f2cefb49799"
 right: "10cbc41852b76438e5781f2cefb4979f"

[apoelstra]

Yeah, it's to do with padding, but the reason seems to be that this library is badly broken/confused.

In codex32 the threshold/id/index 3k00la are all part of the codex32-encoded data along with the "actual" data. The first six characters are 30 bits, meaning that the share data, when encoded, should be right-shifted by 6 bits. Instead, we treat the data as its own bytestring which we convert to/from codex32 without shifting, effectively injecting 2 "padding" bits that aren't recognized as padding, aren't zeroed out, and eventually wind up at the end of the string.

At least, that's my best interpretation of what's going on. The data structures in this library are a real mess and combine strings, Fe vectors and byte vectors in arbitrary/lazy ways.

I apologize for the state of this library -- for a long time I have intended to replace all the error-correction logic with rust-bech32 0.12, which will have codex32 support. (It will not have interpolation logic, but that's easy/small and I will continue to implement it here.)

However, I let rust-bech32 0.12 get scope-creeped into doing error correction, which is still not done. There is a tracking issue here rust-bitcoin/rust-bech32#189. Maybe I should just cut a release so that I can fix this library.

[apoelstra]

Oh, I'm not actually blocked on rust-bech32 0.12. Indeed, the docs for 0.11 have codex32 support as an example.

[BenWestgate]

The .from_seed() factory needs a padding parameter and .to_seed() or codex32_decode(bech) needs to return the encoding's padding.

Without these it won't round trip and worse recovers a different seed.

Using default 0 padding on shares has some other problems:

• Can't disambiguate Bech32 vs Codex32 checksum formats during error correction.
• If attacker with < k derived shares knows the initial k used 0 padding, the last character of the seed is leaked early. Attacker learns 5 bits per share but the random data is only (5-pad_len) * k. So for 128-bit seeds, two k=3 shares known to use zero padding reveal the last 3 bits of the secret.
• Missed chance to detect bit errors, e.g.: 4-bit burst errors on 256-bit seed data. (95% chance of detecting a symbol error)

For the BIP85 codex32 application (and that compact QR idea) I set the padding bits by CRC of the data to avoid this.

That way they depend on 128-bits of unknown data and can't be assumed and are deterministic.

A fast fix would be .from_seed only constructs index "s" strings but that loses useful functionality vs giving it a secure deterministic default.

[apoelstra]

If it's only possible to construct S strings then users might as well just use rust-bech32 :). The point of this library is that it can also do interpolation.

[BenWestgate]

Then from_seed needs a padding parameter and if optional, a secure default that is unknown to an attacker without the seed data the share was encoded from.