Closed Bug 1165981 Opened 9 years ago Closed 9 years ago

Whitelist Flash for NPAPI on 64 bit Firefox on Win64

Categories

(Core Graveyard :: Plug-ins, defect)

40 Branch
x86_64
Windows
defect
Not set
normal

Tracking

(firefox40- wontfix, firefox41+ fixed, firefox42+ verified)

VERIFIED FIXED
mozilla42
Tracking Status
firefox40 - wontfix
firefox41 + fixed
firefox42 + verified

People

(Reporter: javaun, Assigned: qdot)

References

Details

Attachments

(2 files, 8 obsolete files)

2.92 KB, patch
Details | Diff | Splinter Review
19.35 KB, patch
Details | Diff | Splinter Review
Firefox 64 bit will launch with highly restricted access to NPAPI. The requirement here is to create a whitelist so that the only allowed NPAPI plugins are Flash and Silverlight.
Component: General → Plug-ins
Product: Firefox → Core
I assume this means Firefox for Windows.  Does it?
Flags: needinfo?(jmoradi)
Yes, sorry. I added to the bug description, since it's that important.
Flags: needinfo?(jmoradi)
Summary: Whitelist Flash, SIlverlight for NPAPI on Fx64 → Whitelist Flash, SIlverlight for NPAPI on 64 bit Firefox on Win64
OS: Unspecified → Windows
Assignee: nobody → kyle
After some initial research, I can see one solution, which is just whitelisting the plugin libraries via prefs:

https://dxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsNPAPIPlugin.cpp#296

So we just ship with prefs for flash and whatever other plugin libraries whitelisted, and plugins in general turned off. Is that going to be strong enough for what we want to do?
No. That will change the default state of a plugin, but it will still show up in the addon manager and users can enable it manually if they wish.

