1685682 - Firefox keeps asking to "Allow this site to open potplayer link in Potplayer( 64 bit)" every time i send a youtube video link to be opened in Potplayer video player

Status: RESOLVED FIXED

(Firefox :: Security, defect)

Description

[cheloox7]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:84.0) Gecko/20100101 Firefox/84.0

Steps to reproduce:

1.I installed this addon that adds youtube video links in Potplayer Playlist so i can watch them directly in Potplayer https://addons.mozilla.org/en-US/firefox/addon/potplayer-youtube-shortcut/
The addon is set up in the following way:
https://imgur.com/hUsCqlj

2.I install the 64 bit Potplayer video player from https://potplayer.daum.net/ with it's Default installer settings.

3.I open Firefox browser on my PC and go to youtube.com

3.I right click on any youtube video and click "Add link to Potplayer"

4.This dialog box pops up in Firefox:
https://i.imgur.com/YgHCkiC.png

5I click on https://i.imgur.com/5wh628L.pnghttps://i.imgur.com/5wh628L.png
to make sure every time the addon keeps sending the vido links in Potplayer

Actual results:

This dialog box keeps appearing in Firefox

https://i.imgur.com/YgHCkiC.png

Expected results:

Dialog box should not appear because i already clicked previously on

https://i.imgur.com/5wh628L.pnghttps://i.imgur.com/5wh628L.png

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 3

[Francesco Lodolo [:flod]]

Not sure if Firefox::Security is the right component, but moving there for now, based on bug 1565574.

Comment 4

[Johann Hofmann [:johannh]]

Paul, I suppose this is happening because we don't have a principal to remember the permission for here? I guess we'd have to see how exactly the add-on is trying to open the app, but shouldn't we in theory be able to take the add-on principal for remembering permissions? Alternatively we could consider not prompting permissions for add-on principals given they already have somewhat elevated trust?

I can imagine that this is a not uncommon problem since there are quite a few add-ons for integrating with native apps out there.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1565574

Comment 6

[Anonym]

A temporary fix for Firefox users:

1. open "about:config"
2. find "network.protocol-handler.external.potplayer"
3. set it to " true"

Comment 7

[:Gijs (he/him)]

I guess this is because https://searchfox.org/mozilla-central/rev/014fe72eaba26dcf6082fb9bbaf208f97a38594e/toolkit/mozapps/handling/ContentDispatchChooser.jsm#569 returns false somehow. Between that and "this site" in the dialog, because youtube doesn't use frames, I'm guessing somehow we're getting a null principal for the navigation? It looks like the extension uses:

		chrome.tabs.executeScript(tab[0].id, {code: "var url = " + JSON.stringify("potplayer://" + url + " /add") + "; window.open(url, '_self');"});

from a background script to load these URLs. I don't know why that then ends up with a null principal (rather than one for the extension). :zombie, do you know? Perhaps it's an extended principal or something, and then the schemeIs check just always returns false?

Comment 8

[Emma Zühlcke [:emz]]

It's an expanded principal and the scheme is unset. For example:

