search

AntonKueltz / fastecdsa

Python library for fast elliptic curve crypto
https://pypi.python.org/pypi/fastecdsa
The Unlicense
264 stars 77 forks source link

-Make the randfunc to generate a private_key modifiable #26

Closed sirk390 closed 5 years ago

sirk390 commented 5 years ago

I would like to suggest this change as well. It is useful for example when you generate private keys derived from a master secret or want to use another random number generator. This is a similar syntax to "PyCryptodome/ RSA.generate(self.size, randfunc=randfunc) "

AntonKueltz commented 5 years ago

I'd like to think about this a little more. On the one hand flexibility is nice, on the other hand this also lets you use non-cryptographically secure random sources, which gives users a way to shoot themselves in the foot, and it may be "overstepping" the bounds of what this package is trying to accomplish. Any thoughts?

sirk390 commented 5 years ago

I think it is important to have this. Not specifically for me as I can just copy and modify the function, but the random source is an important design decision which you can't decide for the user (this might even be decided by management, internal regulation, etc..). You can provide a good default value (os.urandom), but they might want to use 1/ a hardware RNG, 2/generate their random by PKDF from a master key (like in my problem, e.g. useful for having an immediately "backuped" key), 3/ trust better the random from openssl than from python 4/ Combine multiple of the previous ones to protect against RNG security issues. It can also be useful for testability (see the testcase I attached to this pull request). By writing it I verified the gen_private_key function and it seems correct :) I think the user error providing an unsecure RNG is very unlikely as 1/ users will just use the default values and follow the examples, or they will know what they are doing 2/ the "random" module doesn't provide a "rand_bytes" function (just a getrandbits) 3/ "Autoimport" mistakes which can sometimes happen between "random" and "random.SystemRandom()" are not affecting this particular case because the default is called "urandom". Libraries like PyCryptodome which are the reference also have this feature. I tried looking at more libraries to see if they allowed it, and funnily enough when looking at "ecdsa-python" I found a far worse issue as they were using unsecure random by default both to generate and to sign with the private key ! I submitted an issue and they will fix it