What we want here is for the plugins to not even be detected at all, not show up in the addon manager, and not be user-enablable or even programmatically with an extension.
First shot at this, using library filename and MIME type, as I discussed with bsmedburg on IRC. I /think/ I've covered all of the places we add plugins to the list, and if any further changes are needed we can now just modify the ShouldAddPlugin function.
Ok, was wondering if this would horribly break plugin tests, and sure enough, it did. Adding whitelist for "application/x-test" mime type.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7cb4419d3cb8
I do not believe we need to worry about Silverlight here. Updating bug title.
Summary: Whitelist Flash, SIlverlight for NPAPI on 64 bit Firefox on Win64 → Whitelist Flash for NPAPI on 64 bit Firefox on Win64
Ok, well, having to do lots of mochitest cleanup (current round: https://treeherder.mozilla.org/#/jobs?repo=try&revision=18a438342904), so not quite ready for review, but at least putting in for fb? to make sure I'm on the right track. Seems to work locally at least.
Attachment #8631939 - Attachment is obsolete: true
Attachment #8631994 - Flags: feedback?(benjamin)
Ok, I think I've got the proper mime types whitelists for tests, should be reviewable now.
Attachment #8631994 - Attachment is obsolete: true
Attachment #8631994 - Flags: feedback?(benjamin)
Attachment #8632238 - Flags: review?(benjamin)
Make a new test plugin with a MIME type that's not whitelisted. Only thing I'm worried about here is that it's hard to know whether the plugin is even /there/ from the level of the test. Any ideas? Maybe a chrome test just to see if the file is in the plugins directory?
Attachment #8632241 - Flags: review?(benjamin)
Comment on attachment 8632238 [details] [diff] [review]
Patch 1 (v3) - Only allow Flash as a plugin on Win64 Firefox

The npswf check should probably be case-insensitive. I think you can do this with:

StringBeginsWith(aPluginTag->mFileName, NS_LITERAL_CSTRING("NPSWF"), nsCaseInsensitiveCStringComparator)

Or:
Substring(aPluginTag->mFileName, 0, 5).LowerCaseEqualsASCII("npswf");
Attachment #8632238 - Flags: review?(benjamin) → review+
Changed filename test to be case insensitive.
Attachment #8632238 - Attachment is obsolete: true
Updated test to have Flash plugin test, to make sure whitelisting works correctly.
Attachment #8632241 - Attachment is obsolete: true
Attachment #8632241 - Flags: review?(benjamin)
Attachment #8635043 - Flags: review?(benjamin)
Comment on attachment 8635043 [details] [diff] [review]
Patch 2 (v2) - Mochitests for disallowed plugins

\ No newline at end of file

pls fix while you're touching this

diff --git a/testing/mochitest/Makefile.in b/testing/mochitest/Makefile.in

nit, whitespace should be two spaces not a tab
Attachment #8635043 - Flags: review?(benjamin) → review+
Fixed review comments
Attachment #8635043 - Attachment is obsolete: true
Ok, adding the dummy flash plugin to Patch 2 is causing lots of problems in XPCShell tests. May have to roll patch version back to version 1 just to make sure whitelisting works, then file a followup.
Add application/x-shockwave-flash-test as allowed mimetype for dealing with mochitest, since application/x-shockwave-flash has too many special cases around gecko.
Attachment #8635040 - Attachment is obsolete: true
Add application/x-shockwave-flash-test as allowed mimetype for dealing with mochitest, since application/x-shockwave-flash has too many special cases around gecko.
Attachment #8635409 - Attachment is obsolete: true
Added exception to test_provider_addons xpcshell test for plugin that may or may not show up depending on platform.
Attachment #8636109 - Attachment is obsolete: true
Comment on attachment 8636107 [details] [diff] [review]
Patch 1 (v5) - Only allow Flash as a plugin on Win64 Firefox; r=bsmedberg

Approval Request Comment
[Feature/regressing bug #]: None
[User impact if declined]: All plugins will be allowed on win64 at release, versus just flash.
[Describe test coverage new/current, TreeHerder]: Mochitests on win64 to check for plugin whitelisting
[Risks and why]: We'll probably need to announce the change somewhere.
[String/UUID change made/needed]: None
Attachment #8636107 - Flags: approval-mozilla-aurora?
Comment on attachment 8636629 [details] [diff] [review]
Patch 2 (v5) - Mochitests for disallowed plugins; r=bsmedberg

Approval Request Comment
[Feature/regressing bug #]: None
[User impact if declined]: All plugins will be allowed on win64 at release, versus just flash.
[Describe test coverage new/current, TreeHerder]: Mochitests on win64 to check for plugin whitelisting
[Risks and why]: We'll probably need to announce the change somewhere.
[String/UUID change made/needed]: None
Attachment #8636629 - Flags: approval-mozilla-aurora?
I've just filed bug 1186746. Please reconsider the omission of Silverlight from the whitelist.
Depends on: 1187005
(In reply to Kyle Machulis [:kmachulis] [:qdot] (USE NEEDINFO?) from comment #23)
> Aurora Try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b9c00257aa0

Kyle, while reviewing the Aurora Uplift request, I noticed that one of the linux x64 opt test is busted. It is the red colored "V" in your try push link from above. What does that mean?
Flags: needinfo?(kyle)
(In reply to Kyle Machulis [:kmachulis] [:qdot] (USE NEEDINFO?) from comment #24)
> Comment on attachment 8636107 [details] [diff] [review]
> Patch 1 (v5) - Only allow Flash as a plugin on Win64 Firefox; r=bsmedberg
> 
> Approval Request Comment
> [Feature/regressing bug #]: None
> [User impact if declined]: All plugins will be allowed on win64 at release,
> versus just flash.
> [Describe test coverage new/current, TreeHerder]: Mochitests on win64 to
> check for plugin whitelisting
> [Risks and why]: We'll probably need to announce the change somewhere.
> [String/UUID change made/needed]: None

Kyle, we could announce this in the release notes. If you update the tracking flag "relnote-firefox" -> "?" and fill out the template with suggested wording and other details, I can add this to FF41 release notes. Will this be uplifted to Beta channel before FF40 releases? Thanks.
The Valgrind bustage is unrelated to this patch.
QE team could you please verify this fix has no unexpected side effects on a nightly or aurora build? Thanks.
Comment on attachment 8636107 [details] [diff] [review]
Patch 1 (v5) - Only allow Flash as a plugin on Win64 Firefox; r=bsmedberg

Approving patch for uplift to Aurora based on Ryan's comment and the fact that win64 automated tests pass including new mochitests added in this bug.
Attachment #8636107 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8636629 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Yeah, write in the release notes that Netflix, Amazon Instant Video, Magine TV, Maxdome and an unknown number of other streaming and VOD sites worldwide will stop working with this change. That will certainly make lots of Firefox fans burst with joy.
I'm gonna ni? bsmedburg on the relnote for this, as it's already causing some issues and probably needs to be stated carefully.
Flags: needinfo?(kyle) → needinfo?(benjamin)
[Tracking Requested - why for this release]: win64 builds hits the Release channel with Firefox 40 that will be shipped in 2 weeks.
Flags: needinfo?(jmoradi)
Kyle, based on comment 36, does this need to be uplifted to Beta too?
Flags: needinfo?(kyle)
Um... I thought 64-bit was shipping in 41? Not my call there really. Guessing either bsmedberg or jmoradi can chime in on this.
Flags: needinfo?(kyle)
(In reply to Kohei Yoshino [:kohei] from comment #36)
> [Tracking Requested - why for this release]: win64 builds hits the Release
> channel with Firefox 40 that will be shipped in 2 weeks.

Context: Bug 1180792 and Bug 1181014
This work is planned for 41. We don't need this in 40.
I do not understand this at all.
1. Why does this only affect the 64bit build
2. Why Flash and only just Flash. Silverlight plugins are still available on websites and used in numerous commercial solutions.
3. Why enforce without giving the user the possibility to overrule the preset? This is just crippling functionality!

Very questionable changes here that result in more issues than it solves: bug 1187005
Firefox 64 bit will launch with highly restricted access to NPAPI.

Can i ask why flash is exempt of this restriction?

I mean there's no way for users to add plugins to the whitelist and now all plugins, except flash, are gone.

This is crippling functionality but it's for stability/performance/security reasons so in the long term it'll be ok but flash has demonstrated to have security, performance and stability problems why is still there then?
Once again a very bad move. How Amozon Prime users are supposed to watch videos/movies in Firefox (41+) x64 witout Silverlight plugin?
(In reply to Sergio from comment #43)
> Can i ask why flash is exempt of this restriction?

My guess would be that it's because Flash is still so widespread too many sites would break. Note that both Chrome & Edge support Flash and no other cross-browser plugin. Firefox would fit the narrative.

Of course in the long run it would be good to drop all plugins. But if only Firefox did it it would be at a disadvantage compared to Chrome & Edge.
Flags: needinfo?(benjamin)
Version due to status-firefox41: --- → affected
Version: unspecified → 40 Branch
I apologize for the delay in responding to this bug, but we wanted to be able to make a clear statement in conjunction with partners. 64-bit Firefox for Windows will launch without plugin support other than Flash. We're carefully monitoring the availability of an EME component to enable things like Netflix without Silverlight.

https://blog.mozilla.org/futurereleases/2015/10/08/npapi-plugins-in-firefox/
Status: RESOLVED → VERIFIED
Flags: needinfo?(jmoradi)
(In reply to Ritu Kothari (:ritu) from comment #31)
> QE team could you please verify this fix has no unexpected side effects on a
> nightly or aurora build? Thanks.

Verified using Firefox 42 beta 8 and latest Nightly 44.0a1 (both 64bit build) on Windows 7 64-bit, Windows 8.1 64-bit and Windows 10 64-bit, that only Flash is whitelisted and played properly. 
All other major plugins (Quicktime, Silverlight, Unity and Java) are not listed in about:addons and are not played.
Flags: qe-verify+
Are there technical reasons to disallow other plugins (e.g. I know we have a custom sandbox for 64-bit Flash but not for others) or is this a pure policy decision?

If it's a policy decision, what's the appropriate list/newsgroup for discussing it?
(In reply to Gian-Carlo Pascutto [:gcp] from comment #50)
> Are there technical reasons to disallow other plugins (e.g. I know we have a
> custom sandbox for 64-bit Flash but not for others) or is this a pure policy
> decision?

It is a bit of both. Maintenance on the NPAPI host continues for e10s and performance/stability reasons. The more plugins that are run against the NPAPI host, the more compatibility considerations need to be taken into account. Given that Win64 on release is a brand new plugin ecosystem, we are essentially starting with a clean slate there. Lower plugin diversity = lower support burden.
Silverlight support has been added in Bug 1225293.
As I understand it from everything I have been reading and the information in this thread, this is the reason Java does not work in FF64.  I have to say this is a terrible decision.  In our institution we have several products that rely on Java, so by removing this support we must discontinue use of Firefox unless this is fixed.
(In reply to Yadin from comment #53)
> As I understand it from everything I have been reading and the information
> in this thread, this is the reason Java does not work in FF64.  I have to
> say this is a terrible decision.  In our institution we have several
> products that rely on Java, so by removing this support we must discontinue
> use of Firefox unless this is fixed.

This affects you in negative way but helps lots of people that would otherwise get infected due to Java plugin having numerous security vulnerabilities.

Also, Chrome & Edge are already not supporting Java and Oracle itself announced that they'll deprecate the Java plugin in Java 9 and remove in the future:
https://blogs.oracle.com/java-platform-group/entry/moving_to_a_plugin_free

You should move our organization off Java ASAP, it will soon stop working everywhere.
That's more than a little surprising.  Looks like I'll be calling a lot of vendors to determine how they expect us to use their product all of the sudden if the world has given up on the platform they use.
He's talking about Java plugins (which is what NPAPI is required for). Java itself still works outside the browser as a standalone app.

It also still works in 32-bit Firefox, so it sounds "rather strange" that you have a use case that somehow requires a Firefox release that isn't actively pushed out yet (64-bit) yet have a hard dependency on a plugin that's deprecated by everyone including the vendor.
The appropriate list for this is probably firefox-dev. Please make sure you've read the blog post from comment 47 before starting a new thread.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.