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 most extension permissions optional #352

Closed
FiloSottile opened this issue Aug 14, 2024 · 10 comments
Closed

Make most extension permissions optional #352

FiloSottile opened this issue Aug 14, 2024 · 10 comments

Comments

@FiloSottile
Copy link

FiloSottile commented Aug 14, 2024

Currently the extension has mandatory access to all data on all websites, an extremely dangerous capability for example if a CSRF vulnerability were present or if the publishing keys were compromised. It would be nice to make this opt-in, which is actually very well supported by browsers.

It should be just a matter of moving most permissions in the manifest to optional_permissions.

Here's how the browser UI would look like in settings.

image

And here's how it would look like in the menu bar.

image

"Only when clicked" should be perfectly functional, and way safer.

I have implemented a browserpass-native replacement that uses age (for browserpass/browserpass-native#127) but I am wary to recommend any unsandboxed extension.

Thank you!

@maximbaz
Copy link
Member

Linking some earlier discussion on the topic: #265

@FiloSottile
Copy link
Author

We did actually have <all_urls> as an optional permission in the past, requested when needed. Unfortunately, ActiveTab doesn't allow access to non-root frames in the active tab (see this Chromium bug), and some sites put their login forms in an iframe. To prompt on some sites and not on others makes for an inconsistent and very confusing user experience; even more so if we were to prompt for an origin domain they don't recognise (e.g. when an iframe login form is served from some third party). As such, <all_urls> was set as a mandatory permission in #22.

That's an unfortunate limitation, but I would argue against trading off a lot of security for most users for a little convenience for some users (not having to opt-in to the extended permissions if encountering iframe'd logins).

Similarly with webRequest and HTTP authentication, which I can't imagine is the most popular feature of the extension.

@maximbaz
Copy link
Member

I don't think I quite agree, with enough time eventually everyone would stumble upon a website with a login form in an iframe, and then all we achieved is forcing everyone to research what is wrong with the extension, and how to activate the necessary option...

@FiloSottile
Copy link
Author

Even assuming that iframe'd login pages are that common (which I am not convinced of, but hard to get data), I would much rather copy-paste the password in those few occasions than expose all my Google, GitHub, etc. sessions to an extension.

There're certainly users who will make different tradeoffs, the issue is that right now the only way to opt-out of the permissions is to fork the extension and re-publish it (as Firefox doesn't allow unsigned extensions). If the permissions were optional, users could opt-in with a simple click, which could be documented in the README.

browserpass is arguably targeting technical security-conscious users, so adding a tiny bit of friction to support a high-security setting feels like a justifiable tradeoff to me.

@maximbaz
Copy link
Member

I definitely share the frustration that it's not easy to override, especially on Firefox. Could you explain a bit what do you mean by exposing all your Google and Github sessions to the extension? You are not using Browserpass to login to these websites, but you visit other domains where Github is inside an iframe?

One of the primary goals for this extension is to protect people against phishing attacks, which is arguably more of a real risk for an average user than the risk of publishing keys getting exposed (now that I write this, I wish for this claim to age well 😅🤞), so getting more people to copy paste credentials is counter productive for us as a default experience...

@FiloSottile
Copy link
Author

Could you explain a bit what do you mean by exposing all your Google and Github sessions to the extension? You are not using Browserpass to login to these websites, but you visit other domains where Github is inside an iframe?

It's not about using Browserpass, or about iframes, it's about it having ambient access to every session in my browser. As the extension works today, even if I don't use it on Google and GitHub, an attacker that compromises you or anyone else with the publishing key for the extension will have access to my GitHub session (and from there to, uh, a lot of users 😅).

Same with a UXSS vulnerability in Browserpass.

That's just too much risk, and I don't grant any extension access to all websites.

One of the primary goals for this extension is to protect people against phishing attacks, which is arguably more of a real risk for an average user than the risk of publishing keys getting exposed (now that I write this, I wish for this claim to age well 😅🤞), so getting more people to copy paste credentials is counter productive for us as a default experience...

I agree phishing is a more present risk for the average user, but I don't see why clicking the extension and then copy-pasting the password (for the small % of websites that do iframed logins) would be weaker: the extension will still filter passwords by the top-level origin, so the user can't be tricked into copy-pasting the wrong password any more than they can be tricked into autofilling it.

