[Bug]: Why eth_sign doesn't allow an arbitrary length bytes message

[frozeman]

Describe the bug

According to the RPC specs of ethereum, we allow arbitrary length bytes to be signed, and the hashing of the bytes should happen WITHIN the signer.
https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_sign

Meta mask though expects a 32 bytes hash, which is not correct. (Even the example we put in the RPC docs 0xdeadbeaf is not working)

Why choosing to make this sign endpoint non-standard?

Also it will make it impossible to see the content of a message, if its for example a UTF8 string. Which also makes SIWE impossible by default.

Steps to reproduce

Call eth_sign with [address, n length bytes]

Error messages or log output

-

Version

latest

Build type

No response

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

[bschorchit]

Hey @frozeman, thank you for your report! I'm looping in our dx team (cc: @vandan) to triage that, but I'm curious on why you are looking to use eth_sign if you don't mind sharing.
We usually recommend using signTypedData_v4 as it's most readable signature method for users which allows them to better understand what they are signing. eth_sign is not really readable, can impersonate transaction, be dangerous to unaware users and we flag them as so in the UI.
Also, to implement SIWE, dapps are currently using personal_sign.

[Hugoo]

Hi @bschorchit thanks for the prompt reply.

Here are more details.
But regardless of the use case, it is more about trying to understand if (and why) it was implemented differently.

From MetaMask docs

In particular, the method eth_sign is an open-ended signing method that allows signing an arbitrary hash, which means it can be used to sign transactions, or any other data, making it a dangerous phishing risk.

-> signs a hash (i.e. 32bytes expected as described by error above)

From Ethereum Docs

-> signs a N Bytes long message

[JeneaVranceanu]

To add more context to the issue: the implementation of eth_sign up until October 2016 was indeed expecting a hash for signing, and it looks like this is the behaviour that MetaMask is exhibiting still.
After that in this PR - ethereum/go-ethereum#2940 - eth_sign was changed and personal_sign was introduced. PR is just an example of how eth_sign changed.

Since that PR, at least in go-ethereum, eth_sign expects address and an unlimited number of bytes.
personal_sign does the same but also expects an optional password to decrypt the private key before attempting the signing.

The version of eth_sign that expects a hash was simply replaced.

[bschorchit]

Thank you for the context and additional info!
So far we haven't updated eth_sign to the new standard to avoid breaking changes for existing users of such method in its original format.
Our implementation of personal_sign though should match exactly the expectations you have for the updated eth_sign. It does not expect an optional password to decrypt the private key before attempting the signing (this might only be applicable in the go-ethereum context).
Any reason why it might be preferred to not use personal_sign for your use case and instead use an updated version of eth_sign in MetaMask? Keen to learn more about it.

[frozeman]

@bschorchit to give a bit more context.
We are building a new browser extension that uses a new set of smart contract standards as accounts, rather than just EOAs.
We were wondering why meta mask didnt implement eth_sign properly. Thats why this post.
the addition of personal_sign was made in 2016 (!), as well as the change to an arbitrary message using the "Ethereum Signed Messages" prefix.

personal_sign is not a replacement for eth_sign, as first of the personal_ endpoints are not officially standardised (https://ethereum.org/en/developers/docs/apis/json-rpc/), and second they expect a password as a last parameter, which obviously the user doesn't have/need.

We (I was part of the geth team, at that time) introduced the personal_sign endpoint in geth back in 2016 to make sure that we dont need to unlock accounts before using them via the console, and to prevent people from accidentally keeping it unlocked with an open HTTP RPC.

As for signTypedData_v4, its a standard we are likely not integrating into our browser extension, as its overly complicated.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days. Thank you for your contributions.

[github-actions]

This issue was closed because there has been no follow up activity in the last 45 days. If you feel this was closed in error, please reopen and provide evidence on the latest release of the extension. Thank you for your contributions.