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

Be signed out per domain until signing in #714

Open
danfinlay opened this Issue Oct 7, 2016 · 22 comments

Comments

Projects
None yet
8 participants
@danfinlay
Copy link
Contributor

danfinlay commented Oct 7, 2016

Updated March 21, 2018, to reflect latest plans for this feature's implementation:

MetaMask currently shows the selected account to each site that is visited. This is not an acceptable privacy feature, and user adoption has escalated the urgency of addressing this issue.

It has also been also criticized that the web3 API is visible at all, even to sites a user did not explicitly show it to, since this also provides some identifying information.

Goals/Constraints

  • A user's accounts should never be shown to a site without the user's deliberate consent.
  • web3 should not be exposed to sites by default.
  • Dapps should have a method of requesting a user's account & the web3 API.
  • Old Dapps should continue to work.

Proposal: A PostMessage Based Log-In

A difficult challenge of the constraints above is simultaneously not providing the web3 API while also allowing sites to request a user to sign in. At first I thought this was impossible, but @kumavis, being the browser hacker he is, pointed out that as a WebExtension, we are able to listen to pages that use the postMessage API, and so sites can send us a request without knowing whether we are listening or not.

This flow would look something like this, from an internal mural.ly board:

screen shot 2018-03-21 at 2 56 25 pm

A design for this stage can be seen in #3383:

deliberate log in flow

What Still Needs Defining: Message Format

We need to define the exact format of the message sent via postMessage. To keep this format compatible with other web3 browsers, we could not require the targetOrigin property of that API, which restricts possible recipients.

