Low level signing method roadmap

eth_raw_sign.md

• low-level signing method
  • Things that need to be done
    • Consider this kind of raw signing from security. Ask [[Christian Lundkvist]]
    • Consider drafting an EIP for this, so it can get broader security consideration.
  • Code that would need to be changed
    • eth-json-rpc-middleware is a nice-to-change, although we could make a middleware just for this method.
    • It looks like eth-simple-keyring already signs arbitrary data, so the validation is not done at this level.
    • eth-keyring-controller also just passes through to the keyring, no validation.
    • MetaMask Controller
      • add a new message manager
        • This part is unfortunate because we're really due to DRY up the "message managers" here, we're probably refactoring this code soon. For now you would basically copy the messageManager, probably duplicate that whole controller.
      • add test for new message manager
      • export signing methods to UI
      • expose signing methods from message manager
      • add message manager to UI state
    • UI changes
      • This part of the UI was recently refactored so I'd have to do more work to revisit/figure out what's different here now.
      • You'd be cloning all the eth_sign code, basically.

According to the KeyRingController docs, the message must already be hashed.

The eth_sign method will receive the incoming data, alread hashed,
and must sign that hash, and then return the raw signed hash.

https://github.com/MetaMask/KeyringController/blob/0e2fdda6d237b4c2a8711719f27ca017285af317/docs/keyring.md#signmessageaddress-data

What are the chances that there are some dapps out there not hashing before they pass data in? Maybe there is a check for 32 bytes someplace?

Oh dear. Not hashing would be very bad.

On the bright side, this means we can already sign opaque Bitcoin transactions. I had assumed hashing was enforced 🤔

Okay. So I just verified experimentally that web3.eth.sign signs the raw digest already.

To reproduce:

> const cb = function (err, result) {
    if (err) return console.error(err)
    console.log('SIGNED:' + result.result)
  };

> ethereum.sendAsync({
  method: 'eth_sign',
  params: [myAddr, '0'.repeat(32)]
}, cb);

< SIGNED: 0x S R V

Then recovered that signature to the account ID using riemann-ether. This implies that we can already do what we want, provided that the MetaMask behavior doesn't change to match the geth behavior

placing this here for future reference. uses utils from bitcoin-spv

function srvToDER(srvStr) {
  let srv = '';
  if (srvStr.slice(0, 2) === '0x') {
    srv = srvStr.slice(2);
  } else {
    srv = srvStr;
  }
  let s = deserializeHex(safeSlice(srv, 0, 64));
  const r = deserializeHex(safeSlice(srv, 64, 128));

  // Trim to minimal encoding.
  // If there is a leading 0 and the next bit is 0, trim the lead.
  while (s[0] === 0 && s[1] & 0x80 !== 0) {
    s = safeSlice(s, 1);
  }
  while (r[0] === 0 && r[1] & 0x80 !== 0) {
    r = safeSlice(r, 1);
  }

  const encR = concatUint8Arrays(new Uint8Array([0x02, r.length]), r);
  const encS = concatUint8Arrays(new Uint8Array([0x02, s.length]), s)

  return concatUint8Arrays(
    new Uint8Array([0x30, encR.length + encS.length]),
    encR,
    encS
  );
}

What you are doing looks sane, but I didn't review thoroughly. Here are two more examples of r/s to der encoding:

• https://github.com/bcoin-org/bcrypto/blob/c6494387a88089a17d93a2108fb4c500267b3702/lib/js/ecdsa.js#L765
• https://github.com/bcoin-org/bcrypto/blob/a667e0cdd068c7a3ca27c96819f452ec1e272134/bench/deps/elliptic.js#L8634

Update:

Here is the first Bitcoin tx signed by metamask: https://blockstream.info/testnet/tx/44283c6120c08777ca0e9ce84ceb7b889209be47f6e2b7df112249aad7c88efb