lian / bitcoin-ruby

bitcoin utils and protocol in ruby.
Other
920 stars 320 forks source link

OpenSSL 1.0.1k handling of DER signatures #156

Closed rnicoll closed 9 years ago

rnicoll commented 9 years ago

The issue bitcoin/bitcoin#5634 is relevant to Bitcoin-ruby. I don't have time to look at this myself at the moment, unfortunately, however I think this should be a case of adding a method to lib/bitcoin/ffi/openssl.rb to perform the DER signature re-packing, and then calling it from Bitcoin.verify_signature() and Key.verify() just before calling dsa_verify_asn1()

mhanne commented 9 years ago

So if I understand it correctly, the new version of OpenSSL will check for broken DER-encodings and consider them invalid. Since that would lead to a consensus change (and make it impossible for new clients to validate some old transactions), we need to fix the signatures before passing them to OpenSSL. Fair enough, though as @john-connor pointed out, unfortunately the check will be performed twice...

lian commented 9 years ago

@rnicoll thanks. writing a patch now.

lian commented 9 years ago

@mhanne twice? you men in verify method again? and thus decreasing speed?

mhanne commented 9 years ago

We have to check the signature to know if we need to repair it. And then OpenSSL does the same check again. Not much we can do about it though, other than using libsecp256k1.

lian commented 9 years ago

yep, i want to switch to libsecp256k1 at asom epoint anyway. but openssl will stay as legacy/fallback option.

lian commented 9 years ago

fixed in 82a7c1c100f46807b1c9352e3e541424d84dde8f. please review.