bitshares / bitshares-core

BitShares Blockchain node and command-line wallet
https://bitshares.github.io/
Other
1.17k stars 646 forks source link

Review use of libsecp256k1 contexts #36

Closed vikramrajkumar closed 7 years ago

vikramrajkumar commented 7 years ago

From @pmconrad on July 3, 2015 14:58

I stumbled across this https://github.com/bitcoin/secp256k1/pull/254#issuecomment-118074138 :

That said, your question makes it clear that the intended use isn't clear. That should definitely be clarified in the header file and perhaps a README. I guess it should list this:

  • Do not create/destroy contexts for each call. Their creation is far slower than any library call, and part of their purpose is to reuse precomputed values across calls.
  • If you perform signing, you should call context_randomize after initialization of the context, and perhaps on a regularly basis (daily?).
  • When calling context_randomize pass in external entropy (perhaps from the operating system, perhaps from user input).
  • You are responsible for locking. After initialization, only context_randomize requires exclusive locked, but the caller is responsible for not calling any other functions that use the context during randomization.
  • If you do both validation and signing, you can either use a shared context for both, or separate contexts. One reason for using separate contexts if that for signing, calling context_randomize requires an exclusive lock on the context. If you need to do validation at the same time, it may be useful to separate them, so validation is not blocked by context randomization.

Apparently we need to call context_randomize regularly (for signing), with proper synchronization.

Copied from original issue: cryptonomex/graphene#131

vikramrajkumar commented 7 years ago

From @bytemaster on July 3, 2015 19:9

This deserves some major brownie pts, pmconrad, please post a BTS account name.

vikramrajkumar commented 7 years ago

From @pmconrad on July 3, 2015 20:0

Well, since I coded some of it I don't feel like I really deserve it, but... please send them to "pmc", and thanks!

vikramrajkumar commented 7 years ago

From @arhag on July 3, 2015 20:45

Wait, libsecp256k1 supports deterministic signing.

Derandomized DSA (via RFC6979 or with a caller provided function.)

Here is RFC6979 by the way: https://tools.ietf.org/html/rfc6979

So is that extra entropy necessary at all or is it just an optional bonus? I don't understand why context_randomize should be necessary at all?

Edit: Apparently, this (https://github.com/bitcoin/secp256k1/commit/d2275795ff22a6f4738869f5528fbbb61738aa48) is why it is needed. It is to "reduce exposure to potential power/EMI sidechannels". Crazy stuff. Of course, it isn't a threat at all if the attacker isn't physically near the signing device.

vikramrajkumar commented 7 years ago

From @theoreticalbts on January 8, 2016 20:21

@bytemaster what is the status of this ticket? Is this a real problem, or can I close it?

vikramrajkumar commented 7 years ago

Too unclear