Browse Source

Only create signatures with even S, and verification mode to check.

To fix a minor malleability found by Sergio Lerner (reported here:
https://bitcointalk.org/index.php?topic=8392.msg1245898#msg1245898)

The problem is that if (R,S) is a valid ECDSA signature for a given
message and public key, (R,-S) is also valid. Modulo N (the order
of the secp256k1 curve), this means that both (R,S) and (R,N-S) are
valid. Given that N is odd, S and N-S have a different lowest bit.
We solve the problem by forcing signatures to have an even S value,
excluding one of the alternatives.

This commit just changes the signing code to always produce even S
values, and adds a verification mode to check it. This code is not
enabled anywhere yet. Existing tests in key_tests.cpp verify that
the produced signatures are still valid.
0.10
Pieter Wuille 12 years ago committed by Pieter Wuille
parent
commit
a81cd96805
  1. 19
      src/key.cpp
  2. 26
      src/script.cpp
  3. 11
      src/script.h
  4. 4
      src/test/canonical_tests.cpp

19
src/key.cpp

@ -194,9 +194,26 @@ public:
} }
bool Sign(const uint256 &hash, std::vector<unsigned char>& vchSig) { bool Sign(const uint256 &hash, std::vector<unsigned char>& vchSig) {
vchSig.clear();
ECDSA_SIG *sig = ECDSA_do_sign((unsigned char*)&hash, sizeof(hash), pkey);
if (sig == NULL)
return false;
if (BN_is_odd(sig->s)) {
// enforce even S values, by negating the value (modulo the order) if odd
BN_CTX *ctx = BN_CTX_new();
BN_CTX_start(ctx);
const EC_GROUP *group = EC_KEY_get0_group(pkey);
BIGNUM *order = BN_CTX_get(ctx);
EC_GROUP_get_order(group, order, ctx);
BN_sub(sig->s, order, sig->s);
BN_CTX_end(ctx);
BN_CTX_free(ctx);
}
unsigned int nSize = ECDSA_size(pkey); unsigned int nSize = ECDSA_size(pkey);
vchSig.resize(nSize); // Make sure it is big enough vchSig.resize(nSize); // Make sure it is big enough
assert(ECDSA_sign(0, (unsigned char*)&hash, sizeof(hash), &vchSig[0], &nSize, pkey)); unsigned char *pos = &vchSig[0];
nSize = i2d_ECDSA_SIG(sig, &pos);
ECDSA_SIG_free(sig);
vchSig.resize(nSize); // Shrink to fit actual size vchSig.resize(nSize); // Shrink to fit actual size
return true; return true;
} }

26
src/script.cpp