Constraints of the Message Format

  • Cannot use the targetOrigin API, to allow other web3 browsers to receive this event. (We're friendly!)
  • Should be unique enough to not accidentally interfere with other listeners (ideas?)
  • Should be extensible, to eventually include parameterized requirements of the Dapp (Need a certain network? Need a number of accounts? Need an account with a certain token? Maybe this becomes a way to add DID compliant personal information requests!) Related: #2532, #3663 and #3604

What Still Needs Defining: Design, Scope of Rollout

In issue #3383 you can see initial designs, and how we're exploring what the UX of this type of log-in might look like. There are a variety of open questions there, asking whether we should rush out the privacy component (not showing web3 or accounts) if it means we remove the ability to be signed in to multiple sites at once.

There are also several other UX/design concerns on that issue, and we can keep the design aspect of this feature mostly confined to #3383, and keep this issue for the specification of the API for Dapp developers.

@danfinlay danfinlay added this to the Multi Vault Support milestone Oct 11, 2016

@danfinlay danfinlay modified the milestones: Multi Vault Support, Public Release Nov 22, 2016

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 1, 2017

To clarify to those without context: This would mean not injecting any user accounts into the in-page web3 accounts array.

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 31, 2017

When opening the metamask popup to look at accounts, I would imagine we just show the accounts/balances for the currently open tab.

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented Jan 21, 2018

This issue is getting some public attention, and I think decent points have been made that especially given our spike in usage, we need to raise priority on this to protect our users.

https://twitter.com/backus/status/954921568198574080

https://twitter.com/nicksdjohnson/status/954999819344535553

@laurengarcia

This comment has been minimized.

Copy link

laurengarcia commented May 3, 2018

Unsolicited 2 cents from a Metamask fan and privacy dev: If the message format got too elaborate it could be used to fingerprint users, esp when paired with other info gathered from browser env. The more generic the better (for privacy) IMO.

Thanks and cheers!

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 3, 2018

@laurengarcia Do you think this is still an issue of the message format is totally ignored without user consent? Part of the reason for the use of postMessage is to eliminate nonconsensual metamask related fingerprinting. Am I missing something?

@laurengarcia

This comment has been minimized.

Copy link

laurengarcia commented May 3, 2018

@danfinlay Sorry, no you're not missing anything, your assumption for that use-case sounds correct.

My comment is perhaps too granular for this higher level feature and directed at the "user does consent" use-case actually, in response to the third bullet point in the description above under "Constraints of Message Format":

"Should be extensible, to eventually include parameterized requirements of the Dapp (Need a certain network? Need a number of accounts? Need an account with a certain token? Maybe this becomes a way to add DID compliant personal information requests!)"

Now that I looked into it more, my comment is probably better suited to #2532, the RFC where you discuss "Selective Disclosure" of personal info.

So I am imagining flow is: user visits dApp, and dApp sends postMessage requesting a bunch of information, only some of which I am willing to share in order to use minimum feature set of dApp. Am imagining that instead of a binary share account info / don't share account info button there are some base settings associated with my account which excludes identifying information i dont want to share by default. If those settings get too granular and as a user I have my defaults set and re-use them across dApps, that information could be used to fingerprint me anyway and roll up my activity across dApps. Similar to how cookie data and traditional advertising work on the open web right now.

That was my only point. Probably better suited to #2532 discussion!

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 4, 2018

Cool, that's an interesting set of concerns that I look forward to dealing with when we get there ;D

@ricburton

This comment has been minimized.

Copy link

ricburton commented May 7, 2018

I think you need to write a Medium post about this. This is a surprise to the dapp developers I know.

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 8, 2018

I think you need to write a Medium post about this. This is a surprise to the dapp developers I know.

Oh I know, but we need to finalize exactly the protocol we're proposing. We'll try to launch an article a week or two before pushing it live.

@ricburton

This comment has been minimized.

Copy link

ricburton commented May 9, 2018

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 9, 2018

So I would post the rough plan as soon as you can and add more details.

100++!

I think a cheap solution for existing dapps may be to simply add a line to the top of their JS:

window.postMessage(theFormattedMessage);

This will prompt the user to sign in, and then probably just reload the site with the assumptions that people are developing with today.

Of course this UX can be improved by sites that properly represent the request, with a button that says "Connect to MetaMask" or something, but we will need to hone out the UX of that optimal flow a bit more.

@ricburton

This comment has been minimized.

Copy link

ricburton commented May 9, 2018

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 9, 2018

By all means, sketch up how you think it might work.

I think we also agreed at the WalletConf that it could make sense to ease-in to this breaking change, by allowing end-users (who understand the new expectation of deliberately signing in) to first "enable" full-web3-stealth-mode (which would require manually signing into a site via MetaMask).

Once we have that live, and enable the "request login" API, hopefully Dapp devs will see the new API as beneficial to migrate to, before we perform a hard break.

@ricburton

This comment has been minimized.

Copy link

ricburton commented May 11, 2018

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 11, 2018

That post looks great, @ricburton! I'd be willing to put it under the MetaMask publication if you wanted our reach, we need to get the word out wide.

@ricburton

This comment has been minimized.

Copy link

ricburton commented May 11, 2018

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 11, 2018

Added as a writer.

@roschler

This comment has been minimized.

Copy link

roschler commented May 11, 2018

This may be either overkill or a poor idea but I hope it at least provides food for thought.

Given the immense risk of someone's Ethereum account being compromised and the incredible number of attack surfaces a web-page/browser-extension/browser-code environment provides, perhaps it's time to think about some form of inter-app handshaking?

I was wondering about something like this. It would require Metamask to host a server just to service the handshaking but it would be a lightweight server, probably with just a Redis cache to back it up.

Here's what I was thinking:

  • The web page that's about to initiate a Metamask transaction creates a nonce to identify the new transaction, let's call it the transaction nonce, whose sole purpose is to provide a key for the MTCS to represent the current transaction.

  • It submits the query nonce directly to the Metamask Transaction Control Server (MTCS for brevity's sake), waiting for a new nonce generated by the MTCS as a result of its query. This new nonce would be the control nonce. It uses an encrypted websocket connection to receive the response (to hide the transaction from other browser extensions that might be listening to post/xhr requests).

  • The web page notifies the Metamask extension of the payment request, providing the query nonce sent to the MTCS.

  • Metamask contacts the MTCS with the query nonce to get the control nonce This simultaneously results in the MTCS server pushing the the control nonce to the web page on its encrypted websocket connection.

  • The web page then asks Metamask to trigger the usual SEND/REJECT dialogue to confirm the transaction, providing the control nonce it received from the MTCS

  • Metamask checks the control nonce received from the web page to make sure it matches the one it got directly from the MTCS server earlier. If not, it returns an error and does not show the SEND/REJECT dialogue. Otherwise, execution proceeds at normal.

By having this parallel level of communication over encrypted web sockets between the web page and the Metamask extension, with the MTCS trusted server in the middle, I believe it would create a protocol layer that locks out other browser extensions and domains from snooping in on the transaction and thereby creating havoc.

What do you think?

@danfinlay

This comment has been minimized.

Copy link
Contributor

danfinlay commented May 15, 2018

@roschler This is an interesting idea, but seems outside the scope of this issue. It seems somewhat related to #4260, but with different goals. I don't think a local server would be needed, the extension already has a process with communication methods to each page. I'm also unclear what kind of snooping you're trying to avoid. Feel free to open this proposal in a new issue, and clearly describe the attack vectors you are trying to mitigate.

@ghost

This comment has been minimized.

Copy link

ghost commented Jan 3, 2019

Seems #4703 has fixed this, should be closable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment