Skip to content

Commit

Permalink
Deduplicate the message verifying code
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vasild committed Feb 14, 2020
1 parent 470664f commit 2ce3447
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 62 deletions.
2 changes: 2 additions & 0 deletions src/Makefile.am
Expand Up @@ -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 \
Expand Down Expand Up @@ -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 \
Expand Down
78 changes: 42 additions & 36 deletions src/qt/signverifymessagedialog.cpp
Expand Up @@ -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>
Expand Down Expand Up @@ -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()
Expand Down
32 changes: 11 additions & 21 deletions src/rpc/misc.cpp
Expand Up @@ -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>
Expand Down Expand Up @@ -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)
Expand Down
53 changes: 53 additions & 0 deletions src/test/util_tests.cpp
Expand Up @@ -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>
Expand Down Expand Up @@ -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()
53 changes: 53 additions & 0 deletions 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;
}
49 changes: 49 additions & 0 deletions 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
2 changes: 0 additions & 2 deletions src/util/validation.cpp
Expand Up @@ -21,5 +21,3 @@ std::string FormatStateMessage(const ValidationState &state)

return state.GetRejectReason();
}

const std::string strMessageMagic = "Bitcoin Signed Message:\n";
2 changes: 0 additions & 2 deletions src/util/validation.h
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/wallet/rpcwallet.cpp
Expand Up @@ -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>
Expand Down

0 comments on commit 2ce3447

Please sign in to comment.