@ -227,7 +227,10 @@ const char* GetOpName(opcodetype opcode)
} }
} }
bool IsCanonicalPubKey(const valtype &vchPubKey) { bool IsCanonicalPubKey(const valtype &vchPubKey, unsigned int flags) {
if (!(flags & SCRIPT_VERIFY_STRICTENC))
return true;
if (vchPubKey.size() < 33) if (vchPubKey.size() < 33)
return error("Non-canonical public key: too short"); return error("Non-canonical public key: too short");
if (vchPubKey[0] == 0x04) { if (vchPubKey[0] == 0x04) {
@ -242,7 +245,10 @@ bool IsCanonicalPubKey(const valtype &vchPubKey) {
return true; return true;
} }
bool IsCanonicalSignature(const valtype &vchSig) { bool IsCanonicalSignature(const valtype &vchSig, unsigned int flags) {
if (!(flags & SCRIPT_VERIFY_STRICTENC))
return true;
// See https://bitcointalk.org/index.php?topic=8392.msg127623#msg127623 // See https://bitcointalk.org/index.php?topic=8392.msg127623#msg127623
// A canonical signature exists of: <30> <total len> <02> <len R> <R> <02> <len S> <S> <hashtype> // A canonical signature exists of: <30> <total len> <02> <len R> <R> <02> <len S> <S> <hashtype>
// Where R and S are not negative (their first byte has its highest bit not set), and not // Where R and S are not negative (their first byte has its highest bit not set), and not
@ -286,6 +292,11 @@ bool IsCanonicalSignature(const valtype &vchSig) {
if (nLenS > 1 && (S[0] == 0x00) && !(S[1] & 0x80)) if (nLenS > 1 && (S[0] == 0x00) && !(S[1] & 0x80))
return error("Non-canonical signature: S value excessively padded"); return error("Non-canonical signature: S value excessively padded");
if (flags & SCRIPT_VERIFY_EVEN_S) {
if (S[nLenS-1] & 1)
return error("Non-canonical signature: S value odd");
}
return true; return true;
} }
@ -302,7 +313,6 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, co
if (script.size() > 10000) if (script.size() > 10000)
return false; return false;
int nOpCount = 0; int nOpCount = 0;
bool fStrictEncodings = flags & SCRIPT_VERIFY_STRICTENC;
try try
{ {
@ -841,9 +851,8 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, co
// Drop the signature, since there's no way for a signature to sign itself // Drop the signature, since there's no way for a signature to sign itself
scriptCode.FindAndDelete(CScript(vchSig)); scriptCode.FindAndDelete(CScript(vchSig));
bool fSuccess = (!fStrictEncodings || (IsCanonicalSignature(vchSig) && IsCanonicalPubKey(vchPubKey))); bool fSuccess = IsCanonicalSignature(vchSig, flags) && IsCanonicalPubKey(vchPubKey, flags) &&
if (fSuccess) CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType, flags);
fSuccess = CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType, flags);
popstack(stack); popstack(stack);
popstack(stack); popstack(stack);
@ -903,9 +912,8 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, co
valtype& vchPubKey = stacktop(-ikey); valtype& vchPubKey = stacktop(-ikey);
// Check signature // Check signature
bool fOk = (!fStrictEncodings || (IsCanonicalSignature(vchSig) && IsCanonicalPubKey(vchPubKey))); bool fOk = IsCanonicalSignature(vchSig, flags) && IsCanonicalPubKey(vchPubKey, flags) &&
if (fOk) CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType, flags);
fOk = CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType, flags);
if (fOk) { if (fOk) {
isig++; isig++;

11
src/script.h

@ -32,9 +32,10 @@ enum
enum enum
{ {
SCRIPT_VERIFY_NONE = 0, SCRIPT_VERIFY_NONE = 0,
SCRIPT_VERIFY_P2SH = (1U << 0), SCRIPT_VERIFY_P2SH = (1U << 0), // evaluate P2SH (BIP16) subscripts
SCRIPT_VERIFY_STRICTENC = (1U << 1), SCRIPT_VERIFY_STRICTENC = (1U << 1), // enforce strict conformance to DER and SEC2 for signatures and pubkeys
SCRIPT_VERIFY_NOCACHE = (1U << 2), SCRIPT_VERIFY_EVEN_S = (1U << 2), // enforce even S values in signatures (depends on STRICTENC)
SCRIPT_VERIFY_NOCACHE = (1U << 3), // do not store results in signature cache (but do query it)
}; };
enum txnouttype enum txnouttype
@ -665,8 +666,8 @@ public:
} }
}; };
bool IsCanonicalPubKey(const std::vector<unsigned char> &vchPubKey); bool IsCanonicalPubKey(const std::vector<unsigned char> &vchPubKey, unsigned int flags);
bool IsCanonicalSignature(const std::vector<unsigned char> &vchSig); bool IsCanonicalSignature(const std::vector<unsigned char> &vchSig, unsigned int flags);
bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType); bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType);
bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::vector<unsigned char> >& vSolutionsRet); bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::vector<unsigned char> >& vSolutionsRet);

4
src/test/canonical_tests.cpp

@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(script_canon)
string test = tv.get_str(); string test = tv.get_str();
if (IsHex(test)) { if (IsHex(test)) {
std::vector<unsigned char> sig = ParseHex(test); std::vector<unsigned char> sig = ParseHex(test);
BOOST_CHECK_MESSAGE(IsCanonicalSignature(sig), test); BOOST_CHECK_MESSAGE(IsCanonicalSignature(sig, SCRIPT_VERIFY_STRICTENC), test);
BOOST_CHECK_MESSAGE(IsCanonicalSignature_OpenSSL(sig), test); BOOST_CHECK_MESSAGE(IsCanonicalSignature_OpenSSL(sig), test);
} }
} }
@ -78,7 +78,7 @@ BOOST_AUTO_TEST_CASE(script_noncanon)
string test = tv.get_str(); string test = tv.get_str();
if (IsHex(test)) { if (IsHex(test)) {
std::vector<unsigned char> sig = ParseHex(test); std::vector<unsigned char> sig = ParseHex(test);
BOOST_CHECK_MESSAGE(!IsCanonicalSignature(sig), test); BOOST_CHECK_MESSAGE(!IsCanonicalSignature(sig, SCRIPT_VERIFY_STRICTENC), test);
BOOST_CHECK_MESSAGE(!IsCanonicalSignature_OpenSSL(sig), test); BOOST_CHECK_MESSAGE(!IsCanonicalSignature_OpenSSL(sig), test);
} }
} }

Loading…
Cancel
Save