Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trouble validating signatures from MetaMask #18

Closed
hstove opened this issue Mar 20, 2018 · 15 comments
Closed

Trouble validating signatures from MetaMask #18

hstove opened this issue Mar 20, 2018 · 15 comments

Comments

@hstove
Copy link

hstove commented Mar 20, 2018

Hey there, thanks for the project. It's been super helpful.

What I'm trying to do is validate a signed message that I can get from MetaMask. Getting the signature is easy. Let's say I've signed the message "numa", and I get the signature 0x641a251cce30223cbcf391ecfe3aa939acc7361bbaf166dd929f940bf6589f8b3a2defc1b19c5db7f5816cc83a07a1be5a452e0387491a52e3b50c5138b7acb81c. Here is the code that I think would work:

Eth.configure { |e| e.chain_id = 1 }
message_hash = Eth::Utils.keccak256("numa")
signature = Eth::Utils.hex_to_bin("0x641a251cce30223cbcf391ecfe3aa939acc7361bbaf166dd929f940bf6589f8b3a2defc1b19c5db7f5816cc83a07a1be5a452e0387491a52e3b50c5138b7acb81c")
public_hex = Eth::OpenSsl.recover_compact(message_hash, signature)

But public_hex is nil. After debugging, I'm finding that the BN_cmp(x, field) >= 0 flag is caught, which is why it returns nil. Unfortunately I don't really understand what this aspect of the code is doing. I'd really appreciate any help, if possible.

Thanks!

@hstove
Copy link
Author

hstove commented Mar 20, 2018

In case others run into this as well, I'm able to use node.js to do this, which may be a viable workaround for some people.

const message = utils.hashPersonalMessage(utils.toBuffer('numa'));

const sgn = "0x641a251cce30223cbcf391ecfe3aa939acc7361bbaf166dd929f940bf6589f8b3a2defc1b19c5db7f5816cc83a07a1be5a452e0387491a52e3b50c5138b7acb81c";

const r = utils.toBuffer(sgn.slice(0, 66));
const s = utils.toBuffer('0x' + sgn.slice(66, 130));
const v = utils.bufferToInt(utils.toBuffer('0x' + sgn.slice(130, 132)));
const pub = utils.ecrecover(message, v, r, s);
const addr = utils.pubToAddress(pub);

console.log(utils.bufferToHex(addr));

@shrugs
Copy link

shrugs commented May 10, 2018

I'd prefer not to use javascript for this part of my system so this is rather confusing to me. According to @se3000 's comment here we should be able to recover a signature generated by something like metamask (which afaik uses libsecp).

Here's the code I'm running. I also happen to be using personal_sign, which forces the Ethereum Signed Message prefix.

original_message = 'test'
prefix = "#{Eth::Utils.hex_to_bin('19')}Ethereum Signed Message:\n#{original_message.length}"
personal_message_hash = Eth::Utils.keccak256("#{prefix}#{original_message}")
personal_message_hash_hex = Eth::Utils.bin_to_hex(personal_message_hash)
# -> 4a5c5d454721bbbb25540c3317521e71c373ae36458f960d2ad46ef088110e95
# (which is the same data that metamask signs, checked by breakpoints)

Eth::OpenSsl.recover_compact(
    personal_message_hash_hex,
    signature
)
# false

Any advice/help would be appreciated :)

@se3000
Copy link
Owner

se3000 commented Jun 7, 2018

@shrugs @hstove thanks for reporting. Can you post an example with a signature, a message hash, and the chain ID? Thank you.

@ngan
Copy link

ngan commented Jun 13, 2018

@se3000 Here's an example:

On Metamask:

> web3.personal.sign(web3.fromUtf8("hello"), "0x101b17d50c01B555CCb7534d46CAaCa70D0B9B55", console.log)

// Signature:
"0x7b706331232925682dfa46695914095243eacd65f75c21e0ee3179c55c6cf9e45465b23716aba9eb1ccc36905732a147bc8d7bdebb78e4978d6558e11d69f0781c"

Then, on ruby console:

