From aaaeb9cdd58ebd2a8e91f057e39c6443008a95d2 Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sun, 23 Apr 2023 03:19:36 +0300 Subject: [PATCH 1/5] Revert "key: derive compression status from key length" This reverts commit 86741013c1cf3ca438446784e7deb5c85ce741af. This assumption is not solid for any pre-5.4.0.0 wallets which can contain OpenSSL generated keys. --- src/key.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/key.cpp b/src/key.cpp index cf0f9fb104..68922d2244 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -284,7 +284,7 @@ bool CKey::SignCompact(const uint256 &hash, std::vector& vchSig) bool CKey::Load(const CPrivKey &seckey, const CPubKey &vchPubKey, bool fSkipCheck=false) { if (!ec_seckey_import_der(secp256k1_context_sign, (unsigned char*)begin(), seckey.data(), seckey.size())) return false; - fCompressed = seckey.size() == CKey::COMPRESSED_SIZE; + fCompressed = vchPubKey.IsCompressed(); fValid = true; if (fSkipCheck) From af64e962b44bef0eaa146126c33fca2339342f08 Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sun, 23 Apr 2023 03:58:28 +0300 Subject: [PATCH 2/5] rpc: use WIF instead of HEX encoded keys in sendalert(2) Rationale: HEX encoded keys do not denote enough information on whether the key should be used with compressed or uncompressed public keys. Even though these interfaces should not be affected, better be consistent than sorry. --- src/rpc/net.cpp | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index f652cdc716..e6a023bd05 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -490,7 +490,7 @@ UniValue sendalert(const UniValue& params, bool fHelp) "sendalert [cancelupto]\n" "\n" " ----> is the alert text message\n" - " -> is hex string of alert master private key\n" + " -> is WIF encoded alert master private key\n" " -----> is the minimum applicable internal client version\n" " -----> is the maximum applicable internal client version\n" " ---> is integer priority number\n" @@ -517,8 +517,20 @@ UniValue sendalert(const UniValue& params, bool fHelp) sMsg << (CUnsignedAlert)alert; alert.vchMsg = vector((unsigned char*)&sMsg.begin()[0], (unsigned char*)&sMsg.end()[0]); - vector vchPrivKey = ParseHex(params[1].get_str()); - key.Load(CPrivKey(vchPrivKey.begin(), vchPrivKey.end()), CPubKey(), true); + CBitcoinSecret vchSecret; + + if (!vchSecret.SetString(params[0].get_str())) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); + } + + bool fCompressed; + CSecret secret = vchSecret.GetSecret(fCompressed); + key.Set(secret.begin(), secret.end(), fCompressed); + + if (!key.IsValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); + } + if (!key.Sign(Hash(alert.vchMsg), alert.vchSig)) throw runtime_error( "Unable to sign alert, check private key?\n"); @@ -551,7 +563,7 @@ UniValue sendalert2(const UniValue& params, bool fHelp) // 0 1 2 3 4 5 6 "sendalert2 \n" "\n" - " -> is hex string of alert master private key\n" + " -> is WIF encoded alert master private key\n" " ---------> is the unique alert number\n" " -> comma separated list of versions warning applies to\n" " -> comma separated ids of alerts to cancel\n" @@ -591,8 +603,20 @@ UniValue sendalert2(const UniValue& params, bool fHelp) sMsg << (CUnsignedAlert)alert; alert.vchMsg = vector((unsigned char*)&sMsg.begin()[0], (unsigned char*)&sMsg.end()[0]); - vector vchPrivKey = ParseHex(params[0].get_str()); - key.Load(CPrivKey(vchPrivKey.begin(), vchPrivKey.end()), CPubKey(), true); + CBitcoinSecret vchSecret; + + if (!vchSecret.SetString(params[0].get_str())) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); + } + + bool fCompressed; + CSecret secret = vchSecret.GetSecret(fCompressed); + key.Set(secret.begin(), secret.end(), fCompressed); + + if (!key.IsValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); + } + if (!key.Sign(Hash(alert.vchMsg), alert.vchSig)) throw runtime_error( "Unable to sign alert, check private key?\n"); From fed7884605ad5a6ce94b841e6dab4fb2604c3d7a Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sun, 23 Apr 2023 04:11:31 +0300 Subject: [PATCH 3/5] rpc: do not allow hex encoded keys in importprivkey Rationale: Our DER parser is not sophisticated enough to determine whether the provided key should be accompanied by a compressed or uncompressed public key, so just accept WIF which already has that data. --- src/wallet/rpcdump.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index b806e0378a..b229dc7880 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -125,20 +125,17 @@ UniValue importprivkey(const UniValue& params, bool fHelp) CBitcoinSecret vchSecret; CKey key; - bool fGood = vchSecret.SetString(strSecret); - // If CBitcoinSecret key decode failed, try to decode the key as hex - if(!fGood) - { - auto vecsecret = ParseHex(strSecret); - if (!key.Load(CPrivKey(vecsecret.begin(), vecsecret.end()), CPubKey(), /*fSkipCheck=*/true)) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); + if (!vchSecret.SetString(strSecret)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key."); } - else - { - bool fCompressed; - CSecret secret = vchSecret.GetSecret(fCompressed); - key.Set(secret.begin(), secret.end(), fCompressed); + + bool fCompressed; + CSecret secret = vchSecret.GetSecret(fCompressed); + key.Set(secret.begin(), secret.end(), fCompressed); + + if (!key.IsValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key."); } if (fWalletUnlockStakingOnly) From 747d95787446686e886c6ca8fd885d4fb3d78b62 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 23 Apr 2023 00:43:54 -0400 Subject: [PATCH 4/5] Update changelog for 5.4.5.0 release. --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dd85d4f21..c7ab4f3ba9 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/) and this project adheres to [Semantic Versioning](https://semver.org/). +## [5.4.5.0] 2023-04-23, leisure + +### Added +none + +### Changed +none + +### Removed +none + +### Fixed + - key, rpc: fix key parsing #2682 (@div72) + ## [5.4.4.0] 2023-04-21, leisure ### Added From 0868881c0b391d165b96752a64d2dbbc97c74b97 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 23 Apr 2023 00:44:45 -0400 Subject: [PATCH 5/5] Increment version to 5.4.5.0 for release. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 295ecb2d22..4f0d63ae38 100755 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ dnl require autoconf 2.60 (AS_ECHO/AS_ECHO_N) AC_PREREQ([2.60]) define(_CLIENT_VERSION_MAJOR, 5) define(_CLIENT_VERSION_MINOR, 4) -define(_CLIENT_VERSION_REVISION, 4) +define(_CLIENT_VERSION_REVISION, 5) define(_CLIENT_VERSION_BUILD, 0) define(_CLIENT_VERSION_IS_RELEASE, true) define(_COPYRIGHT_YEAR, 2023)