Skip to content

Commit

Permalink
Refactor message hashing into a utility function
Browse files Browse the repository at this point in the history
And add unit test for it.

The purpose of using a preamble or "magic" text as part of signing and
verifying a message was not given when the code was repeated in a few
locations. Make a test showing how it is used to prevent inadvertently
signing a transaction.
  • Loading branch information
jkczyz authored and vasild committed Feb 14, 2020
1 parent f8f0d98 commit e193a84
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
21 changes: 20 additions & 1 deletion src/test/util_tests.cpp
Expand Up @@ -5,12 +5,14 @@
#include <util/system.h>

#include <clientversion.h>
#include <hash.h> // For Hash()
#include <key.h> // For CKey
#include <optional.h>
#include <sync.h>
#include <test/util/setup_common.h>
#include <test/util/str.h>
#include <util/message.h> // For MessageSign(), MessageVerify()
#include <uint256.h>
#include <util/message.h> // For MessageSign(), MessageVerify(), MESSAGE_MAGIC
#include <util/moneystr.h>
#include <util/strencodings.h>
#include <util/string.h>
Expand Down Expand Up @@ -2116,4 +2118,21 @@ BOOST_AUTO_TEST_CASE(message_verify)
MessageVerificationResult::OK);
}

BOOST_AUTO_TEST_CASE(message_hash)
{
const std::string unsigned_tx = "...";
const std::string prefixed_message =
std::string(1, (char)MESSAGE_MAGIC.length()) +
MESSAGE_MAGIC +
std::string(1, (char)unsigned_tx.length()) +
unsigned_tx;

const uint256 signature_hash = Hash(unsigned_tx.begin(), unsigned_tx.end());
const uint256 message_hash1 = Hash(prefixed_message.begin(), prefixed_message.end());
const uint256 message_hash2 = MessageHash(unsigned_tx);

BOOST_CHECK_EQUAL(message_hash1, message_hash2);
BOOST_CHECK_NE(message_hash1, signature_hash);
}

BOOST_AUTO_TEST_SUITE_END()
26 changes: 15 additions & 11 deletions src/util/message.cpp
Expand Up @@ -15,7 +15,11 @@
#include <string>
#include <vector>

const std::string strMessageMagic = "Bitcoin Signed Message:\n";
/**
* Text used to signify that a signed message follows and to prevent
* inadvertently signing a transaction.
*/
const std::string MESSAGE_MAGIC = "Bitcoin Signed Message:\n";

MessageVerificationResult MessageVerify(
const std::string& address,
Expand All @@ -37,12 +41,8 @@ MessageVerificationResult MessageVerify(
return MessageVerificationResult::ERR_MALFORMED_SIGNATURE;
}

CHashWriter ss(SER_GETHASH, 0);
ss << strMessageMagic;
ss << message;

CPubKey pubkey;
if (!pubkey.RecoverCompact(ss.GetHash(), signature_bytes)) {
if (!pubkey.RecoverCompact(MessageHash(message), signature_bytes)) {
return MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED;
}

Expand All @@ -58,17 +58,21 @@ bool MessageSign(
const std::string& message,
std::string& signature)
{
CHashWriter ss(SER_GETHASH, 0);
ss << strMessageMagic;
ss << message;

std::vector<unsigned char> signature_bytes;

if (!privkey.SignCompact(ss.GetHash(), signature_bytes)) {
if (!privkey.SignCompact(MessageHash(message), signature_bytes)) {
return false;
}

signature = EncodeBase64(signature_bytes.data(), signature_bytes.size());

return true;
}

uint256 MessageHash(const std::string& message)
{
CHashWriter hasher(SER_GETHASH, 0);
hasher << MESSAGE_MAGIC << message;

return hasher.GetHash();
}
9 changes: 8 additions & 1 deletion src/util/message.h
Expand Up @@ -7,10 +7,11 @@
#define BITCOIN_UTIL_MESSAGE_H

#include <key.h> // For CKey
#include <uint256.h>

#include <string>

extern const std::string strMessageMagic;
extern const std::string MESSAGE_MAGIC;

/** The result of a signed message verification.
* Message verification takes as an input:
Expand Down Expand Up @@ -58,4 +59,10 @@ bool MessageSign(
const std::string& message,
std::string& signature);

/**
* Hashes a message for signing and verification in a manner that prevents
* inadvertently signing a transaction.
*/
uint256 MessageHash(const std::string& message);

#endif // BITCOIN_UTIL_MESSAGE_H

0 comments on commit e193a84

Please sign in to comment.