rpc: fix sign message. fixes #460

lib/node/rpc.js

@@ -13,10 +13,10 @@ const IP = require('binet');
 const Validator = require('bval');
 const {BufferMap, BufferSet} = require('buffer-map');
 const hash160 = require('bcrypto/lib/hash160');
-const hash256 = require('bcrypto/lib/hash256');
 const {safeEqual} = require('bcrypto/lib/safe');
 const secp256k1 = require('bcrypto/lib/secp256k1');
 const util = require('../utils/util');
+const messageUtil = require('../utils/message');
 const common = require('../blockchain/common');
 const Amount = require('../btc/amount');
 const NetAddress = require('../net/netaddress');
@@ -73,8 +73,6 @@ const errs = {
   CLIENT_P2P_DISABLED: -31
 };
 
-const MAGIC_STRING = 'Bitcoin Signed Message:\n';
-
 /**
  * Bitcoin RPC
  * @alias module:http.RPC
@@ -2107,15 +2105,12 @@ class RPC extends RPCBase {
       throw new RPCError(errs.TYPE_ERROR, 'Invalid parameters.');
 
     const addr = parseAddress(b58, this.network);
-    const msg = Buffer.from(MAGIC_STRING + str, 'utf8');
-    const hash = hash256.digest(msg);
-
-    const key = secp256k1.recoverDER(hash, sig, 0, true);
+    const key = messageUtil.recover(str, sig);
 
     if (!key)
       return false;
 
-    return safeEqual(hash160.digest(key), addr.hash);
+    return safeEqual(hash160.digest(key), addr.hash) === 1;
   }
 
   async signMessageWithPrivkey(args, help) {
@@ -2129,9 +2124,7 @@ class RPC extends RPCBase {
     const str = valid.str(1, '');
 
     const key = parseSecret(wif, this.network);
-    const msg = Buffer.from(MAGIC_STRING + str, 'utf8');
-    const hash = hash256.digest(msg);
-    const sig = key.sign(hash);
+    const sig = messageUtil.sign(str, key);
 
     return sig.toString('base64');
   }

lib/utils/message.js

@@ -0,0 +1,95 @@
+/*!
+ * message.js - message signing utilities.
+ * Copyright (c) 2019, The Bcoin Developers (MIT License).
+ */
+
+'use strict';
+
+const assert = require('bsert');
+const bufio = require('bufio');
+const hash256 = require('bcrypto/lib/hash256');
+const secp256k1 = require('bcrypto/lib/secp256k1');
+
+/**
+ * @exports utils/message
+ */
+
+const message = exports;
+
+/**
+ * Bitcoin signing magic string.
+ * @const {String}
+ * @default
+ */
+
+message.MAGIC_STRING = 'Bitcoin Signed Message:\n';
+
+/**
+ * Hash message with magic string.
+ * @param {String} message
+ * @param {String} [prefix = message.MAGIC_STRING]
+ * @returns {Hash}
+ */
+
+message.magicHash = (msg, prefix = message.MAGIC_STRING) => {
+  assert(typeof prefix === 'string', 'prefix must be a string.');
+  assert(typeof msg === 'string', 'message must be a string');
+
+  const bw = bufio.write();
+
+  bw.writeVarString(prefix);
+  bw.writeVarString(msg, 'utf8');
+
+  return hash256.digest(bw.render());
+};
+
+/**
+ * Sign message with key.
+ * @param {String} msg
+ * @param {KeyRing} ring
+ * @param {String} [prefix = message.MAGIC_STRING]
+ * @returns {Buffer}
+ */
+
+message.sign = (msg, ring, prefix) => {
+  assert(ring.getPrivateKey(), 'Cannot sign without private key.');
+
+  const hash = message.magicHash(msg, prefix);
+  const compress = 0x04 !== ring.getPublicKey().readInt8(0);
+  const {
+    signature,
+    recovery
+  } = secp256k1.signRecoverable(hash, ring.getPrivateKey());
+
+  const bw = bufio.write();
+
+  bw.writeI8(recovery + 27 + (compress ? 4 : 0));
+  bw.writeBytes(signature);
+
+  return bw.render();
+};
+
+/**
+ * Recover raw public key from message and signature.
+ * @param {String} msg
+ * @param {Buffer} signature
+ * @param {String} [prefix = MAGIC_STRING]
+ */
+
+message.recover = (msg, signature, prefix) => {
+  assert(typeof msg === 'string', 'msg must be a string');
+  assert(Buffer.isBuffer(signature), 'sig must be a buffer');
+
+  const hash = message.magicHash(msg, prefix);
+
+  assert.strictEqual(signature.length, 65, 'Invalid signature length');
+
+  const flagByte = signature.readUInt8(0) - 27;
+
+  assert(flagByte < 8, 'Invalid signature parameter');
+
+  const compressed = Boolean(flagByte & 4);
+  const recovery = flagByte & 3;
+
+  return secp256k1.recover(hash, signature.slice(1), recovery, compressed);
+};

lib/wallet/rpc.js

@@ -12,9 +12,9 @@ const bweb = require('bweb');
 const {Lock} = require('bmutex');
 const fs = require('bfile');
 const Validator = require('bval');
-const hash256 = require('bcrypto/lib/hash256');
 const {BufferMap, BufferSet} = require('buffer-map');
 const util = require('../utils/util');
+const messageUtil = require('../utils/message');
 const Amount = require('../btc/amount');
 const Script = require('../script/script');
 const Address = require('../primitives/address');
@@ -68,8 +68,6 @@ const errs = {
   WALLET_ALREADY_UNLOCKED: -17
 };
 