[Expanded Principal [https://www.youtube.com, moz-extension://9c43f587-7292-42e1-992f-f7952fbb4525]]

I'm not very familiar with expanded principals. How can we tell that this is an extension caller and potentially skip the permission check? Could we do that for all expanded principals?
If we want to do permission checks we'd have to get the principal for the extension, because we can't store permissions for expanded principals. We'd also have to add code to resolve the extension name to show in the dialog.

Comment 9

[:Gijs (he/him)]

(In reply to Paul Zühlcke [:pbz] from comment #8)

It's an expanded principal and the scheme is unset. For example:

[Expanded Principal [https://www.youtube.com, moz-extension://9c43f587-7292-42e1-992f-f7952fbb4525]]

I'm not very familiar with expanded principals. How can we tell that this is an extension caller and potentially skip the permission check? Could we do that for all expanded principals?
If we want to do permission checks we'd have to get the principal for the extension, because we can't store permissions for expanded principals. We'd also have to add code to resolve the extension name to show in the dialog.

Something like this: https://searchfox.org/mozilla-central/rev/07342ce09126c513540c1c343476e026cfa907bf/uriloader/exthandler/nsExternalHelperAppService.cpp#1063 , cf. https://searchfox.org/mozilla-central/rev/07342ce09126c513540c1c343476e026cfa907bf/caps/nsIPrincipal.idl#437-443 ?

Comment 10

[:Gijs (he/him)]

(I think we should skip the permission checks for add-ons, as we do for the "can you do things for this frame")

Comment 11

[Emma Zühlcke [:emz]]

Attachment: Bug 1685682 - Skip protocol handler permission dialog for addon callers. r=gijs!

Comment 12

[Tomislav Jovanovic :zombie]

(In reply to :Gijs (he/him) from comment #7)
I don't know why that then ends up with a null principal (rather than one for the extension). :zombie, do you know? Perhaps it's an extended principal or something, and then the schemeIs check just always returns false?

Sorry for the late response. Yeah it's an expanded principal as you already figured out. For some purposes we treat them just like the principal of the content page, and I think that might also make sense here -- user would be prompted once for each domain where the content scripts tries to open the external protocol.

Treating it as an extension principal, and giving permission per each extension once could work as well. But unless I'm misunderstanding something, wouldn't allowing extensions to open external protocols without any prompts be a regression?

Before bug 1565574, that would prompt at least the first time. We generally try not to give potentially surprising elevation to extensions without tying it to some permissions or at least acknowledgment in the UI.

Comment 13

[Emma Zühlcke [:emz]]

(In reply to Tomislav Jovanovic :zombie from comment #12)

But unless I'm misunderstanding something, wouldn't allowing extensions to open external protocols without any prompts be a regression?

Before Bug 1565574 we didn't permission prompt at all. Now there are two prompts. First we show the permission prompt. Once the user accepts that we show the application chooser dialog. Skipping the permission check means only skipping the first dialog. So we wouldn't really regress in that regard.

Before bug 1565574, that would prompt at least the first time.

Only the first time that protocol was opened (no handler set). We didn't prompt once per extension.

We generally try not to give potentially surprising elevation to extensions without tying it to some permissions or at least acknowledgment in the UI.

Gijs, I think :zombie makes a good point here. Do you think we should stick with our exception approach?

Perhaps there is an extension permission we could check for instead? This way we avoid showing the user more prompts. We already show the permission prompt when installing the addon.

Comment 14

[:Gijs (he/him)]

I don't have a strong opinion about the ideal state, and defer to the add-ons team as to what that is supposed to be. However, I think either way landing this patch as-is makes sense and we should do that to fix the regression, as the status quo is annoying and the patch doesn't make the permission situation any worse than it used to be.

The permission to launch external protocols on a per-site basis was primarily done to avoid cases where a website and a local app are tightly coupled (e.g. zoom / teamviewer / other teleconferencing stuff) and users don't expect other websites to have permissions to open those apps willy-nilly and this can lead to (perceived) security issues.

I guess in theory a similar thing issue may exist for add-ons and external tools, where you want add-on A to be able to open tool A but not add-on B? But I haven't heard about it, it seems kinda far-fetched, and AIUI we cannot easily get an add-on-only permission that we can use to store in the permissions database. I don't know if it's worth additional engineering here to add per-add-on x protocol paired permissions here the way we've added permissions for site x protocol pairs.

If we think we need additional permission checks for extensions, I think we should file a separate bug for that.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0d1e9f0afdb
Skip protocol handler permission dialog for addon callers. r=Gijs,mstriemer

Comment 16

[Tomislav Jovanovic :zombie]

(In reply to Paul Zühlcke [:pbz] from comment #13)

Before Bug 1565574 we didn't permission prompt at all. Now there are two prompts. First we show the permission prompt. Once the user accepts that we show the application chooser dialog. Skipping the permission check means only skipping the first dialog. So we wouldn't really regress in that regard.

Ah, I misunderstood bug 1565574. In that case, Gijs is right, this patch is pure improvement to current situation, and I'm not even sure followup is needed.

Perhaps there is an extension permission we could check for instead? This way we avoid showing the user more prompts. We already show the permission prompt when installing the addon.

There is no extension permission that corresponds to this (we would need to introduce a new one). If this is using standard site permissions, extension principals already work well with that setup, we currently use it for things like indexeddb storage, geo and similar "web" permissions.
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/Extension.jsm#2396-2397,2407,2409,2412,2414

Comment 17

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/c0d1e9f0afdb

Comment 18

[cheloox7]

(In reply to Alexandru Michis [:malexandru] from comment #17)

https://hg.mozilla.org/mozilla-central/rev/c0d1e9f0afdb

Now the plugin does not work at all after 86 version is released.

Comment 19

[:Gijs (he/him)]

(In reply to cheloox7 from comment #18)

(In reply to Alexandru Michis [:malexandru] from comment #17)

https://hg.mozilla.org/mozilla-central/rev/c0d1e9f0afdb

Now the plugin does not work at all after 86 version is released.

What does "does not work at all" mean - where in the process described in comment #0 do things break? Can you provide more details in a separate bug?

Comment 20

[cheloox7]

(In reply to :Gijs (he/him) from comment #19)

(In reply to cheloox7 from comment #18)

(In reply to Alexandru Michis [:malexandru] from comment #17)

https://hg.mozilla.org/mozilla-central/rev/c0d1e9f0afdb

Now the plugin does not work at all after 86 version is released.

What does "does not work at all" mean - where in the process described in comment #0 do things break? Can you provide more details in a separate bug?

Meaning...now there is no issue about Firefox continuously asking for permissions for this plugin being broken, because the plugin I'm talking about in this bug does not even work anymore.
Only by setting "network.protocol-handler.external.potplayer" to TRUE the plugin works as intended.

Comment 21

[:Gijs (he/him)]

(In reply to cheloox7 from comment #20)

(In reply to :Gijs (he/him) from comment #19)

(In reply to cheloox7 from comment #18)

(In reply to Alexandru Michis [:malexandru] from comment #17)

https://hg.mozilla.org/mozilla-central/rev/c0d1e9f0afdb

Now the plugin does not work at all after 86 version is released.

What does "does not work at all" mean - where in the process described in comment #0 do things break? Can you provide more details in a separate bug?

Meaning...now there is no issue about Firefox continuously asking for permissions for this plugin being broken, because the plugin I'm talking about in this bug does not even work anymore.

We can't reproduce this, it works for us.

Only by setting "network.protocol-handler.external.potplayer" to TRUE the plugin works as intended.

... if you remove this pref (not set it to false, remove it - click the trashcan icon next to the pref in about:config ), and restart Firefox, is it still broken?

If so, please file a new bug with your about:support information.

Comment 22

[cheloox7]

(In reply to :Gijs (he/him) from comment #19)

(In reply to cheloox7 from comment #18)

(In reply to Alexandru Michis [:malexandru] from comment #17)

https://hg.mozilla.org/mozilla-central/rev/c0d1e9f0afdb

Now the plugin does not work at all after 86 version is released.

What does "does not work at all" mean - where in the process described in comment #0 do things break? Can you provide more details in a separate bug?

Nevermind,I figured out what was wrong,it was not because of Firefox,it was user error.Bug is indeed solved and plugin works correctly.(In reply to :Gijs (he/him) from comment #21)

(In reply to cheloox7 from comment #20)

(In reply to :Gijs (he/him) from comment #19)

(In reply to cheloox7 from comment #18)

(In reply to Alexandru Michis [:malexandru] from comment #17)

https://hg.mozilla.org/mozilla-central/rev/c0d1e9f0afdb

Now the plugin does not work at all after 86 version is released.

What does "does not work at all" mean - where in the process described in comment #0 do things break? Can you provide more details in a separate bug?

Meaning...now there is no issue about Firefox continuously asking for permissions for this plugin being broken, because the plugin I'm talking about in this bug does not even work anymore.

We can't reproduce this, it works for us.

Only by setting "network.protocol-handler.external.potplayer" to TRUE the plugin works as intended.

... if you remove this pref (not set it to false, remove it - click the trashcan icon next to the pref in about:config ), and restart Firefox, is it still broken?

If so, please file a new bug with your about:support information.

Nevermind,I figured out what was wrong,it was not because of Firefox,it was user error.Bug is indeed solved and plugin works correctly.