From 2ce3447eb1e25ec7aec4b300dabf6c1e394f1906 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Tue, 19 Nov 2019 15:49:35 +0100
Subject: Deduplicate the message verifying code

The logic of verifying a message was duplicated in 2 places:

src/qt/signverifymessagedialog.cpp
  SignVerifyMessageDialog::on_verifyMessageButton_VM_clicked()

src/rpc/misc.cpp
  verifymessage()

with the only difference being the result handling. Move the logic into
a dedicated

src/util/message.cpp
  MessageVerify()

which returns a set of result codes, call it from the 2 places and just
handle the results differently in the callers.
---
 src/Makefile.am                    |  2 +
 src/qt/signverifymessagedialog.cpp | 78 ++++++++++++++++++++------------------
 src/rpc/misc.cpp                   | 32 ++++++----------
 src/test/util_tests.cpp            | 53 ++++++++++++++++++++++++++
 src/util/message.cpp               | 53 ++++++++++++++++++++++++++
 src/util/message.h                 | 49 ++++++++++++++++++++++++
 src/util/validation.cpp            |  2 -
 src/util/validation.h              |  2 -
 src/wallet/rpcwallet.cpp           |  2 +-
 9 files changed, 211 insertions(+), 62 deletions(-)
 create mode 100644 src/util/message.cpp
 create mode 100644 src/util/message.h

(limited to 'src')

diff --git a/src/Makefile.am b/src/Makefile.am
index 6edd5e75b7..eac7b38e03 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -220,6 +220,7 @@ BITCOIN_CORE_H = \
   util/system.h \
   util/macros.h \
   util/memory.h \
+  util/message.h \
   util/moneystr.h \
   util/rbf.h \
   util/settings.h \
@@ -517,6 +518,7 @@ libbitcoin_util_a_SOURCES = \
   util/error.cpp \
   util/fees.cpp \
   util/system.cpp \
+  util/message.cpp \
   util/moneystr.cpp \
   util/rbf.cpp \
   util/settings.cpp \
diff --git a/src/qt/signverifymessagedialog.cpp b/src/qt/signverifymessagedialog.cpp
index 5f2836cc75..aa8e23f53b 100644
--- a/src/qt/signverifymessagedialog.cpp
+++ b/src/qt/signverifymessagedialog.cpp
@@ -11,7 +11,7 @@
 #include <qt/walletmodel.h>
 
 #include <key_io.h>
-#include <util/validation.h> // For strMessageMagic
+#include <util/message.h> // For strMessageMagic, MessageVerify()
 #include <wallet/wallet.h>
 
 #include <vector>
