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

Make eth_sign human readable #185

Closed
danfinlay opened this issue Dec 15, 2016 · 12 comments
Closed

Make eth_sign human readable #185

danfinlay opened this issue Dec 15, 2016 · 12 comments
Labels

Comments

@danfinlay
Copy link
Contributor

danfinlay commented Dec 15, 2016

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: ethereum/interfaces#7

@AlwaysBCoding
Copy link

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
Copy link
Contributor

fjl commented Dec 15, 2016

Maybe it could be called signText?

@danfinlay
Copy link
Contributor Author

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
Copy link

gavofyork commented Dec 16, 2016

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
Copy link
Contributor

frozeman commented Dec 16, 2016

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.

@frozeman frozeman added the ERC label Dec 16, 2016
@MrTibbles
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

pelle commented Dec 16, 2016

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
Copy link

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
Copy link
Contributor Author

@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?

@danfinlay danfinlay changed the title Add new method eth_signTerms Make eth_sign human readable Dec 16, 2016
@github-actions
Copy link

github-actions bot commented Jan 3, 2022

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 github-actions bot added the stale label Jan 3, 2022
@github-actions
Copy link

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.

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

No branches or pull requests

10 participants