trezor / python-shamir-mnemonic

MIT License
165 stars 59 forks source link

Please clarify warning in README #17

Closed jonathancross closed 4 years ago

jonathancross commented 4 years ago

The following warning is unclear:

It is very obviously insecure. DO NOT USE it for generating or decoding any sort of serious secrets.

matejcik commented 4 years ago

It's clearly saying that it is insecure; not incorrect nor incompatible.

The implementation is not using any sort of hardening techniques. The calculations are probably vulnerable to many side-channels, secret handling is done using standard memory, secrets are taken from shell command line (which usually gets stored in shell history), and the results are dumped to stdout. Although the random generation method should be OK, no effort was spent on ensuring that. The purpose of this tool is to be a reference implementation against which other implementers can check correctness, not an actual end-user tool -- hence, end-user-friendly warning, "insecure, do not use for serious secrets".

We might add something to the effect of "if you need to use this, do it on an air-gapped system", do you perhaps have a wording in mind?

jonathancross commented 4 years ago

Yeah, I think such a warning would be a great addition.

If you need to use this, do it on an air-gapped system as it might leak secrets to shell history, temp tiles, insecure use of memory, etc.

Would you still consider this inadvisable?

Thanks!

jonathancross commented 4 years ago

Would be great if we could get this fix to the README merged into master :-)

matejcik commented 4 years ago

fixed in 9ebff7d