-const MAGIC_STRING = 'Bitcoin Signed Message:\n';
-
 /**
  * Wallet RPC
  * @alias module:wallet.RPC
@@ -1519,10 +1517,7 @@ class RPC extends RPCBase {
     if (!wallet.master.key)
       throw new RPCError(errs.WALLET_UNLOCK_NEEDED, 'Wallet is locked.');
 
-    const msg = Buffer.from(MAGIC_STRING + str, 'utf8');
-    const hash = hash256.digest(msg);
-
-    const sig = ring.sign(hash);
+    const sig = messageUtil.sign(str, ring);
 
     return sig.toString('base64');
   }

test/rpc-test.js

@@ -7,6 +7,7 @@ const assert = require('bsert');
 const consensus = require('../lib/protocol/consensus');
 const Address = require('../lib/primitives/address');
 const FullNode = require('../lib/node/fullnode');
+const KeyRing = require('../lib/primitives/keyring');
 
 const ports = {
   p2p: 49331,
@@ -47,33 +48,26 @@ const defaultCoinbaseMaturity = consensus.COINBASE_MATURITY;
 let addressHot = null;
 let addressMiner = null;
 let walletHot = null;
-let walletMiner = null;
 let blocks = null;
 let txid = null;
 let utxo = null;
 
 describe('RPC', function() {
   this.timeout(15000);
 
-  before(() => {
+  before(async () => {
     consensus.COINBASE_MATURITY = 0;
-  });
-
-  after(() => {
-    consensus.COINBASE_MATURITY = defaultCoinbaseMaturity;
-  });
-
-  it('should open node and create wallets', async () => {
     await node.open();
-    await nclient.open();
-    await wclient.open();
 
     const walletHotInfo = await wclient.createWallet('hot');
     walletHot = wclient.wallet('hot', walletHotInfo.token);
-    const walletMinerInfo = await wclient.createWallet('miner');
-    walletMiner = wclient.wallet('miner', walletMinerInfo.token);
-    await walletHot.open();
-    await walletMiner.open();
+
+    await wclient.createWallet('miner');
+  });
+
+  after(async () => {
+    consensus.COINBASE_MATURITY = defaultCoinbaseMaturity;
+    await node.close();
   });
 
   it('should rpc help', async () => {
@@ -100,6 +94,120 @@ describe('RPC', function() {
     assert.strictEqual(info.blocks, 0);
   });
 
+  describe('signmessagewithprivkey', function () {
+    const message = 'This is just a test message';
+    const privKeyWIF = 'cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N';
+    const ring = KeyRing.fromSecret(privKeyWIF, 'regtest');
+
+    const expectedSignature = 'INbVnW4e6PeRmsv2Qgu8NuopvrVjkcxob+sX8OcZG0SALh'
+      + 'WybUjzMLPdAsXI46YZGb0KQTRii+wWIQzRpG/U+S0=';
+
+    it('should sign message', async () => {
+      const sig = await nclient.execute('signmessagewithprivkey', [
+        privKeyWIF,
+        message
+      ]);
+
+      assert.equal(sig, expectedSignature);
+    });
+
+    it('should fail on invalid privkey', async () => {
+      const privKey = 'invalid priv key';
+      let err;
+
+      try {
+        await nclient.execute('signmessagewithprivkey', [
+          privKey,
+          message
+        ]);
+      } catch (e) {
+        err = e;
+      }
+
+      assert.ok(err);
+      assert.strictEqual(err.message, 'Invalid key.');
+    });
+
+    it('should fail on wrong network privkey', async () => {
+      const privKeyWIF = ring.toSecret('main');
+
+      let err;
+
+      try {
+        await nclient.execute('signmessagewithprivkey', [
+          privKeyWIF,
+          message
+        ]);
+      } catch (e) {
+        err = e;
+      }
+
+      assert.ok(err);
+      assert.strictEqual(err.message, 'Invalid key.');
+    });
+  });
+
+  describe('verifymessage', function() {
+    const message = 'This is just a test message';
+    const address = 'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB';
+    const signature = 'INbVnW4e6PeRmsv2Qgu8NuopvrVjkcxob+sX8OcZG0SALh'
+      + 'WybUjzMLPdAsXI46YZGb0KQTRii+wWIQzRpG/U+S0=';
+
+    it('should verify correct signature', async () => {
+      const result = await nclient.execute('verifymessage', [
+        address,
+        signature,
+        message
+      ]);
+
+      assert.equal(result, true);
+    });
+
+    it('should verify invalid signature', async () => {
+      const result = await nclient.execute('verifymessage', [
+        address,
+        signature,
+        'different message.'
+      ]);
+
+      assert.equal(result, false);
+    });
+
+    it('should fail on invalid address', async () => {
+      let err;
+
+      try {
+        await nclient.execute('verifymessage', [
+          'Invalid address',
+          signature,
+          message
+        ]);
+      } catch (e) {
+        err = e;
+      }
+
+      assert.ok(err);
+      assert.strictEqual(err.message, 'Invalid address.');
+    });
+
+    it('should fail on invalid signature', async () => {
+      let err;
+
+      try {
+        await nclient.execute('verifymessage', [
+          address,
+          '.',
+          message
+        ]);
+      } catch (e) {
+        err = e;
+      }
+
+      assert.ok(err);
+      assert.strictEqual(err.message, 'Invalid signature length');
+    });
+  });
+
   it('should rpc selectwallet', async () => {
     const response = await wclient.execute('selectwallet', ['miner']);
     assert.strictEqual(response, null);
@@ -269,12 +377,4 @@ describe('RPC', function() {
       message: 'Block not found.'
     });
   });
-
-  it('should cleanup', async () => {
-    await walletHot.close();
-    await walletMiner.close();
-    await wclient.close();
-    await nclient.close();
-    await node.close();
-  });
 });