search

mvondracek / PA193_mnemonic_Slytherin

BIP39 Mnemonic Phrase Generator and Verifier
0 stars 3 forks source link

Implement tranformation from mnemonic phrase to seed #12

Closed sobuch closed 4 years ago

mvondracek commented 4 years ago

We need to test program behaviour when mnemonic isn't valid UTF-8. https://github.com/mvondracek/PA193_mnemonic_Slytherin/pull/21#discussion_r336257567

mvondracek commented 4 years ago

Please add validation of user's password.

salt should be about 16 or more bytes https://docs.python.org/3.7/library/hashlib.html#hashlib.pbkdf2_hmac

https://github.com/mvondracek/PA193_mnemonic_Slytherin/blob/9907977d97aca204b272351a66ebe3638e0e0481/PA193_mnemonic_Slytherin/mnemonic.py#L24-L35

https://github.com/mvondracek/PA193_mnemonic_Slytherin/blob/9907977d97aca204b272351a66ebe3638e0e0481/PA193_mnemonic_Slytherin/mnemonic.py#L33

passphrase is used as salt.

seed_password should be at least 8 bytes long, otherwise raise instance of ValueError.

lsolodkova commented 4 years ago

salt should be about 16 or more bytes

Funny, but BIP-39 specification allows empty passwords.

If a passphrase is not present, an empty string "" is used instead.

Anyway, it's up to you, one more line of code isn't a problem.

mvondracek commented 4 years ago

I didn't check BIP-39, sorry. Thanks for info. Validation of passwrod is not needed as we should stick to the standard. :)

mvondracek commented 4 years ago

Related: #29

mvondracek commented 4 years ago

Concerning #29, what if we limit maximum password length to... let say 256 UTF-8 characters?

lsolodkova commented 4 years ago

It happens that for Latin (in particular, English) phrases all is fine; for Japanese (and other non-Latin), though, we have problems. Added some tests in https://github.com/mvondracek/PA193_mnemonic_Slytherin/commit/73104798252087f654fef3eb2c2fce492dd04127.

what if we limit

If I don't see the code, I don't know how to break it. Btw, who must check the length of the password?

mvondracek commented 4 years ago

@lsolodkova , @sobuch

During today's lecture, some students asked if they can use PBKDF2 from standard library of Go and from standard library of Java, doc. Švenda told them they cannot -- they have to implement it and they can use only functions for hashes. So I told him that I explicitly asked about Python's PBKDF2 after previous lecture and that it was accepted. The result is there was some misunderstanding and we have to implement PBKDF2. We talked about it and it makes sense, because teams working in pure C would have great disadvantage.

Seed generation has to be implemented without hashlib.pbkdf2_hmac, just with hashing functions. It's _generate_seed or Mnemonic.toSeed depending on when #36 gets merged.

Related: #16

lsolodkova commented 4 years ago

depending on when #36 gets merged.

Should I work on that now or wait for #36 ?

sobuch commented 4 years ago

you could review https://github.com/mvondracek/PA193_mnemonic_Slytherin/pull/36 and then we can merge it :)

mvondracek commented 4 years ago

@lsolodkova #36 is already merged. Please have a look at the seed generation.

I also updated branch fix-_generate_seed_invalid-password-too-long from dev and fixed conflicts. Please keep in mind bug #29 when you implement PBKDF2 and set some reasonable limit for password.

mvondracek commented 4 years ago

https://github.com/mvondracek/PA193_mnemonic_Slytherin/pull/42#issuecomment-546536654

mvondracek commented 4 years ago

I think this can be closed, what about you, @lsolodkova?

lsolodkova commented 4 years ago

Of course. Looking forward to any bugs and issues:)