moserware / SecretSplitter

A program to split up secrets to go along with the blog post "Life, Death, and Splitting Secrets" at Moserware.com
http://www.moserware.com/2011/11/life-death-and-splitting-secrets.html
MIT License
88 stars 21 forks source link

Problem with SecretSplitter/SecretCombiner when secretMessage starts with zeros #2

Open waldemarburaczewskiiq5 opened 7 years ago

waldemarburaczewskiiq5 commented 7 years ago

If byte array parameter secretMessage has zeros at start then reconstructed secret doesn't have those leading zeros. Example code byte[] secret = new byte[] { 0x00, 0x00, 0x00, 0x05 }; SplitSecret splitSecret = SecretSplitter.SplitMessage(secret, 3); IEnumerable secretShares = splitSecret.GetShares(5); List shares = secretShares.Select(i => i.ToString()).ToList(); CombinedSecret combinedSecret = SecretCombiner.Combine(shares.Take(3)); byte[] result= combinedSecret.RecoveredBytes; //result length is 1

BlueRaja commented 6 years ago

Has this been looked at yet? Not recovering the correct secret is a huge issue.

lellis1936 commented 5 years ago

Very old comment, and even much older project.

However, I can say that ssss-split and ssss-combine, on which this project is based, are not designed to fully support binary secrets of arbitrary length, though they do appear to handle leading zeros better than this project. Ultimately secrets are considered "numbers", and as such leading zeros are insignificant. This probably explains why leading zeros in the output are removed.

You must avoid leading binary zeros in secrets. Typically this is done by converting binary secrets to hex strings before splitting. In fact that is the way this program handles file-based secrets: it generates a binary secret to serve as an encryption key and converts it to a hex string before splitting. Another option is to prepend a non-zero byte to the secret and remove it after the secret is recovered.

That said, although this project was a very fine piece of work I'd recommend Bouncy Castle or an alternative for serious crypto work, as that library has significant support. Bouncy unfortunately doesn't appear to support Shamir's secret sharing, though.

lellis1936 commented 5 years ago

While my previous comment is true, the loss of leading binary zeros is a bug that has a simple fix. I have a working modification.

BlueRaja commented 5 years ago

I have a working modification.

Nice! Would you mind sending a PR? Hopefully @moserware is still around to look at it.

lellis1936 commented 5 years ago

I have done so. In the meantime my fork has the fix.