diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/bench/verify_script.cpp | 2 | ||||
-rw-r--r-- | src/key.cpp | 24 | ||||
-rw-r--r-- | src/key.h | 2 | ||||
-rw-r--r-- | src/script/sign.cpp | 22 | ||||
-rw-r--r-- | src/script/sign.h | 4 | ||||
-rw-r--r-- | src/test/key_tests.cpp | 36 | ||||
-rw-r--r-- | src/test/script_tests.cpp | 2 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 33 | ||||
-rw-r--r-- | src/wallet/wallet.h | 29 |
9 files changed, 109 insertions, 45 deletions
diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp index 3421d56ed2..312b66e38a 100644 --- a/src/bench/verify_script.cpp +++ b/src/bench/verify_script.cpp @@ -76,7 +76,7 @@ static void VerifyScriptBench(benchmark::State& state) CMutableTransaction txSpend = BuildSpendingTransaction(scriptSig, txCredit); CScriptWitness& witness = txSpend.vin[0].scriptWitness; witness.stack.emplace_back(); - key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0), witness.stack.back(), 0); + key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0), witness.stack.back()); witness.stack.back().push_back(static_cast<unsigned char>(SIGHASH_ALL)); witness.stack.push_back(ToByteVector(pubkey)); diff --git a/src/key.cpp b/src/key.cpp index 22fc7047ce..df452cd330 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -189,7 +189,20 @@ CPubKey CKey::GetPubKey() const { return result; } -bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, uint32_t test_case) const { +// Check that the sig has a low R value and will be less than 71 bytes +bool SigHasLowR(const secp256k1_ecdsa_signature* sig) +{ + unsigned char compact_sig[64]; + secp256k1_ecdsa_signature_serialize_compact(secp256k1_context_sign, compact_sig, sig); + + // In DER serialization, all values are interpreted as big-endian, signed integers. The highest bit in the integer indicates + // its signed-ness; 0 is positive, 1 is negative. When the value is interpreted as a negative integer, it must be converted + // to a positive value by prepending a 0x00 byte so that the highest bit is 0. We can avoid this prepending by ensuring that + // our highest bit is always 0, and thus we must check that the first byte is less than 0x80. + return compact_sig[0] < 0x80; +} + +bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, bool grind, uint32_t test_case) const { if (!fValid) return false; vchSig.resize(CPubKey::SIGNATURE_SIZE); @@ -197,7 +210,14 @@ bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, uint32_ unsigned char extra_entropy[32] = {0}; WriteLE32(extra_entropy, test_case); secp256k1_ecdsa_signature sig; - int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, test_case ? extra_entropy : nullptr); + uint32_t counter = 0; + int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, (!grind && test_case) ? extra_entropy : nullptr); + + // Grind for low R + while (ret && !SigHasLowR(&sig) && grind) { + WriteLE32(extra_entropy, ++counter); + ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, extra_entropy); + } assert(ret); secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig); vchSig.resize(nSigLen); @@ -114,7 +114,7 @@ public: * Create a DER-serialized signature. * The test_case parameter tweaks the deterministic nonce. */ - bool Sign(const uint256& hash, std::vector<unsigned char>& vchSig, uint32_t test_case = 0) const; + bool Sign(const uint256& hash, std::vector<unsigned char>& vchSig, bool grind = true, uint32_t test_case = 0) const; /** * Create a compact signature (65 bytes), which allows reconstructing the used public key. diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 65b5bf7aeb..1e4102c20f 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -417,22 +417,25 @@ public: const DummySignatureChecker DUMMY_CHECKER; class DummySignatureCreator final : public BaseSignatureCreator { +private: + char m_r_len = 32; + char m_s_len = 32; public: - DummySignatureCreator() {} + DummySignatureCreator(char r_len, char s_len) : m_r_len(r_len), m_s_len(s_len) {} const BaseSignatureChecker& Checker() const override { return DUMMY_CHECKER; } bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override { // Create a dummy signature that is a valid DER-encoding - vchSig.assign(72, '\000'); + vchSig.assign(m_r_len + m_s_len + 7, '\000'); vchSig[0] = 0x30; - vchSig[1] = 69; + vchSig[1] = m_r_len + m_s_len + 4; vchSig[2] = 0x02; - vchSig[3] = 33; + vchSig[3] = m_r_len; vchSig[4] = 0x01; - vchSig[4 + 33] = 0x02; - vchSig[5 + 33] = 32; - vchSig[6 + 33] = 0x01; - vchSig[6 + 33 + 32] = SIGHASH_ALL; + vchSig[4 + m_r_len] = 0x02; + vchSig[5 + m_r_len] = m_s_len; + vchSig[6 + m_r_len] = 0x01; + vchSig[6 + m_r_len + m_s_len] = SIGHASH_ALL; return true; } }; @@ -450,7 +453,8 @@ bool LookupHelper(const M& map, const K& key, V& value) } -const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator(); +const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator(32, 32); +const BaseSignatureCreator& DUMMY_MAXIMUM_SIGNATURE_CREATOR = DummySignatureCreator(33, 32); const SigningProvider& DUMMY_SIGNING_PROVIDER = SigningProvider(); bool IsSolvable(const SigningProvider& provider, const CScript& script) diff --git a/src/script/sign.h b/src/script/sign.h index 461aedc6da..24cddda51b 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -80,8 +80,10 @@ public: bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override; }; -/** A signature creator that just produces 72-byte empty signatures. */ +/** A signature creator that just produces 71-byte empty signatures. */ extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR; +/** A signature creator that just produces 72-byte empty signatures. */ +extern const BaseSignatureCreator& DUMMY_MAXIMUM_SIGNATURE_CREATOR; typedef std::pair<CPubKey, std::vector<unsigned char>> SigPair; diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index d9c3525b19..61db70decb 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -152,4 +152,40 @@ BOOST_AUTO_TEST_CASE(key_test1) BOOST_CHECK(detsigc == ParseHex("2052d8a32079c11e79db95af63bb9600c5b04f21a9ca33dc129c2bfa8ac9dc1cd561d8ae5e0f6c1a16bde3719c64c2fd70e404b6428ab9a69566962e8771b5944d")); } +BOOST_AUTO_TEST_CASE(key_signature_tests) +{ + // When entropy is specified, we should see at least one high R signature within 20 signatures + CKey key = DecodeSecret(strSecret1); + std::string msg = "A message to be signed"; + uint256 msg_hash = Hash(msg.begin(), msg.end()); + std::vector<unsigned char> sig; + bool found = false; + + for (int i = 1; i <=20; ++i) { + sig.clear(); + key.Sign(msg_hash, sig, false, i); + found = sig[3] == 0x21 && sig[4] == 0x00; + if (found) { + break; + } + } + BOOST_CHECK(found); + + // When entropy is not specified, we should always see low R signatures that are less than 70 bytes in 256 tries + // We should see at least one signature that is less than 70 bytes. + found = true; + bool found_small = false; + for (int i = 0; i < 256; ++i) { + sig.clear(); + std::string msg = "A message to be signed" + std::to_string(i); + msg_hash = Hash(msg.begin(), msg.end()); + key.Sign(msg_hash, sig); + found = sig[3] == 0x20; + BOOST_CHECK(sig.size() <= 70); + found_small |= sig.size() < 70; + } + BOOST_CHECK(found); + BOOST_CHECK(found_small); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index e56080b278..bc671394c0 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -369,7 +369,7 @@ public: std::vector<unsigned char> vchSig, r, s; uint32_t iter = 0; do { - key.Sign(hash, vchSig, iter++); + key.Sign(hash, vchSig, false, iter++); if ((lenS == 33) != (vchSig[5 + vchSig[3]] == 33)) { NegateSignatureS(vchSig); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 73d2b205c2..5a7fdf9a85 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1542,30 +1542,29 @@ int64_t CWalletTx::GetTxTime() const return n ? n : nTimeReceived; } -// Helper for producing a max-sized low-S signature (eg 72 bytes) -bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const +// Helper for producing a max-sized low-S low-R signature (eg 71 bytes) +// or a max-sized low-S signature (e.g. 72 bytes) if use_max_sig is true +bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig) const { // Fill in dummy signatures for fee calculation. const CScript& scriptPubKey = txout.scriptPubKey; SignatureData sigdata; - if (!ProduceSignature(*this, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) - { + if (!ProduceSignature(*this, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) { return false; - } else { - UpdateInput(tx_in, sigdata); } + UpdateInput(tx_in, sigdata); return true; } -// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes) -bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts) const +// Helper for producing a bunch of max-sized low-S low-R signatures (eg 71 bytes) +bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, bool use_max_sig) const { // Fill in dummy signatures for fee calculation. int nIn = 0; for (const auto& txout : txouts) { - if (!DummySignInput(txNew.vin[nIn], txout)) { + if (!DummySignInput(txNew.vin[nIn], txout, use_max_sig)) { return false; } @@ -1574,7 +1573,7 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> return true; } -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet) +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig) { std::vector<CTxOut> txouts; // Look up the inputs. We should have already checked that this transaction @@ -1588,14 +1587,14 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall assert(input.prevout.n < mi->second.tx->vout.size()); txouts.emplace_back(mi->second.tx->vout[input.prevout.n]); } - return CalculateMaximumSignedTxSize(tx, wallet, txouts); + return CalculateMaximumSignedTxSize(tx, wallet, txouts, use_max_sig); } // txouts needs to be in the order of tx.vin -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts) +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, bool use_max_sig) { CMutableTransaction txNew(tx); - if (!wallet->DummySignTx(txNew, txouts)) { + if (!wallet->DummySignTx(txNew, txouts, use_max_sig)) { // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) // implies that we can sign for every input. return -1; @@ -1603,11 +1602,11 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall return GetVirtualTransactionSize(txNew); } -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, bool use_max_sig) { CMutableTransaction txn; txn.vin.push_back(CTxIn(COutPoint())); - if (!wallet->DummySignInput(txn.vin[0], txout)) { + if (!wallet->DummySignInput(txn.vin[0], txout, use_max_sig)) { // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) // implies that we can sign for every input. return -1; @@ -2334,7 +2333,7 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const bool solvable = IsSolvable(*this, pcoin->tx->vout[i].scriptPubKey); bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)); - vCoins.push_back(COutput(pcoin, i, nDepth, spendable, solvable, safeTx)); + vCoins.push_back(COutput(pcoin, i, nDepth, spendable, solvable, safeTx, (coinControl && coinControl->fAllowWatchOnly))); // Checks the sum amount of all UTXO's. if (nMinimumSumAmount != MAX_MONEY) { @@ -2889,7 +2888,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac txNew.vin.push_back(CTxIn(coin.outpoint,CScript())); } - nBytes = CalculateMaximumSignedTxSize(txNew, this); + nBytes = CalculateMaximumSignedTxSize(txNew, this, coin_control.fAllowWatchOnly); if (nBytes < 0) { strFailReason = _("Signing transaction failed"); return false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ab3e38e807..57b22c0e49 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -276,7 +276,7 @@ public: }; //Get the marginal bytes of spending the specified output -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet); +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, bool use_max_sig = false); /** * A transaction with a bunch of additional info that only the owner cares about. @@ -461,9 +461,9 @@ public: CAmount GetChange() const; // Get the marginal bytes if spending the specified output from this transaction - int GetSpendSize(unsigned int out) const + int GetSpendSize(unsigned int out, bool use_max_sig = false) const { - return CalculateMaximumSignedInputSize(tx->vout[out], pwallet); + return CalculateMaximumSignedInputSize(tx->vout[out], pwallet, use_max_sig); } void GetAmounts(std::list<COutputEntry>& listReceived, @@ -507,6 +507,9 @@ public: /** Whether we know how to spend this output, ignoring the lack of keys */ bool fSolvable; + /** Whether to use the maximum sized, 72 byte signature when calculating the size of the input spend. This should only be set when watch-only outputs are allowed */ + bool use_max_sig; + /** * Whether this output is considered safe to spend. Unconfirmed transactions * from outside keys and unconfirmed replacement transactions are considered @@ -514,13 +517,13 @@ public: */ bool fSafe; - COutput(const CWalletTx *txIn, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn) + COutput(const CWalletTx *txIn, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn, bool use_max_sig_in = false) { - tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; + tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; use_max_sig = use_max_sig_in; // If known and signable by the given wallet, compute nInputBytes // Failure will keep this value -1 if (fSpendable && tx) { - nInputBytes = tx->GetSpendSize(i); + nInputBytes = tx->GetSpendSize(i, use_max_sig); } } @@ -976,14 +979,14 @@ public: void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries); bool AddAccountingEntry(const CAccountingEntry&); bool AddAccountingEntry(const CAccountingEntry&, WalletBatch *batch); - bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts) const + bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, bool use_max_sig = false) const { std::vector<CTxOut> v_txouts(txouts.size()); std::copy(txouts.begin(), txouts.end(), v_txouts.begin()); - return DummySignTx(txNew, v_txouts); + return DummySignTx(txNew, v_txouts, use_max_sig); } - bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts) const; - bool DummySignInput(CTxIn &tx_in, const CTxOut &txout) const; + bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, bool use_max_sig = false) const; + bool DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig = false) const; CFeeRate m_pay_tx_fee{DEFAULT_PAY_TX_FEE}; unsigned int m_confirm_target{DEFAULT_TX_CONFIRM_TARGET}; @@ -1308,9 +1311,9 @@ public: }; // Calculate the size of the transaction assuming all signatures are max size -// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. +// Use DummySignatureCreator, which inserts 71 byte signatures everywhere. // NOTE: this requires that all inputs must be in mapWallet (eg the tx should // be IsAllFromMe). -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet); -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts); +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false); +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, bool use_max_sig = false); #endif // BITCOIN_WALLET_WALLET_H |