>> key = Eth::Key.new(priv: "510c561d3d79ab0b80e60310929bdf9ff96200658b036b1a81ae5852e426e343")
>> key.address
=> "0x101b17d50c01B555CCb7534d46CAaCa70D0B9B55"
>> key.verify_signature("hello", Eth::Utils.hex_to_bin("0x7b706331232925682dfa46695914095243eacd65f75c21e0ee3179c55c6cf9e45465b23716aba9eb1ccc36905732a147bc8d7bdebb78e4978d6558e11d69f0781c"))
=> false

This is Chain ID 1 (mainnet)

@buhrmi
Copy link
Contributor

buhrmi commented Jun 13, 2018

Hello, having the same issue. When signing a personal message with metamask, a prefix is applied, which is missing in the example provided by @ngan. The example below should be correct, but still unable to verify the signature from metamask. I can't figure out, why...

In a browser with metamask installed:

> web3.personal.sign(web3.fromUtf8("test"), "0x101b17d50c01B555CCb7534d46CAaCa70D0B9B55", console.log)

// Signature:
"0x3eb24bd327df8c2b614c3f652ec86efe13aa721daf203820241c44861a26d37f2bffc6e03e68fc4c3d8d967054c9cb230ed34339b12ef89d512b42ae5bf8c2ae1c"

In ruby console:

msg = 'test'
prefix = "#{Eth::Utils.hex_to_bin('19')}Ethereum Signed Message:\n#{msg.length}"
message_hash = Eth::Utils.keccak256("#{prefix}#{msg}")
key = Eth::Key.new(priv: "510c561d3d79ab0b80e60310929bdf9ff96200658b036b1a81ae5852e426e343")
Eth::OpenSsl.recover_compact(message_hash, Eth::Utils.hex_to_bin("0x3eb24bd327df8c2b614c3f652ec86efe13aa721daf203820241c44861a26d37f2bffc6e03e68fc4c3d8d967054c9cb230ed34339b12ef89d512b42ae5bf8c2ae1c"))
=> nil

Any help would be appreciated...

@buhrmi
Copy link
Contributor

buhrmi commented Jun 30, 2018

Hey guys I'm offering a 1 eth bounty if somebody can help me figure this out

@dagi3d
Copy link

dagi3d commented Jul 6, 2018

We recently faced a similar issue and it looks we found a solution that seems to work.
In our case we wanted to check that the signature generated in backend with this library was valid while trying to verify it in our app that talks to the eth node.
The error we got was this:

# ruby
key       = Eth::Key.new(priv: 'a82645680175738c6d8e64adcbc14edbfab76797c5254e30900266a775d2b921')
msg       = 'test'
prefix    = "\x19Ethereum Signed Message:\n#{msg.length}"
puts Eth::Utils.bin_to_prefixed_hex(key.sign(msg))
// geth console
web3.personal.importRawKey("a82645680175738c6d8e64adcbc14edbfab76797c5254e30900266a775d2b921", "")
web3.personal.ecRecover(web3.fromAscii("test"), "0x1cf88a9cb3d7e55dd7b4522eb6a33c2ab54aa92b229e39a4c5bd36339c3573dd6929ed9d84832be71bf820827c39a5bd84c628d0c328670e6c7f96899652f2a279")
Error: invalid Ethereum signature (V is not 27 or 28)

We realized that the V value was being checked against the last signature's byte and for some reason, with this library that value was added in the first byte instead of the last one, so it was just a matter of moving it to the end:

module Eth
   class Key
     alias_method :orig_sign, :sign

     def sign(message)
       message = "\x19Ethereum Signed Message:\n#{message.length}#{message}"
       orig_sign(message).bytes.rotate(1).pack('c*')
     end
   end
 end

Now, we can sign the message again and pass it back to the geth console:

> web3.personal.ecRecover(web3.fromAscii("test"), "0x7a682587a5ae85b7648c8e0e8429bde40b9042d87433a8f8d91d0dc3dbca14b35e239a0bf2a8befd3ddffc4fb055d48ea2b89b22f886bdb7c062fd42c1a639c71c")
"0x79a600fff02fe4933a5f33aa2bc725c6179e79e0"

So, if you want to check it the other way round, to generate the signature from the geth console(and I assume that doing it from Metamask should work too), it's just a matter of moving the signature's V byte to the beginning:

 module Eth
   class Key
     alias_method :orig_verify_signature, :verify_signature

     def verify_signature(message, signature)
       message = "\x19Ethereum Signed Message:\n#{message.length}#{message}"
       orig_verify_signature(message, Eth::Utils.hex_to_bin(signature).bytes.rotate(-1).pack('c*'))
     end
   end
 end