(This is assuming you filter based on the top-level origin. If you filter based on the iframe origin, I would actually argue that's dangerous, as it risks exposing the password to the parent.)

@FiloSottile
Copy link
Author

Oh, one more threat vector I can't responsibly expose all my sessions and my users to: the 176 npm packages in src/yarn.lock. As things stand, compromising any of those (for long enough for dependabot to do its thing and for you to make a release) leads to full compromise of the browser of every Browserpass user.

@erayd
Copy link
Collaborator

erayd commented Aug 15, 2024

Even assuming that iframe'd login pages are that common (which I am not convinced of, but hard to get data)...

They are common. That is the whole reason the logic to address them exists in the first place; there are simply far too many systems built that way. I think it's a terrible way to build a login workflow, but nonetheless, it is there and we need to serve that need.

I would much rather copy-paste the password in those few occasions than expose all my Google, GitHub, etc. sessions to an extension.

I appreciate your perspective on it. That is your call to make. However, usability matters (particularly when there is limited time available to provide support), plus clipboard sniffing is another notable exploit vector that going down the copy / paste path exposes you to.

There're certainly users who will make different tradeoffs, the issue is that right now the only way to opt-out of the permissions is to fork the extension...

That is also something you are welcome to do, if you would prefer to draw your lines in a different place to where this project has drawn them.

I feel it's worth pointing out that if you use this in Chrome rather than Firefox, you can revoke the all-sites permission on your end, and grant it only-when-launched access instead. This should work around your key concern. I do not believe that Firefox has this capability.

browserpass is arguably targeting technical security-conscious users, so adding a tiny bit of friction to support a high-security setting feels like a justifiable tradeoff to me.

We have enough users asking really obvious questions that the additional friction will be considerably problematic.

One of the primary goals for this extension is to protect people against phishing attacks, which is arguably more of a real risk for an average user than the risk of publishing keys getting exposed (now that I write this, I wish for this claim to age well 😅🤞), so getting more people to copy paste credentials is counter productive for us as a default experience

I agree strongly with this.

..the extension will still filter passwords by the top-level origin, so the user can't be tricked into copy-pasting the wrong password any more than they can be tricked into autofilling it.

In fact, Browerpass actually filters by both origins. Foreign iframes with fillable credential fields get caught this way - Browserpass will warn the user if they try to fill into an iframe from a different origin, and ask if that is really what they want to do.

@FiloSottile Honestly, it sounds like you may be better served by finding a different browser extension to serve your pass-in-the-browser needs. It's clear that your security lines lie in a different place to those of this project.

While you do make some valid points, and PRs to improve security are always welcome (for example, if you feel like re-implementing the functionality of some of our dependencies in order to reduce the supply chain risk, that would be very welcome indeed), I don't feel it's reasonable to break important functionality and materially inconvenience many users simply because you feel uncomfortable about the tradeoffs involved. They were not made accidentally.

One further point of note - simply by using it, you are willingly exposing sensitive credential material to Browserpass. That risk remains, regardless of whether or not the extension has an all-sites permission. It's also a risk that is true of any password manager. Any analysis of whether or not it's reasonable to expose website sessions to it needs to be evaluated in that context. The choice we have made is a common one, and is fairly standard for password managers (as a high-profile example, 1Password also uses an all-sites permission).

@FiloSottile
Copy link
Author

Very well, sounds like we disagree on this, I will look for alternative solutions for my needs. Thank you for your time!

@erayd
Copy link
Collaborator

erayd commented Aug 19, 2024

Very well, sounds like we disagree on this, I will look for alternative solutions for my needs. Thank you for your time!

I think so too - but thank you for engaging with us on this. If nothing else, the mere existence of this issue thread is helpful context for users to read. I wish you good luck with finding something that better fits your requirements 🙂

If you do find a suitable alternative, would you be willing to post a link here? I daresay you won't be the only user who would prefer the security line be drawn in a less-convenient-but-more-secure place, and I'd be keen to add such an option to our README file so that we have somewhere to direct them. I'm not aware of any that currently occupy that space for pass stores, but if one exists then signposting it clearly for our users (and potential users) seems like a sensible thing to do.

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

No branches or pull requests

3 participants