ethereum / EIPs

The Ethereum Improvement Proposal repository
https://eips.ethereum.org/
Creative Commons Zero v1.0 Universal
12.86k stars 5.27k forks source link

Make eth_sign human readable #185

Closed danfinlay closed 2 years ago

danfinlay commented 7 years ago

There is an increasing trend towards using eth_sign to prove account ownership for off-chain matters, such as website registration, and this calls for a version of eth_sign that ensures human readability.

Current favorite strategy: MIME-types for eth.sign

If clients agree on a format for including MIME headers in signed data, dapp developers could create styled text in the format of their choosing, and the client could have that text rendered correctly.

Another strategy: eth.signText

The web3.js framework could expose a method that is designed to sign a body of text that is shown to the user, rendered appropriately, for approval.

Implementation Basics

For normal client developers, implementation could be as simple as aliasing eth_signTerms to eth_sign. The method would only be useful or interesting to implement for UI signer developers, like Mist, MetaMask, or Parity UI, for whom it would be mostly just rendering the text correctly.

Originally posted here: https://github.com/ethereum/interfaces/issues/7

AlwaysBCoding commented 7 years ago

Functionality like this is definitely needed for app developers.

I'm curious though, is there a reason why this needs to be implemented as a change to the protocol instead of as a feature in MetaMask. For example couldn't MetaMask have it's own function that accepts a UTF string and displays it for the user, while actually signing the sha3 hash of that string under the hood, using the existing eth_sign function?

I guess my question is why would adding this functionality (which I agree is necessary) require a protocol change vs. application specific code on the part of MetaMask, Mist etc...

fjl commented 7 years ago

Maybe it could be called signText?

danfinlay commented 7 years ago

I wouldn't mind signText as a name.

@AlwaysBCoding MetaMask could definitely just add new methods whenever we want to the web3 object, but this would encourage dapp developers to create MetaMask-only compatible flows.

This is an offering of cross-client compatibility for a stronger ecosystem.

gavofyork commented 7 years ago

I tend to agree that a new RPC is unnecessary. Signer UIs ought to be checking with the user that they mean to sign these things anyway. The real question is how to optimise that check. I would suggest that we come up with a clear content format for what is being signed so that the content can be properly displayed to the user at the point of signing. Probably just copying e-mail MIME declarations is sufficient, e.g.:

MIME-Version: 1.0
Content-Type: text/markdown; charset=UTF-8; variant=Original

# Terms of Agreement
This is some content to be rendered to the user by the UI for signing...
frozeman commented 7 years ago

If we add a method i would agree with eth_signText to prevent accidental tx signing. But this can also be integrated in web3.js or other libraries as well. Not sure if a RPC method is necessary here.

I would also like to get a web3.js function for login into websites, by defining a standard what a website requires to let somebody in. E.g. siging a JSON:

{
   loginTarget: 'http://mywebsite.com'
   ip: ..,
   userAgent: ...
   timestamp: ...
   expiration:  60 * 60 * 24 //s
}

This way Dapps e.g. in Mist can be sure that the account is allowed to log in. Just relying on web3.eth.accounts to be present is not enough, as a browser could fake that. With a simply login message to be signed, the Dapp backend (if not blockchain based ;) ), can verify its really the intend of the owner, to log in.

MrTibbles commented 7 years ago

The whole ability surrounding secure login and logged in sessions is a debate in itself.

In regards to eth_signText || eth_signTerms, rather than a new RPC method, is this not more so a web3.js integration/update for UX, as @AlwaysBCoding outlines.

danfinlay commented 7 years ago

I would suggest that we come up with a clear content format for what is being signed so that the content can be properly displayed to the user at the point of signing. Probably just copying e-mail MIME declarations is sufficient

I actually really like this suggestion, and it's sufficient to negate the original intention of this EIP, replacing it with a formatting discussion, which I'm tempted to open in another EIP to avoid dual conversations.

This would benefit from collaboration between UI clients, so we could render the same format, but since this is backwards-compatible, there isn't really a downside to just going ahead with this unless two clients end up implementing it differently.

Also actually @MrTibbles and @AlwaysBCoding I've come around, this could be done at the web3.js layer, but what would you think of just an optional mime header for the encoded body?

pelle commented 7 years ago

I don't think it needs to be on the json-rpc level. Rather a standard for accepting terms is better implemented on the smart contract level.

I think it's important having a way of showing the terms and optionally accepting it is more important.

I've been meaning to create an EIP about this for a while based on what I suggest here: https://blog.stakeventures.com/articles/smart-contract-terms

juanfranblanco commented 7 years ago

Why not have different data schemas which can be encoded in RLP and signed the same way as transactions. You just need to agree to the data type structure. The first value could be the "Mime type" as thought by @gavofyork

Also the signing could be done anywhere RPC should not be necessary.

danfinlay commented 7 years ago

@pelle this proposal is merely talking about the eth_sign method, and how to show that in a human-readable way. Since it's an off-chain function, it's much more trivial, and I think it's a good warm-up for the more deeply integrated concerns of smart-contract readability that your blog post explores, and I can't wait for that EIP!

I'm redacting my request for any RPC changes here, this is a signer interface issue.

I'm going to update the top level issue to reflect the current state of the discussion, apologies to historians. Is that taboo?

github-actions[bot] commented 2 years ago

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

github-actions[bot] commented 2 years ago

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.