// geth console
> eth.sign(eth.accounts[0], web3.fromUtf8("test"))
"0xbf70c5b8ad3ac99b2220ef9a9a2fcb02ab5361a65219967a0f5ddf66025f070d7bcb027c258e1d3bee4b960f3c2366ae3721fbca59a08ef935a49859ed28a8651b"
key       = Eth::Key.new(priv: 'a82645680175738c6d8e64adcbc14edbfab76797c5254e30900266a775d2b921')
msg       = 'test'
signature = "0xbf70c5b8ad3ac99b2220ef9a9a2fcb02ab5361a65219967a0f5ddf66025f070d7bcb027c258e1d3bee4b960f3c2366ae3721fbca59a08ef935a49859ed28a8651b"
 puts key.verify_signature(msg, signature) # => true

Hope it makes sense and we are not forgetting anything critical

@buhrmi if the 1 ETH bounty is still on the table, happy to receive it and share it with the team: 0x99B8ff31aa0Ef61Fa79c96Ea947b1ACdA209605f :)

@buhrmi
Copy link
Contributor

buhrmi commented Jul 7, 2018

Awesome @dagi3d you are freaking geniuses. I sent the eth :D

@se3000 should get some too for making this gem in the first place. Do you have a donation address?

@se3000
Copy link
Owner

se3000 commented Jul 9, 2018

@dagi3d @buhrmi Awesome, thanks for figuring this out.

So far we return and expect signatures in the format V,R,S. This is how they are ordered in serialized Ethereum transactions(see page 4 of the yellow paper). V is that first byte that you move to the end. You can see in the EIP 191 spec that for some reason the signature is expected as R,S,V. Looking into the discussion around 191, it seems like this signature ordering was to fit with geth, but it's unclear why geth made that choice to begin with.

I'm a bit swamped at the moment, and won't be able to get to this anytime soon. Anyone interested in turning this into a PR?

@se3000
Copy link
Owner

se3000 commented Jul 9, 2018

@buhrmi Thanks, that's very generous of you. You can send donations to 0xc021b4474F665a0b1C7A68ae0C5c723d08d69185. I'll put anything sent there towards a bounty for a native libsecp256k1 gem. I think the ruby community shouldn't have to rely on OpenSSL for crypto related work long term, and a native libsecp gem would is our best option as far as I can tell.

Anyway, I'll try to get more details on the bounty together in the future. Thanks again!

@buhrmi
Copy link
Contributor

buhrmi commented Jul 9, 2018

Does it make sense to update the sign method to use a different byte order? I have not tried it, but I would assume that geth accepts transactions signed with the current implementation. So changing the byte order in the sign method would break that, no?

How about adding personal_sign and personal_recover to the library?

@se3000
Copy link
Owner

se3000 commented Jul 16, 2018

@buhrmi yes, I think personal_sign and personal_recover would be best. They definitely need to be different because ordering of the signature parts and they do not take chain_id into account.

@se3000
Copy link
Owner

se3000 commented Aug 5, 2018

There is now Eth::Key#personal_sign and Eth::Key.personal_recover. Thanks again @buhrmi!

@sofianeOuafir
Copy link

@se3000 what does personal_recover do?

@q9f
Copy link
Collaborator

q9f commented Sep 22, 2021

@se3000 what does personal_recover do?

It returns the public key that signed the message.

# signer information
signer_address = "0xe611a720778a5f6723d6b4866F84828504657181"

# message and signatre (from javascript/metamask)
message = "Hello from Rails! 07b54ba3-fe2b-4932-8723-52e6c1d109c1"
signature = "0x03a45db0ec6cada40f0227bb67cfd99e69a92c5c6809450d4db884c83b41401c062c039f1465012965ec2426c0dd2d4d3737cf08009866f9a3bccfa3f312bda81c"

# recover the public key from the signature
recovered_public_key = Eth::Key.personal_recover message, signature

# recover the address from the public key
recovered_address = Eth::Utils.public_key_to_address recovered_public_key

# are they the same? huge, if true ;)
p signer_address
p recovered_address
p signer_address.eql? recovered_address

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants