search

trezor / trezor-crypto

:lock: Don't use this repo, use the new monorepo instead:
https://github.com/trezor/trezor-firmware
MIT License
501 stars 201 forks source link

Optionally pad the bip39 wordlist. #184

Closed keepkeyjon closed 5 years ago

keepkeyjon commented 5 years ago

This allows constant time word matching, without exercising UB.

Subsumes trezor/trezor-crypto #182.

jhoenicke commented 5 years ago

Question: can we just change the declaration to const char wordlist[][8] or ...[9] (for NUL-terminators) to pad the words and allow using memcpy/memcmp with fixed length. This should also use less extra memory. With [8] all current uses of strcpy/strcmp must be replaced by memcpy, [9] would be backwards compatible, but unalignedness would cause more non-constant timing.

Of course, then you also need to change return type of mnemonic_wordlist in bip39.c/h.

In the end you cannot get around non-constant timing. We cannot pad the words in the final seed, or we would violate bip39.

keepkeyjon commented 5 years ago

Question: can we just change the declaration to const char wordlist[][8] or ...[9] (for NUL-terminators) to pad the words and allow using memcpy/memcmp with fixed length. This should also use less extra memory.

I considered that, but wasn't sure whether any other consumers of this library would be sensitive to the type. Good point about unaligned memory.

prusnak commented 5 years ago

Let's not merge for the reasons stated above.