@@ -189,51 +189,57 @@ void SignVerifyMessageDialog::on_addressBookButton_VM_clicked()
 
 void SignVerifyMessageDialog::on_verifyMessageButton_VM_clicked()
 {
-    CTxDestination destination = DecodeDestination(ui->addressIn_VM->text().toStdString());
-    if (!IsValidDestination(destination)) {
+    const std::string& address = ui->addressIn_VM->text().toStdString();
+    const std::string& signature = ui->signatureIn_VM->text().toStdString();
+    const std::string& message = ui->messageIn_VM->document()->toPlainText().toStdString();
+
+    const auto result = MessageVerify(address, signature, message);
+
+    if (result == MessageVerificationResult::OK) {
+        ui->statusLabel_VM->setStyleSheet("QLabel { color: green; }");
+    } else {
         ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
-        ui->statusLabel_VM->setText(tr("The entered address is invalid.") + QString(" ") + tr("Please check the address and try again."));
-        return;
     }
-    if (!boost::get<PKHash>(&destination)) {
+
+    switch (result) {
+    case MessageVerificationResult::OK:
+        ui->statusLabel_VM->setText(
+            QString("<nobr>") + tr("Message verified.") + QString("</nobr>")
+        );
+        return;
+    case MessageVerificationResult::ERR_INVALID_ADDRESS:
+        ui->statusLabel_VM->setText(
+            tr("The entered address is invalid.") + QString(" ") +
+            tr("Please check the address and try again.")
+        );
+        return;
+    case MessageVerificationResult::ERR_ADDRESS_NO_KEY:
         ui->addressIn_VM->setValid(false);
-        ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
-        ui->statusLabel_VM->setText(tr("The entered address does not refer to a key.") + QString(" ") + tr("Please check the address and try again."));
+        ui->statusLabel_VM->setText(
+            tr("The entered address does not refer to a key.") + QString(" ") +
+            tr("Please check the address and try again.")
+        );
         return;
-    }
-
-    bool fInvalid = false;
-    std::vector<unsigned char> vchSig = DecodeBase64(ui->signatureIn_VM->text().toStdString().c_str(), &fInvalid);
-
-    if (fInvalid)
-    {
+    case MessageVerificationResult::ERR_MALFORMED_SIGNATURE:
         ui->signatureIn_VM->setValid(false);
-        ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
-        ui->statusLabel_VM->setText(tr("The signature could not be decoded.") + QString(" ") + tr("Please check the signature and try again."));
+        ui->statusLabel_VM->setText(
+            tr("The signature could not be decoded.") + QString(" ") +
+            tr("Please check the signature and try again.")
+        );
         return;
-    }
-
-    CHashWriter ss(SER_GETHASH, 0);
-    ss << strMessageMagic;
-    ss << ui->messageIn_VM->document()->toPlainText().toStdString();
-
-    CPubKey pubkey;
-    if (!pubkey.RecoverCompact(ss.GetHash(), vchSig))
-    {
+    case MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED:
         ui->signatureIn_VM->setValid(false);
-        ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
-        ui->statusLabel_VM->setText(tr("The signature did not match the message digest.") + QString(" ") + tr("Please check the signature and try again."));
+        ui->statusLabel_VM->setText(
+            tr("The signature did not match the message digest.") + QString(" ") +
+            tr("Please check the signature and try again.")
+        );
         return;
-    }
-
-    if (!(CTxDestination(PKHash(pubkey)) == destination)) {
-        ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
-        ui->statusLabel_VM->setText(QString("<nobr>") + tr("Message verification failed.") + QString("</nobr>"));
+    case MessageVerificationResult::ERR_NOT_SIGNED:
+        ui->statusLabel_VM->setText(
+            QString("<nobr>") + tr("Message verification failed.") + QString("</nobr>")
+        );
         return;
     }
-
-    ui->statusLabel_VM->setStyleSheet("QLabel { color: green; }");
-    ui->statusLabel_VM->setText(QString("<nobr>") + tr("Message verified.") + QString("</nobr>"));
 }
 
 void SignVerifyMessageDialog::on_clearButton_VM_clicked()
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index f360cb7525..9686a111ed 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -11,9 +11,9 @@
 #include <rpc/util.h>
 #include <script/descriptor.h>
 #include <util/check.h>
+#include <util/message.h> // For strMessageMagic, MessageVerify()
 #include <util/strencodings.h>
 #include <util/system.h>
-#include <util/validation.h>
 
 #include <stdint.h>
 #include <tuple>
@@ -276,31 +276,21 @@ static UniValue verifymessage(const JSONRPCRequest& request)
     std::string strSign     = request.params[1].get_str();
     std::string strMessage  = request.params[2].get_str();
 
-    CTxDestination destination = DecodeDestination(strAddress);
-    if (!IsValidDestination(destination)) {
+    switch (MessageVerify(strAddress, strSign, strMessage)) {
+    case MessageVerificationResult::ERR_INVALID_ADDRESS:
         throw JSONRPCError(RPC_TYPE_ERROR, "Invalid address");
-    }
-
-    const PKHash *pkhash = boost::get<PKHash>(&destination);
-    if (!pkhash) {
+    case MessageVerificationResult::ERR_ADDRESS_NO_KEY:
         throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
-    }
-
-    bool fInvalid = false;
-    std::vector<unsigned char> vchSig = DecodeBase64(strSign.c_str(), &fInvalid);
-
-    if (fInvalid)
+    case MessageVerificationResult::ERR_MALFORMED_SIGNATURE:
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed base64 encoding");
-
-    CHashWriter ss(SER_GETHASH, 0);
-    ss << strMessageMagic;
-    ss << strMessage;
-
-    CPubKey pubkey;
-    if (!pubkey.RecoverCompact(ss.GetHash(), vchSig))
+    case MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED:
+    case MessageVerificationResult::ERR_NOT_SIGNED:
         return false;
+    case MessageVerificationResult::OK:
+        return true;
+    }
 
-    return (pubkey.GetID() == *pkhash);
+    return false;
 }
 
 static UniValue signmessagewithprivkey(const JSONRPCRequest& request)
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 42c2c50fa5..0b40df0b37 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -9,6 +9,7 @@
 #include <sync.h>
 #include <test/util/setup_common.h>
 #include <test/util/str.h>
+#include <util/message.h> // For MessageVerify()
 #include <util/moneystr.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -2025,4 +2026,56 @@ BOOST_AUTO_TEST_CASE(test_tracked_vector)
     BOOST_CHECK_EQUAL(v8[2].copies, 0);
 }
 
