3

I have recently changed verification code in python-bitcointx to use libsecp256k1 library.

In the python-bitcoinlib code (that python-bitcointx is a derivative of), openssl functions were used for signature verification.

I want to be able to remove dependency on openssl in the future, and use only libsecp256k1 in python-bitcointx, but there is a problem that I noticed when I re-wrote CECKey.verify() method:

libsecp256k1 function secp256k1_ecdsa_signature_parse_der() only allows strictly valid DER-encoded signatures.

bitcoind uses ecdsa_signature_parse_der_lax() function, copied from libsecp256k1's contrib/ directory. This function is not compiled in a normal compilation cycle of libsecp256k1.

It is expected for bitcoin-focused library to be able to verify signatures that bitcoind will also verify.

To be able to verify non-strict-encoded signatures with libsecp256k1, I see 3 options:

EDIT: I might just implement two verification methods, strict and non-strict, and then use them in tests. openssl dependency will stay because of verify_nonstrict().

  • get ecdsa_signature_parse_der_lax() function from libsecp256k1/contrib, add it to python-bitcointx repository, compile at python package build time, install with a package.

This is not desireable, because it adds requirement to have C compiler to build the python package. In some environments this may be inconvenient.

  • re-implement ecdsa_signature_parse_der_lax() in python

I would like to avoid this, to reduce the size of the code (reduce the surface where bug may be introduced). Porting this function correctly will require to invest some time.

It will also make verification slower to some degree, because more operations will be done in python, vs calling C functions.

  • do a cycle of decode-with-openssl / encode-with-openssl / decode-with-libsecp256k1

This way all encoding violations that openssl allows are also allowed by first decode-with-openssl. encode-with-openssl then creates valid encoding, and decode-with-libsecp256k1 is happy.

The problem with this approach (in addition to this decode/encode cycle) is that it will allow some format violations that bitcoind won't allow, because ecdsa_signature_parse_der_lax() only allows an arbitrary subset of violations allowed by openssl.

python-bitcoinlib was using openssl to decode, too, so I went with the later approach - decode/encode/decode. This means I cannot drop openssl dependency.

My question is: what may be the implications of this choice ?

What I can see is that when someone uses python-bitcointx or python-bitcoinlib to verify inputs in some transaction, and these inputs contain signatures that is accepted by openssl, but not by bitcoind, they might be convinced that tx is valid, but will have unpleasant surprise when they try to broadcast it.

With some protocols that require parties to store signed transactions, that might be relevant.

As this is historical behavior of python-bitcoinlib, I am inclined to just document this in the README.md as a 'gotcha', and leave it as it is now.

But maybe there is some other potential problems that could arise out of this issue, that may be more serious, and the first or second options described above should be considered ?

5

It isn't clear to me from your question what your goal is. If you want to accept all transactions and only transactions that Bitcoin would accept, libsecp256k1 alone is what you want. (You may need to use the signature normalize call if you want to match consensus acceptance instead of standardness acceptance).

If you want to validate historical signatures from the blockchain then either openssl or the code in contrib is sufficient. If you use either of these things on current transactions you will accept things that Bitcoin will reject.

We, the authors of libsecp256k1, are currently of the belief that no application should use a lax parser, except for historical compatibility. Unfortunately, no function is likely to be universally compatible with OpenSSL's parser except OpenSSL itself... but for cases where the set of signatures that you need to be compatible with is known a specific alternative lax parser might work, as is the case for Bitcoin's history and the lax parser in contrib.

The contrib lax parser isn't included in the library because it's only provably sufficient for Bitcoin's history. Someone trying to lax-parse a different set of signatures may need to modify that lax parser to accommodate the specific weird signatures that they're dealing with. We also, in general, don't think anyone should be lax parsing unless its strictly needed since it can result in surprising vulnerabilities.

As an aside, in the current versions of OpenSSL the ECDSA validator attempts to validate signature encodings by checking that the serialization is unchanged under decode and encode and it rejects signatures that aren't. This likely means they reject any signature that libsecp256k1 (with normalize) rejects. [And annoyingly, OpenSSL made this breaking change in a security update.]

  • I mistakenly thought that lax parser may be still relevant, and this question arose out of that. This was because python-bitcoinlib tests also test script evaluation code for historical, lax-encoded signatures. The tests failed after I introduced strict decoding. Now I added handling for STRICTENC flag (that was missing) and do lax verification when STRICTENC is not specified. Tests with historical signatures now pass. I also added a warning to README that people should not use python-bitcointx's VerifyScript as authoritative source on input validity, because it may not be in-sync with core. – Dmitry Petukhov Dec 2 '18 at 5:35
  • Because I thought that python-bitcointx's VerifyScript might be relied upon by someone to check input validity, I was worried that the differences in handling strict encoding might bite them. I thought that this is the only serious difference b/w this lib and the core. The goal of the question was to understand the scope of the differences to decide how to handle this. It turned out that the differences are huge, and all bets are off - so the warning in README is necessary. – Dmitry Petukhov Dec 2 '18 at 6:16
1

If you want to guarantee strict DER encoded signatures which are accepted by Bitcoind then they need to be bip66 compliant(low s values(ecdsa malleability), remove leading nulls (encoding malleability)). I am not sure you are guarding for that with your decode-encode-decode cycle. These do not enforce r,s value-space constraints.

  • low s values are enforced separately, with secp256k1_ecdsa_signature_normalize(). This is about DER encoding (en.wikipedia.org/wiki/X.690#DER_encoding) that openssl and bitcoind may treat differently – Dmitry Petukhov Dec 1 '18 at 15:59
  • So you need to ensure for bip66 compliant DER encoding, which is a subset of the standard you linked above, no? – James C. Dec 1 '18 at 16:06
  • 1
    looks like the ecdsa_signature_parse_der_lax() is only relevant to the signatures before activation of BIP66, and I can just go with requiring strict DER encoding. There is some tests in that may fail as a result, but since it is irrelevant, I may just disable these tests. – Dmitry Petukhov Dec 1 '18 at 16:11
  • 1
    The parsing code used is always the same, before and after BIP66 activated. The difference is in an extra sanity check before the parser is invoked. – Pieter Wuille Dec 1 '18 at 17:57
  • 1
    I will need to use ported IsValidSignatureEncoding() to check for STRICTENC, and cannot just rely on secp256k1_ecdsa_signature_parse_der(), to be compatible with bitcoin's script interpreter. – Dmitry Petukhov Dec 1 '18 at 20:33
1

It was the case that python-bitcoinlib's script evaluation code did not handle SCRIPT_VERIFY_STRICTENC flag.

I had to create additional method, verify_nonstrict(), and make the scripteval use it when STRICTENC flag is not specified.

Handled in https://github.com/Simplexum/python-bitcointx/commit/d8ab8ff76de412f90480c3c68addbbb1791d3413

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.