- 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
- add a new message manager
- 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_signcode, basically.
- Things that need to be done
Created
March 9, 2020 19:17
-
-
Save danfinlay/33afe20a4ddb5c40bc899b5ddb99ff78 to your computer and use it in GitHub Desktop.
Low level signing method roadmap
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.
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:
Update:
Here is the first Bitcoin tx signed by metamask: https://blockstream.info/testnet/tx/44283c6120c08777ca0e9ce84ceb7b889209be47f6e2b7df112249aad7c88efb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Okay, from some research:
The message is passed to the keyring controller here
Where is the message data hashed as part of this process?