+BOOST_AUTO_TEST_CASE(message_verify)
+{
+    BOOST_CHECK_EQUAL(
+        MessageVerify(
+            "invalid address",
+            "signature should be irrelevant",
+            "message too"),
+        MessageVerificationResult::ERR_INVALID_ADDRESS);
+
+    BOOST_CHECK_EQUAL(
+        MessageVerify(
+            "3B5fQsEXEaV8v6U3ejYc8XaKXAkyQj2MjV",
+            "signature should be irrelevant",
+            "message too"),
+        MessageVerificationResult::ERR_ADDRESS_NO_KEY);
+
+    BOOST_CHECK_EQUAL(
+        MessageVerify(
+            "1KqbBpLy5FARmTPD4VZnDDpYjkUvkr82Pm",
+            "invalid signature, not in base64 encoding",
+            "message should be irrelevant"),
+        MessageVerificationResult::ERR_MALFORMED_SIGNATURE);
+
+    BOOST_CHECK_EQUAL(
+        MessageVerify(
+            "1KqbBpLy5FARmTPD4VZnDDpYjkUvkr82Pm",
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=",
+            "message should be irrelevant"),
+        MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED);
+
+    BOOST_CHECK_EQUAL(
+        MessageVerify(
+            "15CRxFdyRpGZLW9w8HnHvVduizdL5jKNbs",
+            "IPojfrX2dfPnH26UegfbGQQLrdK844DlHq5157/P6h57WyuS/Qsl+h/WSVGDF4MUi4rWSswW38oimDYfNNUBUOk=",
+            "I never signed this"),
+        MessageVerificationResult::ERR_NOT_SIGNED);
+
+    BOOST_CHECK_EQUAL(
+        MessageVerify(
+            "15CRxFdyRpGZLW9w8HnHvVduizdL5jKNbs",
+            "IPojfrX2dfPnH26UegfbGQQLrdK844DlHq5157/P6h57WyuS/Qsl+h/WSVGDF4MUi4rWSswW38oimDYfNNUBUOk=",
+            "Trust no one"),
+        MessageVerificationResult::OK);
+
+    BOOST_CHECK_EQUAL(
+        MessageVerify(
+            "11canuhp9X2NocwCq7xNrQYTmUgZAnLK3",
+            "IIcaIENoYW5jZWxsb3Igb24gYnJpbmsgb2Ygc2Vjb25kIGJhaWxvdXQgZm9yIGJhbmtzIAaHRtbCeDZINyavx14=",
+            "Trust me"),
+        MessageVerificationResult::OK);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/util/message.cpp b/src/util/message.cpp
new file mode 100644
index 0000000000..33e9c3384b
--- /dev/null
+++ b/src/util/message.cpp
@@ -0,0 +1,53 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <hash.h>            // For CHashWriter
+#include <key_io.h>          // For DecodeDestination()
+#include <pubkey.h>          // For CPubKey
+#include <script/standard.h> // For CTxDestination, IsValidDestination(), PKHash
+#include <serialize.h>       // For SER_GETHASH
+#include <util/message.h>
+#include <util/strencodings.h> // For DecodeBase64()
+
+#include <string>
+#include <vector>
+
+const std::string strMessageMagic = "Bitcoin Signed Message:\n";
+
+MessageVerificationResult MessageVerify(
+    const std::string& address,
+    const std::string& signature,
+    const std::string& message)
+{
+    CTxDestination destination = DecodeDestination(address);
+    if (!IsValidDestination(destination)) {
+        return MessageVerificationResult::ERR_INVALID_ADDRESS;
+    }
+
+    if (boost::get<PKHash>(&destination) == nullptr) {
+        return MessageVerificationResult::ERR_ADDRESS_NO_KEY;
+    }
+
+    bool invalid = false;
+    std::vector<unsigned char> signature_bytes = DecodeBase64(signature.c_str(), &invalid);
+    if (invalid) {
+        return MessageVerificationResult::ERR_MALFORMED_SIGNATURE;
+    }
+
+    CHashWriter ss(SER_GETHASH, 0);
+    ss << strMessageMagic;
+    ss << message;
+
+    CPubKey pubkey;
+    if (!pubkey.RecoverCompact(ss.GetHash(), signature_bytes)) {
+        return MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED;
+    }
+
+    if (!(CTxDestination(PKHash(pubkey)) == destination)) {
+        return MessageVerificationResult::ERR_NOT_SIGNED;
+    }
+
+    return MessageVerificationResult::OK;
+}
diff --git a/src/util/message.h b/src/util/message.h
new file mode 100644
index 0000000000..6adda5d3ff
--- /dev/null
+++ b/src/util/message.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_MESSAGE_H
+#define BITCOIN_UTIL_MESSAGE_H
+
+#include <string>
+
+extern const std::string strMessageMagic;
+
+/** The result of a signed message verification.
+ * Message verification takes as an input:
+ * - address (with whose private key the message is supposed to have been signed)
+ * - signature
+ * - message
+ */
+enum class MessageVerificationResult {
+    //! The provided address is invalid.
+    ERR_INVALID_ADDRESS,
+
+    //! The provided address is valid but does not refer to a public key.
+    ERR_ADDRESS_NO_KEY,
+
+    //! The provided signature couldn't be parsed (maybe invalid base64).
+    ERR_MALFORMED_SIGNATURE,
+
+    //! A public key could not be recovered from the provided signature and message.
+    ERR_PUBKEY_NOT_RECOVERED,
+
+    //! The message was not signed with the private key of the provided address.
+    ERR_NOT_SIGNED,
+
+    //! The message verification was successful.
+    OK
+};
+
+/** Verify a signed message.
+ * @param[in] address Signer's bitcoin address, it must refer to a public key.
+ * @param[in] signature The signature in base64 format.
+ * @param[in] message The message that was signed.
+ * @return result code */
+MessageVerificationResult MessageVerify(
+    const std::string& address,
+    const std::string& signature,
+    const std::string& message);
+
+#endif // BITCOIN_UTIL_MESSAGE_H
diff --git a/src/util/validation.cpp b/src/util/validation.cpp
index 89bc6665a4..ffbee21aeb 100644
--- a/src/util/validation.cpp
+++ b/src/util/validation.cpp
@@ -21,5 +21,3 @@ std::string FormatStateMessage(const ValidationState &state)
 
     return state.GetRejectReason();
 }
-
-const std::string strMessageMagic = "Bitcoin Signed Message:\n";
diff --git a/src/util/validation.h b/src/util/validation.h
index da2cf9f102..5ee260a055 100644
--- a/src/util/validation.h
+++ b/src/util/validation.h
@@ -13,6 +13,4 @@ class ValidationState;
 /** Convert ValidationState to a human-readable message for logging */
 std::string FormatStateMessage(const ValidationState &state);
 
-extern const std::string strMessageMagic;
-
 #endif // BITCOIN_UTIL_VALIDATION_H
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index e56125cdca..e139c69568 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -19,11 +19,11 @@
 #include <script/sign.h>
 #include <util/bip32.h>
 #include <util/fees.h>
+#include <util/message.h> // For strMessageMagic
 #include <util/moneystr.h>
 #include <util/string.h>
 #include <util/system.h>
 #include <util/url.h>
-#include <util/validation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/feebumper.h>
 #include <wallet/psbtwallet.h>
-- 
cgit v1.2.3