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

Warn about sideloaded extensions #5063

Closed
tildelowengrimm opened this issue Jun 28, 2019 · 38 comments
Closed

Warn about sideloaded extensions #5063

tildelowengrimm opened this issue Jun 28, 2019 · 38 comments
Labels
closed/stale Issue is no longer relevant, perhaps because the feature it refers to has been deprecated. feature/extensions OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. regression security

Comments

@tildelowengrimm
Copy link
Contributor

tildelowengrimm commented Jun 28, 2019

Sideloaded extensions have been a common malware vector, but are less effective at present thanks to browser interventions. To protect people from surreptitiously-sideloaded extensions, Brave should show a warning whenever the browser starts.

57393206-a2149d00-7190-11e9-855d-383d74ad0e94

Unfortunately, any preference which allowed this warning to be bypassed could be manipulated by the same tooling which sneakily-sideload extensions in the first place. So for this to work, there can't be a preference, configuration flag, or similar for official builds. Non-official builds can have a preference (either a flag or a setting) to disable the warning.

[Note that this warning is not present in Brave's Dev or Nightly builds. If this bugs you, try out Brave Dev.]

@Kaerakh

This comment has been minimized.

@epycurasWynter

This comment has been minimized.

@Mr-Mondragon

This comment has been minimized.

@tildelowengrimm tildelowengrimm added security priority/P3 The next thing for us to work on. It'll ride the trains. labels Jun 28, 2019
@tildelowengrimm
Copy link
Contributor Author

Hi everyone, I want to make something absolutely clear: if you are commenting on this issue, if you are using GitHub, if you make the decision that you want to side-load an extension permanently then this warning is not for you.

This warning exists to protect people who have had extensions sneakily sideloaded without their knowledge. There is a tradeoff between protecting those people and avoiding an inconvenience when y'all open your browsers. We can't meet both sets of needs in the same build. If you have proposed a configurable preference for this notice, you have proposed a plan which will not protect people against surreptitious sideloading, because whatever software sideloads an extension can muck with that preference. There is no middle ground here.

I know that this is going to annoy you occasionally. But I think that is a worthwhile tradeoff for the protection it provides to other people who are not you. I appreciate that you care deeply about this. But more comments of this sort (or of the sort seen on previous related issues) are offtopic. If you want to discuss this topic, please head to community.brave.com. This issue is not the place for further conversation about this.

@Kaerakh

This comment has been minimized.

@epycurasWynter

This comment has been minimized.

@Kaerakh

This comment has been minimized.

@bridiver
Copy link
Contributor

bridiver commented Jul 9, 2019

is this only happening on windows?

bool EnableDevModeBubble() {
  if (extensions::FeatureSwitch::force_dev_mode_highlighting()->IsEnabled())
    return true;

  // If an automated test is controlling the browser, we don't show the dev mode
  // bubble because it interferes with focus. This isn't a security concern
  // because we'll instead show an (even scarier) infobar. See also
  // AutomationInfoBarDelegate.
  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
  if (command_line->HasSwitch(switches::kEnableAutomation))
    return false;

#if defined(OS_WIN)
  if (chrome::GetChannel() >= version_info::Channel::BETA)
    return true;
#endif

  return g_override_for_testing ==
         ExtensionMessageBubbleFactory::OVERRIDE_ENABLED;
}

and

class CommonSwitches {
 public:
  CommonSwitches()
      : force_dev_mode_highlighting(switches::kForceDevModeHighlighting,
                                    FeatureSwitch::DEFAULT_DISABLED),
        prompt_for_external_extensions(

@bsclifton
Copy link
Member

bsclifton commented Jul 11, 2019

@bridiver it should be happening on macOS too - I know I was getting it (along with others)

@50P15

This comment has been minimized.

@theAeon
Copy link

theAeon commented Jul 16, 2019

Gonna pitch an idea here assuming that having a non-disable-able notification is non-negotiable. The notification is annoying not because it exists, not because there is no way to disable it, but because it grabs the cursor focus. While I do think for average use it makes sense for it to interrupt like that as most will not be mucking around in developer mode, maybe it would make sense to have a toggle on an extension-by-extension basis to have the notification not grab focus and possibly dismiss after a set period of time? Does this comply with you security-based view on this problem, @tomlowenthal ?

@tildelowengrimm
Copy link
Contributor Author

Again, and I cannot emphasize this enough: this tool does not work if there are any settings which change its behavior. It seems fine to me for it not to grab focus, but I don't know how that interacts with the a11y requirements for warnings. Auto-dismiss not so much.

@theAeon
Copy link

theAeon commented Jul 17, 2019

Hmm, another way of approaching it would be having it grab focus as it does now but having actions functionally dismiss it. For example, if, on browser start, I click into the URL bar and search, upon my request of the page the warning would dismiss.

@bsclifton
Copy link
Member

bsclifton commented Jul 22, 2019

@theAeon one possible solution would be a "bespoke" version of Brave, similar to how Firefox has a "Developer" edition. I created an issue to track that (and any other specific requests):
#5315

Until there are enough requirements to take action, that issue is just a good place for discussion. In the meantime, folks can grab the source and compile with the appropriate field trial configs flipped. Docs about setting up and building can be found here:
https://github.com/brave/brave-browser/wiki

@mikhoul
Copy link

mikhoul commented Aug 12, 2019

Just migrated from Chromium to Brave but this a major annoying issue for me and my users and I remember why I begun to use Chromium because it don't display the warning.

I manage ~65 computers where I choose which browser is used and since we use custom extensions we need to have an option to disable the warning for our users.

There is a way to make this option without implementing it in the GUI : Just put an option on the command line when you start brave.exe with something like brave.exe -StopDevWarning. Other Chromium forks use this method for implementing custom options.

But honestly the more elegant way would be with a toggle switch that when activated generate a HUGE WARNING that once this option is enabled that your computer can explode.

Right now I will wait before migrating my users to something else that Chromium until there is a way to disable the popup.

Note: Even Vivaldi which is a major chromium fork disabled this warning.

Regards :octocat:

@theAeon
Copy link

theAeon commented Aug 12, 2019

Seconding the command line flag as an option. If a malicious program is able to change the command line flag in the start menu shortcut it's already achieved system privilege escalation and the user in question seems to be already screwed.

@paulej

This comment has been minimized.

@BenOravetz
Copy link

Here's a solution to this problem: have a whitelist for extensions which we know and can verify as being safe. Only allow that whitelist to be updated when the warning prompts and once an extension is whitelisted, don't warn again.

Having a warning every time a browser loads is just silly. It's a holdover from Google to force users to only use "approved" extensions from their web store. But there some solid extensions out there which are not "approved" by Google which we know are safe and would like to use.

@cthu1hoo

This comment has been minimized.

@paulej

This comment has been minimized.

@cthu1hoo

This comment has been minimized.

@paulej

This comment has been minimized.

@Arcuplas

This comment has been minimized.

@cthu1hoo

This comment has been minimized.

@Arcuplas

This comment was marked as off-topic.

@theAeon

This comment has been minimized.

@Arcuplas

This comment was marked as off-topic.

@bridiver
Copy link
Contributor

bridiver commented Nov 8, 2019

@mikhoul the reason that Chromium doesn't display the notification is because it doesn't have beta/release channel like Brave and Chrome. It is enabled on beta and release for Brave and Chrome, but not for dev. You could either build Brave yourself (setting it to dev channel) or use Brave dev channel.

@bridiver
Copy link
Contributor

bridiver commented Nov 8, 2019

I haven't actually verified ^^ myself, but that's what the code says #5063 (comment)

@Arcuplas

This comment has been minimized.

@Arcuplas

This comment has been minimized.

@BrendanEich
Copy link
Member

The off-topic comments here are not wrong, just off-topic. We'll work on helping users who've been warned sufficiently and know what they're doing to sideload more conveniently, without opening up large numbers of users to socially engineered attacks. For now please keep the OT comments on hold. Thanks.

@Arcuplas

This comment has been minimized.

@tivalin

This comment was marked as abuse.

@tivalin

This comment was marked as abuse.

@Colin-Mac-Donald

This comment has been minimized.

@bsclifton
Copy link
Member

bsclifton commented Dec 17, 2019

@cthu1hoo
personally I've switched back to Chrome which allows me to self-host my extensions in a supported way via enterprise policy (i.e. GPO) which Brave obviously doesn't support.

GPO is supported actually 😄 If this case works for you in Chrome, it should work great for Brave too

On Windows, the location in registry for the keys is going to be:
1.2 and older
HKLM\SOFTWARE\Policies\Chromium

1.3 and newer
HKLM\SOFTWARE\Policies\BraveSoftware\Brave

Once set, you can confirm by visiting brave://policy

@bsclifton bsclifton added the closed/stale Issue is no longer relevant, perhaps because the feature it refers to has been deprecated. label Sep 26, 2022
@bsclifton
Copy link
Member

Closing as stale. The UI that was shown in original post was back when we were using a different build config that enabled this by default. We reverted that in 2019 or so and this issue was created to track potentially adding that back in. There hasn't been any updates for ~3 years

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed/stale Issue is no longer relevant, perhaps because the feature it refers to has been deprecated. feature/extensions OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. regression security
Projects
None yet
Development

No branches or pull requests