Closed
Bug 757726
Opened 12 years ago
Closed 8 years ago
disallow enumeration of navigator.plugins
Categories
(Core Graveyard :: Plug-ins, defect, P2)
Core Graveyard
Plug-ins
Tracking
(firefox28 disabled, firefox29 disabled, firefox30 disabled, firefox31 disabled)
People
(Reporter: shawnlandden, Assigned: cpeterson)
References
(Blocks 1 open bug, )
Details
(Keywords: addon-compat, feature, site-compat, Whiteboard: [fingerprinting] Websites tech advo bug 934107)
Attachments
(8 files, 11 obsolete files)
11.52 KB,
patch
|
jaas
:
review+
|
Details | Diff | Splinter Review |
16.36 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
3.00 KB,
patch
|
johns
:
review+
|
Details | Diff | Splinter Review |
7.87 KB,
patch
|
johns
:
review+
|
Details | Diff | Splinter Review |
3.81 KB,
patch
|
johns
:
review+
|
Details | Diff | Splinter Review |
23.16 KB,
patch
|
johns
:
review+
|
Details | Diff | Splinter Review |
23.50 KB,
patch
|
benjamin
:
superreview+
|
Details | Diff | Splinter Review |
1.40 KB,
patch
|
benjamin
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This would lower browser entropy, and Internet Explorer does not allow enumeration
A number of common plugins such as Google Talk/Voice and Gnome Extentions manager, are only applicable and loadable on certain web sites, and if any are to be in that list, these should not.
This is one of the few fingerprinting bugs which may actually make a difference and not just have an impact on use cases. Having some "exotic" plug-in installed likely makes one substantially recognizable, and the ability to get a list of plug-ins (assuming that's what enumeration of navigator.plugins entitles) doesn't appear to be necessary. A site may need to know if a /specific/ plug-in is available, and possibly which version of it to provide the respective code, but won't need to know about any other plug-ins which may be installed and enabled.
As for opting out, that would only apply to plug-ins considering this for a future update. Older plug-ins which may no longer be actively maintained to that extent would still be leaked (and those are the ones highly relevant for a fingerprint). Alternatively to that concept, if proceeded with, would be an explicit "Hide" button in the Add-Ons manager to allow the user to remove plug-ins from the listing if the ability to enumerate plug-ins is retained.
Comment 2•12 years ago
|
||
Seems reasonable to limit the ability to enumerate navigator.plugins. Confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 3•12 years ago
|
||
Mozilla's Plugin Check page breaks if navigator.plugins enumeration is disabled. Plugin Check could be fixed by querying navigator.plugins for known plugins. We should probably not disable navigator.plugins enumeration until Plugin Check is fixed.
Disabling navigator.plugins enumeration also breaks about:plugins (but not the about:addons Plugins page).
https://www.mozilla.org/en-US/plugincheck/
Comment 4•12 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #3)
> Mozilla's Plugin Check page breaks if navigator.plugins enumeration is
> disabled.
That it is possible to write an unprivileged Web app that scans for vulnerable plug-in versions is part of the problem here. I think it would be a sensible long-term goal to make that impossible, which implies implementing plugincheck inside Firefox or as a privileged Web app.
Doing this would impact our recommended solution for dealing with click-to-play from web content. See this discussion:
http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-June/036359.html
Comment 6•12 years ago
|
||
I'm not sure why that directly affects this: the recommended CTP solution involves querying particular plugins from navigator.plugins, not enumerating them, unless I misunderstand it.
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Henri Sivonen (not reading bugmail until 2012-09-17) (:hsivonen) from comment #2)
> Seems reasonable to limit the ability to enumerate navigator.plugins.
Also note that preventing enumeration of navigator.plugins is not adequate. We would also need to prevent enumeration of navigator.mimeTypes because web apps can retrieve the same plugin info from navigator.mimeTypes' MimeType.enabledPlugin.
Assignee | ||
Comment 8•12 years ago
|
||
This WIP patch prevents the navigator.plugins and navigator.mimeTypes arrays from being enumerated by index. Web content can still query for plugins and mimeTypes by name (e.g. navigator.plugins["Shockwave Flash"] or navigator.mimeTypes["video/mp4"]).
Unfortunately,
1. This patch breaks about:plugins. This could be fixed by allowing local files or chrome files to enumerate navigator.plugins.
2. This patch breaks Mozilla's plugincheck site. plugincheck could be fixed by querying navigator.plugins for all known plugins by name. Alternately, plugincheck could be implemented as a native browser feature of about:addons.
Attachment #664380 -
Flags: feedback?(joshmoz)
Assignee | ||
Comment 9•12 years ago
|
||
I opened bug 793978 (randomly shuffle navigator.plugins array) as an interim alternative fix.
Depends on: 793978
Summary: disallow enumeration navigator.plugins, or at least allow plugins to opt out of enumeration → disallow enumeration of navigator.plugins
Comment 10•12 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #8)
> Alternately,
> plugincheck could be implemented as a native browser feature of about:addons.
plugincheck is supposed to work on non-Gecko browsers.
Comment 11•12 years ago
|
||
Thanks, Chris, for your efforts. I think fixing this bug here immediately is the better route.
> implementing plugincheck inside Firefox
[fixing about:plugins]
+1 That's trivial, and should have been done like that from the start.
Comment 12•12 years ago
|
||
> plugincheck is supposed to work on non-Gecko browsers.
If we want that, we can still have a webpage for other browsers, and direct Firefox users (detected) to our UI. I'd think it's low prio, though.
Comment 13•12 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #6)
> I'm not sure why that directly affects this: the recommended CTP solution
> involves querying particular plugins from navigator.plugins, not enumerating
> them, unless I misunderstand it.
The actual recommendation for web developers is in this email:
http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-June/036370.html
The developer needs to be able to check whether or not a MIME type is supported by a plugin. That would involve enumerating, wouldn't it?
Comment 14•12 years ago
|
||
> The developer needs to be able to check whether or not a MIME type is supported
> by a plugin. That would involve enumerating, wouldn't it?
1. MIME types IMHO shouldn't be checked. The browser can't and shouldn't list all applications available on the local system.
2. As mentioned above, querying "is A available" or ("is A available in version >= N") is a different question than "give me all available".
Comment 15•12 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #14)
> 1. MIME types IMHO shouldn't be checked. The browser can't and shouldn't
> list all applications available on the local system.
It's not applications on the local system, it's types the browser supports.
> 2. As mentioned above, querying "is A available" or ("is A available in
> version >= N") is a different question than "give me all available".
That's trivially worked around by anyone with malicious intent via the creation of a small database to check.
I'm not sure any of this is relevant though. All we really need for the click-to-play recommendation to work is for a page to be able to get an answer to the question "Do any enabled plugins (c2p aside) support MIME type x/foo?".
Comment 16•12 years ago
|
||
I don't know what we're arguing about. The question script ought to be asking is
'application/x-shockwave-flash' in navigator.mimeTypes
We aren't proposing removing that feature, just the enumeration. Although fingerprinting users would still be pretty damn easy by having an array of possibilities and checking them, so I'm not sure how much benefit you actually get out of it.
Assignee | ||
Comment 17•12 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #11)
> > implementing plugincheck inside Firefox
> [fixing about:plugins]
>
> +1 That's trivial, and should have been done like that from the start.
Ben, do you recommend that all local pages be allowed or just chrome? What is the best way to check those conditions? I wanted to post this patch for feedback before I investigated further.
(In reply to Josh Aas (Mozilla Corporation) from comment #15)
> I'm not sure any of this is relevant though. All we really need for the
> click-to-play recommendation to work is for a page to be able to get an
> answer to the question "Do any enabled plugins (c2p aside) support MIME type
> x/foo?".
Josh, if web content only needs to know if a given MIME type is supported, can we remove the navigator.plugins array and the navigator.mimeTypes['some/mime-type'].enabledPlugin field entirely?
Comment 18•12 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #17)
> Josh, if web content only needs to know if a given MIME type is supported,
> can we remove the navigator.plugins array and the
> navigator.mimeTypes['some/mime-type'].enabledPlugin field entirely?
Maybe, so long as you can deal with the legacy compat problems well enough. That will probably be tough.
Comment 19•12 years ago
|
||
> Ben, do you recommend that all local pages be allowed or just chrome? What is the best way
> to check those conditions? I wanted to post this patch for feedback before I investigated further.
Does the difference matter? I'd make it chrome only. There are a number of about: pages with chrome rights, in fact most of them, e.g. about:chrome, about:support etc. The latter is particularly relevant, because it already lists extensions. I'd copy that and modify it to list only plugins.
Comment 20•12 years ago
|
||
Comment on attachment 664380 [details] [diff] [review]
WIP-prevent-plugin-enumeration.patch
Review of attachment 664380 [details] [diff] [review]:
-----------------------------------------------------------------
This should be fine in terms of still allowing people to reasonably work with click-to-play scenarios. Please make sure your final solution doesn't break about:plugins. There's always a chance this will cause too much breakage on the web, but lets give it a shot.
Attachment #664380 -
Flags: feedback?(joshmoz) → feedback+
Assignee | ||
Comment 21•12 years ago
|
||
Part 2: about:plugins must enumerate plugins using AddonManager, not navigator.plugins.
Since navigator.plugins would no longer be enumerable, about:plugins must query the AddonManager for the list of installed plugins.
*** This patch fixes about:plugins, but https://www.mozilla.org/plugincheck would still be broken. Plugin Check would need to be modified so it enumerates its list of known plugins to query navigator.plugins by name, instead of enumerating navigator.plugins to query the list of known plugins.
https://wiki.mozilla.org/PFS2
Assignee: nobody → cpeterson
Attachment #674153 -
Flags: feedback?(joshmoz)
Assignee | ||
Comment 22•12 years ago
|
||
btw, because AddonManager.getAddonsByTypes() is asynchronous, the callback function to write the plugin list can no longer use document.write() because the HTML document has already finished loading.
Comment 23•12 years ago
|
||
Re patch 2:
I don't want to delay this bug, but given that you're pretty much rewriting about:plugins anyway:
It would be better to use DOM manipulation (doc.appendChild(doc.createElement("div")) instead of that document.write() or innerHTML. This is particularly important as this is running as Chrome and importing data from plugins. Yes, they are running on the host anyway, but better safe than sorry. If nothing else, that prevents accidental messup of the page layout.
Comment 24•12 years ago
|
||
Comment on attachment 674153 [details] [diff] [review]
part-2-fix-about-plugins.patch
Review of attachment 674153 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/content/plugins.html
@@ +63,2 @@
>
> + // Alphabetize plugin list.
We don't currently display in alphabetical order. We sort by most recent modification date (most recent on top). That usually means the most recently updated plugin will be on top. I'd like to keep it this way. The default order you get from Gecko might just be that.
Attachment #674153 -
Flags: feedback?(joshmoz) → feedback+
Assignee | ||
Comment 26•12 years ago
|
||
plugins.html patch v2:
* Enumerate plugins using AddonManager module (because navigator.plugins won't be available after the next patch is applied)
* Remove alphabetization of plugins
* Replace document.write() with document.createElement()
* Use strict mode
* Add some more comments
This is a standalone patch. It does not depend on the second patch (but the second patch depends on this one). AFAICT, the page layout looks identical to the current about:plugins page.
Attachment #664380 -
Attachment is obsolete: true
Attachment #674153 -
Attachment is obsolete: true
Attachment #693268 -
Flags: review?(joshmoz)
Assignee | ||
Comment 27•12 years ago
|
||
Prevent web content from enumerating navigator.plugins and navigator.mimeTypes.
With patch 1 and 2 applied, about:plugins works correctly, but Mozilla's Plugin Check service is broken!
Plugin Check depends on enumerating navigator.plugins, which would no longer be supported. Plugin Check would need to query navigator.plugins for known plugin names. In fact, plugincheck.js already does something like this for IE (because IE did not even have a navigator.plugins object until IE9).
Who owns the Plugin Check service and PFS2?
Attachment #693273 -
Flags: feedback?(joshmoz)
Comment 28•12 years ago
|
||
Comment on attachment 693273 [details] [diff] [review]
part-2-prevent-plugin-enumeration-v2.patch
Review of attachment 693273 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/base/nsDOMClassInfo.cpp
@@ +9867,5 @@
> {
> + // Prevent web content from enumerating the navigator.plugins array to avoid
> + // leaking fingerprintable entropy. Content can still query plugins by name.
> + *aResult = NS_ERROR_NOT_IMPLEMENTED;
> + return nullptr;
Sounds like this will make navigator.plugins[0] will throw; it should return undefined instead.
Comment 29•12 years ago
|
||
> * Replace document.write() with document.createElement()
Your new patch resolves comment 23. Thank you!
Assignee | ||
Comment 30•12 years ago
|
||
(In reply to :Ms2ger from comment #28)
> Sounds like this will make navigator.plugins[0] will throw; it should return
> undefined instead.
oops, you're correct. I changed NS_ERROR_NOT_IMPLEMENTED to NS_OK so navigator.plugins[0] and navigator.mimeTypes[0] will quietly return undefined.
Attachment #693273 -
Attachment is obsolete: true
Attachment #693273 -
Flags: feedback?(joshmoz)
Attachment #693793 -
Flags: feedback?(joshmoz)
Comment 31•12 years ago
|
||
I would like to test this feature. When will this be possible? Are there dev builds for this purpose or I just need to wait till next version of Firefox gets released?
Reporter | ||
Comment 32•12 years ago
|
||
Valent, This hasn't landed yet, nor AFAIK been submitted for a try build, so you would have to build it yourself. You can get instructions https://developer.mozilla.org/en-US/docs/Simple_Firefox_build
And if you have difficulty building, feel free to stop by #introduction on irc.mozilla.org irc://irc.mozilla.org/#introduction
Reporter | ||
Comment 33•12 years ago
|
||
You can also get the source code via git https://github.com/mozilla/mozilla-central
if you need a run-down on how to check out the source code and commit the proposed patches see http://git-scm.com/ (perhaps I should teach that git class someone has been pestering me to do...)
Comment 34•12 years ago
|
||
(In reply to Valent from comment #31)
> I would like to test this feature. When will this be possible? Are there dev
> builds for this purpose or I just need to wait till next version of Firefox
> gets released?
If you just want to test this as once it is checked in / landed, you can use the Nightly builds:
http://nightly.mozilla.org/
Comment 35•12 years ago
|
||
While this patch as implemented would raise the bar somewhat, it doesn't seem like it'd really make the fingerprinting impossible. You'd just have to build a big list of possible plugins, and query for all of them. I suppose this is impossible to prevent in reality, because you can always just try instantiating each plugin in turn in an <object> and see if it loads.
Comment 36•12 years ago
|
||
Yes, but that's part of the trade-off as I'd understand it. Web sites may still need to know if a specific plugin is installed but would no longer be able to see the complete list in a potentially user-specific order, thus fingerprinting is reduced even though not prevented. Such a test list would less likely contain infrequently used or specialized plug-ins with a high fingerprinting value, and more likely contain mainstream plugins frequently installed and thus with less value for fingerprinting. So, overall it helps.
Assignee | ||
Comment 37•12 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #35)
> While this patch as implemented would raise the bar somewhat, it doesn't
> seem like it'd really make the fingerprinting impossible. You'd just have to
> build a big list of possible plugins, and query for all of them.
We would need to do this ourselves to support plugin version check. However, this is not a big problem for us because our plugin check already has a (server-side) list of known plugins.
One benefit would be that fingerprinters' plugin lists would not include every plugin known to humanity. Rare plugins increase a user's uniqueness. For example, I have the Wacom Table Plugin installed, which fingerprinters may not think to query (though they would probably use our plugin check's plugin list, which does know about the Wacom Tablet Plugin).
Another benefit would be that the querying plugins does not reveal the *order* in which users installed their plugins. Many people have Flash, Java, QuickTime, and Silverlight installed, but revealing the order in which the user installed them increases the number of "user buckets" from 1 to factorial(4) -> 24 subgroups! I opened bug 793978 with a patch that shuffles the navigator.plugin list as a stop-gap measure to hide the plugin installation order, but the bug was closed as WONTFIX.
Comment 38•12 years ago
|
||
Comment on attachment 693793 [details] [diff] [review]
part-2-prevent-plugin-enumeration-v3.patch
Review of attachment 693793 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/plugins/test/mochitest/test_secondPlugin.html
@@ +22,3 @@
> for (var index in navigator.plugins) {
> + // Plugin enumeration *will* see "Test Plug-in" because we have already queried it by name.
> + isnot(navigator.plugins[index].name, "Second Test Plug-in", "Plugin enumeration should not see 'Second Test Plug-in'");
I don't understand how this works with your patch. How does enumeration work for a plugin just because you queried for it by name once? Your enumeration method never returns a result.
Attachment #693793 -
Flags: feedback?(joshmoz) → feedback+
Attachment #693268 -
Flags: review?(joshmoz) → review+
Updated•12 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 39•12 years ago
|
||
I landed patch 1 on inbound. It changes how the about:plugins page constructs the plugin list, but should not introduce any functional changes.
https://hg.mozilla.org/integration/mozilla-inbound/rev/42b22f0ab58c
Status: NEW → ASSIGNED
Whiteboard: [leave open]
Assignee | ||
Comment 40•12 years ago
|
||
(In reply to Josh Aas (Mozilla Corporation) from comment #38)
> ::: dom/plugins/test/mochitest/test_secondPlugin.html
> @@ +22,3 @@
> > for (var index in navigator.plugins) {
> > + // Plugin enumeration *will* see "Test Plug-in" because we have already queried it by name.
> > + isnot(navigator.plugins[index].name, "Second Test Plug-in", "Plugin enumeration should not see 'Second Test Plug-in'");
>
> I don't understand how this works with your patch. How does enumeration work
> for a plugin just because you queried for it by name once? Your enumeration
> method never returns a result.
That's a good question. I don't know exactly. navigator.plugins seems to be remembering the the item names that nsPluginArraySH::GetNamedItem() returns.
Before this test calls navigator.plugins["Test Plug-in"], for..in of navigator.plugins will return an Iterator that queries "namedItem" and "refresh" items. After calling navigator.plugins["Test Plug-in"], for..in of navigator.plugins will return an Iterator that queries "Test Plugin-in", "namedItem", and "refresh" items. Since the second Iterator knows the "Test Plugin-in" item name, the for..in loop will look up the plugin by name.
Comment 41•12 years ago
|
||
Comment 42•12 years ago
|
||
Comment 43•12 years ago
|
||
Comment 44•12 years ago
|
||
I expect this to cause web compatibility problems. Many sites enumerate just to check whether you have a specific plugin installed:
https://www.google.com/search?q=%22for+*+in+navigator.plugins%22
Sorting or shuffling the array (bug 793978) would get us most of the privacy benefit without affecting web compatibility.
Updated•12 years ago
|
relnote-firefox:
--- → ?
Assignee | ||
Comment 45•12 years ago
|
||
(In reply to Jesse Ruderman from comment #44)
> I expect this to cause web compatibility problems. Many sites enumerate
> just to check whether you have a specific plugin installed:
>
> https://www.google.com/search?q=%22for+*+in+navigator.plugins%22
>
> Sorting or shuffling the array (bug 793978) would get us most of the privacy
> benefit without affecting web compatibility.
I have a small patch on bug 793978 that shuffles the plugins array, if anyone wants to r+ it. :) It doesn't require fixing the Plugin Check website or risk (much) web compatibility.
Most users have the same few plugins installed. Without sorting or shuffling the plugins array, those users can be divided into N! subgroups by the particular order in which they installed those plugins.
Comment 46•12 years ago
|
||
(In reply to Jesse Ruderman from comment #44)
> Sorting or shuffling the array (bug 793978) would get us most of the privacy
> benefit without affecting web compatibility.
Would be nice, if it did, but unfortunately, shuffling doesn't "get us most of the privacy benefit". The existence of somewhat rare plugins is the most revealing and problematic.
Shuffling only removes one bug we have - that the site sees the order of installation -, but not the general problem, the wealth of information available, some of it quite revealing.
Updated•12 years ago
|
Updated•12 years ago
|
Comment 47•12 years ago
|
||
Why relnote-, given the potential to break existing websites?
Flags: needinfo?(bbajaj)
Comment 48•12 years ago
|
||
1) this patch hasn't landed and it's not clear that it *can* land
2) We typically relnote end-user issues, not webdeveloper issues. Developer-facing features end up on the MDN "Firefox N for developers" page.
Comment 49•12 years ago
|
||
I'd also be highly concerned about the webcompat impact of this. It's also trivial to work around, per comment 35. So it's all downside, no actual upside. WONTFIX?
However... It might be interesting to think about going in a somewhat different direction: only list the top-N plugins in navigator.plugins. There are probably a bajillion pages out there detecting major plugins like Flash and Java in delightfully fragile ways, and breaking those would be a big problem. But breaking detection of uncommon plugins might be an acceptable risk?
Assignee | ||
Comment 50•12 years ago
|
||
I like your suggestion! Listing only the top-N plugins *and* shuffling (or sorting) the order in they are enumerated (bug 793978) addresses both fingerprinting concerns: the existence of rare plugins and the order in which plugins were installed.
Do we have metrics (from Telemetry or Plugin Check) of the top-N plugins? Web pages designed to use uncommon plugins probably have more robust plugin checks because they must handle failure conditions more often.
Comment 51•12 years ago
|
||
(In reply to Justin Dolske [:Dolske] from comment #49)
> only list the top-N plugins in navigator.plugins. There
> are probably a bajillion pages out there detecting major plugins like Flash
> and Java in delightfully fragile ways, and breaking those would be a big
> problem. But breaking detection of uncommon plugins might be an acceptable
> risk?
+1
Comment 52•12 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #50)
> Do we have metrics (from Telemetry or Plugin Check) of the top-N plugins?
We don't have Telemetry data (privacy concerns), not sure about "Plugin Check" although i'd assume the same problem there. A test-pilot study is planned though.
I'm personally only aware of the top-plugin numbers collected via Chrome:
http://blog.chromium.org/2012/07/npapi-plug-ins-in-windows-8-metro-mode.html
Updated•12 years ago
|
Flags: needinfo?(bbajaj)
Comment 53•12 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #50)
>
> Do we have metrics (from Telemetry or Plugin Check) of the top-N plugins?
> Web pages designed to use uncommon plugins probably have more robust plugin
> checks because they must handle failure conditions more often.
we have these metrics, I suggest contacting Kris Maglione who has been helping security engineering with these same sort of questions :)
Comment 54•12 years ago
|
||
Afaik, the fingerprinting problem also goes for the addons, not only the plugins. I would highly appreciate if the fingerprinting surface could be controlled, and esp. limited, by the user, and in a way that prevents malicious code from circumventing the measure.
Comment 55•11 years ago
|
||
What if the browser detected when a page tried to use enumeration, and offered the user the choice whether to add that page to a whitelist of sites that are allowed to enumerate; mentioning the security reasons for not allowing enumeration and reminding it should only be necessary in case the site uses the "old" method of detecting plugins an fails to detect a plugin you do got installed?
Comment 56•11 years ago
|
||
This isn't something that you can explain an ordinary user, it's a too fine point. Also, he shouldn't be bothered with this, it should just work (in the sense of: pages work, but privacy is protected).
Comment 57•11 years ago
|
||
I agree with BenB, this is not a decision that even most informed users can really be expected to make. I do think we should try to proceed with limiting enumeration to a small set of well-known plugins/MIME types. We'll probably have to control that with a hidden pref in case we break some edge case.
Priority: -- → P2
Comment 58•11 years ago
|
||
I have submitted a chromium issue detailing this privacy problem:
https://code.google.com/p/chromium/issues/detail?id=271772
Assignee | ||
Comment 60•11 years ago
|
||
Part 2: Populate Preferences' Applications list using PluginHost
The Preferences window's Applications tab is populated using navigator.plugins, which won't include hidden-but-enabled plugins after the subsequent patches. This patch preemptively refactors the Preferences to query the PluginHost service (for PluginTag objects) instead of navigator.plugins.
This patch also changes HandlerInfoWrapper's `plugin` property to `pluginName` because HandlerInfoWrapper didn't use any other property of plugin objects. There's no need to keep a PluginTag object alive with an extra reference. Also, the _loadPluginHandlers() function is now enumerating PluginTag objects instead of Plugin objects, which are not interchangable.
https://tbpl.mozilla.org/?tree=Try&rev=bbd86e835aac
Attachment #693793 -
Attachment is obsolete: true
Attachment #820124 -
Flags: review?(benjamin)
Assignee | ||
Updated•11 years ago
|
Attachment #820124 -
Attachment description: 757726-part-2-fix-Preferences-Applications.patch → part-2-fix-Preferences-Applications.patch
Assignee | ||
Comment 61•11 years ago
|
||
Part 3: Refactor nsPluginArray::GetPlugins() to GetMimeTypes()
This nsPluginArray refactoring removes an unnecessary nsTArray copy and makes the subsequent patches simpler.
Attachment #820126 -
Flags: review?(joshmoz)
Assignee | ||
Comment 62•11 years ago
|
||
Part 4: Consolidate duplicate code into EnsurePlugins() and EnsureMimeTypes()
This refactoring consolidates some duplicate code into nsPluginArray::EnsurePlugins() and nsMimeTypeArray::EnsureMimeTypes() to make subsequent patches simpler.
Attachment #820127 -
Flags: review?(joshmoz)
Assignee | ||
Comment 63•11 years ago
|
||
Part 5: Hide most plugins from navigator.plugins and navigator.mimeTypes enumeration
This patch implements plugin hiding as described in the HTML Living Standard:
http://www.whatwg.org/specs/web-apps/current-work/multipage/timers.html#dom-navigator-plugins
> A PluginArray object represents none, some, or all of the plugins
> supported by the user agent, each of which is represented by a Plugin
> object. Each of these Plugin objects may be hidden plugins. A hidden
> plugin can't be enumerated, but can still be inspected by using its name.
WARNING! This patch breaks Mozilla's Plugin Check service! Plugin Check enumerates navigator.plugins then uploads the plugin names to the server to be compared to a database of known plugin versions. Since this patch hides most plugins from navigator.plugins enumeration, Plugin Check won't be able to enumerate.
Possible workarounds include:
* Plugin Check could download the plugin data to the client so JS can query every known plugin by name without enumerating navigator.plugins.
* Unhide all plugins known by Plugin Check. Unfortunately, Plugin Check knows about 45 plugins, even though some of them have not been updated since 2010. I demonstrate this workaround in a subsequent patch.
* The real solution: Firefox can implement Plugin Check natively so it has full access to the plugins and the user doesn't need to manually check whether their plugins are outdated.
Based on comment 49, the current list of non-hidden plugins includes Flash, Java, and "Second Test Plug-in". I have exposed "Second Test Plug-in" for expediency in this patch. In a subsequent patch, I remove "Second Test Plug-in" from the list of non-hidden plugins and change all these test files.
The nsMimeTypes::mPluginMimeTypeCount member variable is no longer necessary. It was used to prevent ItemGetter() from indexing into mMimeTypes's MIME types handled by applications. Those MIME types are now stored in mHiddenMimeTypes, so there is no extra check needed to prevent ItemGetter() from indexing them.
TBD:
* Which plugins must be unhidden? Reasons we would want to unhide a plugin:
1. To avoid breaking poorly-written web content that inefficiently enumerate navigator.plugins, hunting for plugin.name == "Shockwave Flash", instead of just querying navigator.plugins["Shockwave Flash"].
2. Some plugins names contain version numbers (e.g. "Adobe Acrobat NPAPI Plug-in, Version 11.0.04" or "QuickTime Plug-in 7.7.1"), so web content can't query navigator.plugins without knowing the full plugin name including version name. Web content might have been instead enumerating navigator.plugins and comparing string prefixes. Web content should probably just query navigator.mimeTypes for the plugin MIME type it cares about.
* Plugin Check currently uses the following plugin name patterns to determine whether any flavor of Java is installed. AFAICT, Oracle's Java plugin name is "Java Applet Plug-in". Can we unhide just Oracle's plugin or should we unhide all these Java flavors?
"IcedTea Java Web Browser Plugin"
"Java Runtime Environment"
"Java Embedding Plugin .*"
"Java.* Platform SE.*"
".*Java Deployment Toolkit.*"
"Java.* Plug-.*"
https://tbpl.mozilla.org/?tree=Try&rev=040b99a95f20
Attachment #820130 -
Flags: review?(joshmoz)
Assignee | ||
Comment 64•11 years ago
|
||
Part 6: Unhide all plugins known to Mozilla's Plugin Check service
This patch fixes Plugin Check, but weakens the fingerprinting protection of hiding plugins in the first place! Here is the list of the (45) plugins known to Plugin Check:
https://plugins.mozilla.org
btw, here are the plugins I have installed that Plugin Check does NOT recognize, so at least these uncommon plugins would be hidden by this patch:
* AdobeExManDetect
* Adobe Acrobat NPAPI Plug-in, Version 11.0.04
* Google Earth Plug-in
* Google Talk Plugin Video Renderer
We could deploy these patches incrementally, hiding more plugins every Firefox release. Maybe something like:
* Firefox 28: Hide all plugins except Plugin Check plugins
* Firefox 29: Hide all plugins except Plugin Check plugins updated in 2013
* Firefox 30: Hide all plugins except Flash and Java
* Firefox 31: Hide all plugins except Flash
* Firefox 32: Hide all plugins
Attachment #820132 -
Flags: feedback?(joshmoz)
Assignee | ||
Comment 65•11 years ago
|
||
Part 7: Rename "Second Test Plug-in" to "Second (Not Java) Test Plug-in".
Add the keyword "Java" to the plugin name so it will be unhidden based on the plugin name rules. I can then remove the "Second Test Plug-in" special case from nsPluginArray's list of unhidden plugins.
Unfortunately, this new plugin name must be changed in many test files. Is this worth the churn? Or can I just leave "Second Test Plug-in" in the plugin list? <:)
Attachment #820133 -
Flags: feedback?(joshmoz)
Comment 66•11 years ago
|
||
> * Plugin Check could download the plugin data to the client so JS can query every known plugin by name without enumerating navigator.plugins.
I like that idea.
> * The real solution: Firefox can implement Plugin Check natively
I filed bug 929474 about this.
Comment 67•11 years ago
|
||
Comment on attachment 820126 [details] [diff] [review]
part-3-refactor-GetPlugins-to-GetMimeTypes.patch
Review of attachment 820126 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/base/nsPluginArray.cpp
@@ +82,5 @@
>
> + for (uint32_t i = 0; i < mPlugins.Length(); ++i) {
> + nsPluginElement *plugin = mPlugins[i];
> + aMimeTypes.AppendElements(plugin->MimeTypes());
> + }
I understand that this method doesn't change behavior, but are we sure we want to include duplicate MIME types? Just checking.
Attachment #820126 -
Flags: review?(joshmoz) → review+
Attachment #820127 -
Flags: review?(joshmoz) → review+
Comment 68•11 years ago
|
||
Comment on attachment 820130 [details] [diff] [review]
part-5-hide-plugins.patch
Review of attachment 820130 [details] [diff] [review]:
-----------------------------------------------------------------
I think we should rename "EnsureMimeTypes" to indicate that it only deals with plugin mime types.
This is DOM code, so I can't r+, please get that from jst or jschoenick.
::: dom/base/nsMimeTypeArray.h
@@ +49,5 @@
> // mMimeTypes contains all mime types handled by plugins followed by
> // any other mime types that we handle internally and have been
> // looked up before.
> nsTArray<nsRefPtr<nsMimeType> > mMimeTypes;
> + nsTArray<nsRefPtr<nsMimeType> > mHiddenMimeTypes;
Document the difference between mMimeTypes and mHiddenMimeTypes here. Why would something be in one vs. the other? It seems like one holds plugin MIME types and the other holds helper application MIME types, is that perhaps a better distinction to make in the name than visible/hidden?
::: dom/base/nsPluginArray.cpp
@@ +272,5 @@
> + const nsCString& pluginName = pluginTag->mName;
> +
> + return !pluginName.Equals("Shockwave Flash") &&
> + !pluginName.Equals("Java Applet Plug-in") &&
> + !pluginName.Equals("Second Test Plug-in"); // UNHIDE PLUGIN FOR TESTS
nsPluginTag has 'mIsJavaPlugin' and 'mIsFlashPlugin' because we special-case these elsewhere as well. Use that, it's probably more robust and we're not duplicating code.
::: dom/base/nsPluginArray.h
@@ +61,5 @@
> void EnsurePlugins();
>
> nsCOMPtr<nsPIDOMWindow> mWindow;
> nsTArray<nsRefPtr<nsPluginElement> > mPlugins;
> + nsTArray<nsRefPtr<nsPluginElement> > mHiddenPlugins;
Explain the difference here in the header please.
Attachment #820130 -
Flags: review?(joshmoz) → feedback+
Comment 69•11 years ago
|
||
Comment on attachment 820132 [details] [diff] [review]
part-6-unhide-PluginCheck-plugins.patch
Review of attachment 820132 [details] [diff] [review]:
-----------------------------------------------------------------
I'm going to be offline (vacation) for a week+. Please ask bsmedberg, jst, or jschoenick about the rest of this.
Attachment #820132 -
Flags: feedback?(joshmoz)
Attachment #820133 -
Flags: feedback?(joshmoz)
Comment 70•11 years ago
|
||
In bug 558184 I'm going to make these classes only use nsIPluginTag to allow multiple implementors of tags, which I fear is going to replace all this nice cleanup with more heinous XPCOM array manipulation :( (You should make sure to land this first, and then look away)
(In reply to Josh Aas (Mozilla Corporation) from comment #68)
> I think we should rename "EnsureMimeTypes" to indicate that it only deals
> with plugin mime types.
>
> This is DOM code, so I can't r+, please get that from jst or jschoenick.
>
> ::: dom/base/nsMimeTypeArray.h
> @@ +49,5 @@
> > // mMimeTypes contains all mime types handled by plugins followed by
> > // any other mime types that we handle internally and have been
> > // looked up before.
> > nsTArray<nsRefPtr<nsMimeType> > mMimeTypes;
> > + nsTArray<nsRefPtr<nsMimeType> > mHiddenMimeTypes;
>
> Document the difference between mMimeTypes and mHiddenMimeTypes here. Why
> would something be in one vs. the other? It seems like one holds plugin MIME
> types and the other holds helper application MIME types, is that perhaps a
> better distinction to make in the name than visible/hidden?
>
> ::: dom/base/nsPluginArray.cpp
> @@ +272,5 @@
> > + const nsCString& pluginName = pluginTag->mName;
> > +
> > + return !pluginName.Equals("Shockwave Flash") &&
> > + !pluginName.Equals("Java Applet Plug-in") &&
> > + !pluginName.Equals("Second Test Plug-in"); // UNHIDE PLUGIN FOR TESTS
>
> nsPluginTag has 'mIsJavaPlugin' and 'mIsFlashPlugin' because we special-case
> these elsewhere as well. Use that, it's probably more robust and we're not
> duplicating code.
For the flash/java stuff, I also ran into this in 558184 and added a nsPluginHost::IsSpecialPluginType call, I can split that out and post it here if it would be useful.
(In reply to Chris Peterson (:cpeterson) from comment #64)
> * Firefox 28: Hide all plugins except Plugin Check plugins
> * Firefox 29: Hide all plugins except Plugin Check plugins updated in 2013
> * Firefox 30: Hide all plugins except Flash and Java
> * Firefox 31: Hide all plugins except Flash
> * Firefox 32: Hide all plugins
This seems more silly. How many plugins does plugin check know about? Wouldn't it be much simpler to just have it query a list of known plugins? Java and Flash are going to be in any list of potential plugins for even the most trivial fingerprinting code, so removing them from enumeration would just be prompting any existing fingerprinting implementations to re-examine their code and see the need to include a list of plugins -- while potentially breaking a lot of sites.
Comment 71•11 years ago
|
||
> * Firefox 28: Hide all plugins except Plugin Check plugins
> * Firefox 29: Hide all plugins except Plugin Check plugins updated in 2013
> * Firefox 30: Hide all plugins except Flash and Java
> * Firefox 31: Hide all plugins except Flash
> * Firefox 32: Hide all plugins
This is not reasonable. Plugincheck doesn't need enumeration at all (in theory... it would need to be recoded to look for specific plugins by MIME type or name).
But external sites depend on enumeration for detection of Flash, Silverlight, and Java, and we must at least include these three basically permanently. Probably also shockwave and quicktime. We also need to understand whether existing google earth, google talk or unity player scripts stop working if we remove them from enumeration.
Assignee | ||
Comment 72•11 years ago
|
||
(In reply to John Schoenick [:johns] from comment #70)
> This seems more silly. How many plugins does plugin check know about?
> Wouldn't it be much simpler to just have it query a list of known plugins?
I agree, but I included this Plugin Check whitelist so these patches could land without being blocked on a Plugin Check rewrite.
When I spoke with the Plugin Check team in March 2013, they were reluctant to rewrite the site to use a client-side list (mostly for code and list maintenance reasons). However, they agreed that the list of plugin names and versions *itself* is not a secret. There is no security risk in making the list publicly available for clients because the same information is available from other sources.
Here is a list of the (45) plugins that Plugin Check currently knows about:
https://plugins.mozilla.org/
Assignee | ||
Updated•11 years ago
|
See Also: → jsplugins-base
Assignee | ||
Comment 73•11 years ago
|
||
Comment on attachment 820126 [details] [diff] [review]
part-3-refactor-GetPlugins-to-GetMimeTypes.patch
Review of attachment 820126 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/base/nsPluginArray.cpp
@@ +82,5 @@
>
> + for (uint32_t i = 0; i < mPlugins.Length(); ++i) {
> + nsPluginElement *plugin = mPlugins[i];
> + aMimeTypes.AppendElements(plugin->MimeTypes());
> + }
Can I defer the removal of duplicate MIME types to a follow-up bug? I'd rather not pile on more changes to this patch queue. <:)
Assignee | ||
Comment 74•11 years ago
|
||
oops. Comment 73 was supposed to be a reply to comment 67:
(In reply to Josh Aas (Mozilla Corporation) from comment #67)
> Comment on attachment 820126 [details] [diff] [review]
> part-3-refactor-GetPlugins-to-GetMimeTypes.patch
>
> I understand that this method doesn't change behavior, but are we sure we
> want to include duplicate MIME types? Just checking.
Can I defer the removal of duplicate MIME types to a follow-up bug? I'd rather not pile on more changes to this patch queue (that I started 13 months ago). <:)
Updated•11 years ago
|
Attachment #820124 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 75•11 years ago
|
||
Part 3: Refactor nsPluginArray::GetPlugins() to GetMimeTypes().
I did not remove duplicate MIME types from different plugins because that is the existing behavior.
Attachment #820126 -
Attachment is obsolete: true
Attachment #825121 -
Flags: review?(jschoenick)
Assignee | ||
Comment 76•11 years ago
|
||
Part 4: Consolidate duplicate code into EnsurePlugins() and EnsureMimeTypes().
As per review comments in comment 68:
* Renamed nsMimeTypeArray::EnsureMimeTypes() to nsMimeTypeArray::EnsurePluginMimeTypes();
* Renamed nsPluginElement::EnsureMimeTypes() to nsPluginElement::EnsurePluginMimeTypes();
Attachment #825122 -
Flags: review?(jschoenick)
Assignee | ||
Updated•11 years ago
|
Attachment #820127 -
Attachment is obsolete: true
Assignee | ||
Comment 77•11 years ago
|
||
Part 5: Hide most plugins from navigator.plugins and navigator.mimeTypes enumeration.
As per review comments in comment 68:
* Added comments about mPlugins and mHiddenPlugins
* Added comments about mMimeTypes and mHiddenMimeTypes
* Check nsPluginTag's mIsFlashPlugin and mIsJavaPlugin instead of the plugin name
* Renamed IsPluginHidable() to IsSpecialPluginType() to mirror jschoenick's WIP (comment 70)
As per review comments in comment 71:
* Added QuickTime to the list of visible plugins (because Apple's iTunes Movie Trailers breaks without it).
* I did NOT unhide the following plugins:
** Silverlight, because Netflix streaming and the silverlightshow.net demos work for me when plugin is hidden.
** Google Earth, because Google's demos work for me when plugin is hidden: http://www.google.com/earth/explore/products/plugin.html
** Unity because Unity's demos work for me when plugin is hidden: http://unity3d.com/gallery/demos/live-demos
** Shockwave (Director), because I can't get ANY Shockwave content to work in any browser on my OS X 10.9 MacBook Pro because of missing Carbon symbols! http://www.adobe.com/shockwave/welcome/
Attachment #820130 -
Attachment is obsolete: true
Attachment #825124 -
Flags: review?(jschoenick)
Comment 78•11 years ago
|
||
I don't think that "not working on mac" is the right data for dealing with Shockwave. Shockwave may only work on windows, but the people who want it won't do without it. We need to understand whether common shockwave-based sites use enumeration in order to make that call.
Assignee | ||
Updated•11 years ago
|
Attachment #825124 -
Flags: review?(jschoenick)
Assignee | ||
Comment 79•11 years ago
|
||
Maybe I should move the plugin hiding policy from code to a pref. I will add a (semicolon-delimited?) pref of plugin name substrings so people can easily test hiding or unhiding different plugins. We could ship different plugin hiding lists on different platforms or only add "Second Test Plug-in" when running mochitests. Something like:
pref("plugins.visible", "Flash;Java;QuickTime Plug-in;Shockwave;Second Test Plug-in");
Comment 80•11 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #79)
> pref("plugins.visible", "Flash;Java;QuickTime Plug-in;Shockwave;Second Test
> Plug-in");
I would suggest naming it something that better indicates what it's for, though. Something like "plugins.scriptdetectable" or just "plugins.enumeratable" would be better. (just not "visible", because that implies we're talking about whether or not you can see something on screen)
Updated•11 years ago
|
Attachment #825121 -
Flags: review?(jschoenick) → review+
Updated•11 years ago
|
Attachment #825122 -
Flags: review?(jschoenick) → review+
Comment 81•11 years ago
|
||
It might make more sense to whitelist mime types than plugin names, and make visible plugins that use that mime type.
As far as what shouldn't be hidden -- any website serious about fingerprinting will respond to a lack of enumeration by just including a list of the most common mime types (several extensive lists can be found easily by googling), which also links back to the providing plugin via mimetype.enabledPlugin. The protection this offers, then, is primarily for preventing the discovery of uncommon plugins - so I would be fairly aggressive about whitelisting, and ensure release management/QA/SUMO is aware of this so they can be on the lookout for issues.
Comment 82•11 years ago
|
||
(In reply to John Schoenick [:johns] from comment #81)
> As far as what shouldn't be hidden -- any website serious about
> fingerprinting will respond to a lack of enumeration by just including a
> list of the most common mime types (several extensive lists can be found
> easily by googling)
This could theoretically be mitigated somewhat by setting some arbitrary maximum amount of times the plugins object could be queried for non-whitelisted plugins/types per page.
Comment 83•11 years ago
|
||
(In reply to Dave Garrett from comment #82)
> This could theoretically be mitigated somewhat by setting some arbitrary
> maximum amount of times the plugins object could be queried for
> non-whitelisted plugins/types per page.
Web pages can just reload themselves (perhaps with some parameters) to continue query.
Comment 84•11 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #83)
> (In reply to Dave Garrett from comment #82)
> > This could theoretically be mitigated somewhat by setting some arbitrary
> > maximum amount of times the plugins object could be queried for
> > non-whitelisted plugins/types per page.
>
> Web pages can just reload themselves (perhaps with some parameters) to
> continue query.
That would require quite a bit more effort and be a bit more noticeable than just running a script in the background. If we were really worried about that the limit could be per domain within some small window of time. (i.e. just detect and block flooding plugin support queries) The goal is just to knock out the straightforward methods to get around this bug and enumerate plugins manually.
Assignee | ||
Comment 85•11 years ago
|
||
John: in comment 81, you suggested whitelisting MIME type instead of plugin names.
Assignee | ||
Comment 86•11 years ago
|
||
... Can you please explain why you think MIME types are preferable? I believe the plugins that will need to be unhidden are because most problematic content is searching through navigator.plugins[] for a given plugin name (e.g. "Shockwave Flash") and doesn't care about "application/x-shockwave-flash". So I think it makes sense to whitelist plugin name (by substrings to deal with plugin names that include changing version numbers, e.g. "QuickTime Plugin 7.7.3").
Flags: needinfo?(jschoenick)
Comment 87•11 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #86)
> ... Can you please explain why you think MIME types are preferable? I
> believe the plugins that will need to be unhidden are because most
> problematic content is searching through navigator.plugins[] for a given
> plugin name (e.g. "Shockwave Flash") and doesn't care about
> "application/x-shockwave-flash". So I think it makes sense to whitelist
> plugin name (by substrings to deal with plugin names that include changing
> version numbers, e.g. "QuickTime Plugin 7.7.3").
Sorry, I meant, we should be whitelisting *plugins* by what mimetypes they *claim*. That way any plugin claiming application/x-shockwave-flash would get whitelisted - this would prevent plugins renaming themselves causing bustage (that would need a hotfix to solve).
Another bonus is making it less tempting for plugin authors to try to work other plugin names into their own as substrings to get whitelisted -- claiming another plugin's exact MIME type is a lot more intrusive than putting "(like QuickTime Plugin)" in your name.
Flags: needinfo?(jschoenick)
Comment 88•11 years ago
|
||
Maybe the most important situation where the difference matters are alternative plugin implementations. For example, on Linux, you have an mplayer plugin that can play the same mimetypes as Quicktime, and I think it's even installed by default on many distros.
So, I agree whitelisting plugin names based on the mimetypes they implement is a better approach. (That said, I wouldn't block commit of this important bug based on such points.)
Assignee | ||
Comment 89•11 years ago
|
||
BenB: Does http://trailers.apple.com/ work correctly with the Linux mplayer plugin?
mplayer could support the same MIME types as QuickTime, which would work if content queries navigator.mimeTypes[]. But advertising "mplayer Plug-in" in navigator.plugins[] enumeration won't help of content is searching for "QuickTime Plug-in" string matches.
Comment 90•11 years ago
|
||
Will Shumway return the exact same string as genuine Flash Player when bug 867626 is fixed?
Assignee | ||
Comment 91•11 years ago
|
||
I see how whitelisting plugins by their MIME types is more robust than by their names. However, plugin names are the whole reason plugins need to be unhidden to support content that enumerates navigator.plugins[] for a name. Whitelisting MIME types would unnecessarily unhide more plugins than necessary.
Consider the http://trailers.apple.com/ example. Here is Apple's QuickTime plugin detection script:
http://trailers.apple.com/trailers/global/v3.1/scripts/lib/ac_media.js
My QuickTime plugin's name is "QuickTime Plug-in 7.7.3". Apple's script enumerates navigator.plugins[] for a plugin matching `plugin.name.indexOf("QuickTime") > -1`.
If I whitelist MIME type "application/quicktime" to unhide the QuickTime plugin, then alternative plugins like "VLC Multimedia Plug-in" will also be unhidden. Unfortunately, Apple's script won't recognize the VLC plugin name without a "QuickTime" substring, so unhiding this uncommon plugin has a small downside and no upside for VLC plugin users.
The problem of plugins including other plugins' names in their own names (e.g. "My Super Plug-in (like QuickTime Plugin)") could be reduced by using a whitelist of name prefixes instead of substring matches. For example, whitelist pref "Shockwave" would match "Shockwave Flash" and "Shockwave for Director", but not "My Super Plug-in (like Shockwave Flash)".
Assignee | ||
Comment 92•11 years ago
|
||
Landed patches 2, 3, and 4:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04811425fc1d
https://hg.mozilla.org/integration/mozilla-inbound/rev/46be10c9cb30
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bc5f1f28031
Keywords: feature
Comment 93•11 years ago
|
||
re comment 89: I'm not using the mplayer plugin myself.
Comment 91 is convincing (thank you) and I retract my comment 88.
Comment 94•11 years ago
|
||
I've created tech-evan bug 934107 to get rid of needless enumeration on high-profile sites like you mentioned.
String name change happened for Java: bug 914690 comment 17.
Comment 95•11 years ago
|
||
Assignee | ||
Comment 96•11 years ago
|
||
Part 5: Add mEnsuredPlugins flag for case when no plugins are installed.
nsPluginArray::EnsurePlugins() assumes mPlugins.IsEmpty() means that mPlugins has not be lazily initialized yet. But if a user has no plugins installed, then mPlugins.IsEmpty() will always be true and EnsurePlugins() will unnecessarily attempt to reinitialize mPlugins every time.
I did not add similar MIME type checks for nsMimeTypeArray because I assume mMimeTypes is unlikely to be empty because it can contain native PreferredApplicationHandlers.
I did not add similar MIME type checks for nsPluginElement because I assume that an installed plugin will handle at least one MIME type.
Attachment #825124 -
Attachment is obsolete: true
Attachment #829927 -
Flags: review?(jschoenick)
Assignee | ||
Comment 97•11 years ago
|
||
Comment on attachment 829927 [details] [diff] [review]
part-5-add-mEnsuredPlugins-flag.patch
Obsoleting incomplete patch.
Attachment #829927 -
Attachment is obsolete: true
Attachment #829927 -
Flags: review?(jschoenick)
Assignee | ||
Comment 98•11 years ago
|
||
Part 5: Fix test_refresh_navigator_plugins to not recycle variables.
The test_refresh_navigator_plugins.html test's testPart3() was inadvertently using the `navTestPlugin` variable from the outer scope without fetching the current plugin status. I changed the test to use separate `navTestPlugin` variable names to avoid shadowing.
This test fix is independent of my plugin hiding patch, but a bug in my patch revealed the bug in this test. <:)
Attachment #829988 -
Flags: review?(jschoenick)
Assignee | ||
Comment 99•11 years ago
|
||
Part 6: Hide most plugins from navigator.plugins and navigator.mimeTypes enumeration.
This patch moves the policy of which plugins to unhide from C++ to a "plugins.enumerableNames" pref. This pref is a comma-delimited list of *prefixes* to match plugin names that should unhidden. I don't really like the pref name "enumerableNames", so I'm open to suggestions for better names. Perhaps the name should mention that the strings are matching plugin name prefixes, not exact names?
The current plugin name prefixes that are unhidden are:
* "Shockwave", which will match both Adobe Flash Player ("Shockwave Flash") and Adobe Shockwave Player ("Shockwave for Director").
* "QuickTime Plug-in", which will match many QuickTime versions such as "QuickTime Plug-in 7.7" and "QuickTime Plug-in 7.7.4".
* "Java", which should match a variety of Java plugin names, such as:
* "Java Applet Plug-in"
* "Java Deployment Toolkit 7.0.250.16"
* "Java Plug-in 1.7.0_25"
* "Java(TM) Platform SE 7"
* "Java(TM) Platform SE 7 U6"
* "Java(TM) Plug-in 10.45.2"
For detailed lists of plugin names, see https://wiki.mozilla.org/QA/Plugins/About:Plugins and https://plugins.mozilla.org/en-us
Limitations: the "plugins.enumerableNames" pref is lazily initialized the first time a page accesses navigator.plugins (i.e. the nsPluginArray::EnsurePlugins() function). If the pref changes, navigator.plugins for a given page will NOT reflect changes in plugin visibility unless the page is reloaded.
Attachment #820132 -
Attachment is obsolete: true
Attachment #820133 -
Attachment is obsolete: true
Attachment #829992 -
Flags: review?(jschoenick)
Assignee | ||
Comment 100•11 years ago
|
||
Comment 101•11 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #99)
> * "QuickTime Plug-in", which will match many QuickTime versions such as
> "QuickTime Plug-in 7.7" and "QuickTime Plug-in 7.7.4".
Just curious, but is there a reason to single out QuickTime and match "QuickTime Plug-in" rather than just "QuickTime"? I'm not suggesting changing it or not; just wondering if there was a specific case that needs to be worried about here or if you're just using the largest prefix you know you need.
> I don't really like
> the pref name "enumerableNames", so I'm open to suggestions for better
> names. Perhaps the name should mention that the strings are matching plugin
> name prefixes, not exact names?
The best I can come up with is "plugins.enumerable_short_names" or "plugins.enumerable_truncated_names" if you'd like to get nice and wordy. (easier to read with underscores instead of camel-case, similar to most of the other "plugins.*", now that it's three words)
Assignee | ||
Comment 102•11 years ago
|
||
(In reply to Dave Garrett from comment #101)
> Just curious, but is there a reason to single out QuickTime and match
> "QuickTime Plug-in" rather than just "QuickTime"? I'm not suggesting
> changing it or not; just wondering if there was a specific case that needs
> to be worried about here or if you're just using the largest prefix you know
> you need.
"QuickTime Plug-in", "Java", and "Shockwave" were the longest prefixes that would match the plugins I wanted. (See some example names in comment 99.) I don't know if there is a QuickTime plugin variant that has a name that would not match like "QuickTime $WHATEVER Plug-in".
Updated•11 years ago
|
Attachment #829988 -
Flags: review?(jschoenick) → review+
Comment 103•11 years ago
|
||
Comment on attachment 829992 [details] [diff] [review]
part-6-v3-hide-plugins-pref.patch
Review of attachment 829992 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good to me with the nits below -- using plugin name prefixes is probably the sanest way to do this. This should probably get sr? from benjamin to land pref'd on, however.
::: dom/base/nsPluginArray.cpp
@@ +274,5 @@
> + return str.Compare(prefix.BeginReading(), false, prefix.Length()) == 0;
> +}
> +
> +bool
> +nsPluginArray::IsSpecialPluginType(const nsPluginTag* pluginTag)
This is doing something different from nsPluginHost::IsSpecialPluginType now, which takes a mime type and identifies stuff like flash/java that have scattered special handling -- IsPluginHidable probably made more sense.
@@ +282,5 @@
> + const uint32_t length = mEnumerablePluginNames.Length();
> + for (uint32_t i = 0; i < length; i++) {
> + const nsCString& name = mEnumerablePluginNames[i];
> + if (HasStringPrefix(pluginName, name)) {
> + return true; // hide plugin!
This should be don't hide plugin -- this function is actually returning IsPluginEnumerable
@@ +298,5 @@
> return;
> }
>
> + const nsAdoptingCString& enumerableNames =
> + Preferences::GetCString("plugins.enumerableNames");
We should have a pref to revert to having all plugins enumerable, either through a plugin.restrictEnumeration pref or a special value here (e.g. *)
@@ +305,5 @@
> + nsCCharSeparatedTokenizer tokens(enumerableNames, ',');
> + while (tokens.hasMoreTokens()) {
> + const nsCSubstring& token = tokens.nextToken();
> + if (!token.IsEmpty()) {
> + mEnumerablePluginNames.AppendElement(token);
mEnumerablePluginNames is only used for the scope of this function, does it need to be a class member?
@@ +311,5 @@
> + }
> + }
> +
> + mPlugins.Clear();
> + mHiddenPlugins.Clear();
If either of these isn't empty we would've taken an early return already
Attachment #829992 -
Flags: review?(jschoenick) → review+
Assignee | ||
Comment 104•11 years ago
|
||
Part 5: Fix test_refresh_navigator_plugins to not recycle variables. r=johns
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ecd8d06586b
Assignee | ||
Comment 105•11 years ago
|
||
patch 6 v4
As per review feedback in comment 101 and comment 103:
* Renamed "plugin.enumerableNames" pref to "plugins.enumerable_names"
* Added special pref value '*' to disable all plugin hiding
* Renamed IsSpecialPluginType() to IsPluginHidable()
* Fixed comments on IsPluginHidable()'s return values
* Moved member variable mEnumerablePluginNames to a local variable
* Removed redundant nsTArray Clear() calls
The default pref hides all plugins except Java, QuickTime, Flash, and Shockwave. Please note that, because Mozilla's Plugin Check website won't see hidden plugins when it enumerates navigator.plugins[], the website will no longer check any plugins that are not Java, QuickTime, Flash, or Shockwave!
For example, Plugin Check will will no longer check the following plugins on my Mac (though these plugins may have their own auto-updaters):
* Silverlight
* Unity Player
* Google Talk Plugin
* Google Talk Plugin Video Accelerator
https://www.mozilla.org/en-US/plugincheck/
Attachment #831356 -
Flags: superreview?(benjamin)
Comment 106•11 years ago
|
||
Why don't you rewrite plugincheck so that it works without enumeration?
Comment 107•11 years ago
|
||
I filed bug 938885 to fix the plugin check. If somebody could please assign this to the right person? Or any volunteers here?
Comment 108•11 years ago
|
||
Flags: in-testsuite+
Comment 109•11 years ago
|
||
Comment on attachment 831356 [details] [diff] [review]
part-6-v4-hide-plugins-pref.patch
We're going to have to watch this pretty closely for regressions. Please set addon-compat and make sure QA knows about this.
Attachment #831356 -
Flags: superreview?(benjamin) → superreview+
Assignee | ||
Comment 110•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae4cfe0a9bdb
https://hg.mozilla.org/integration/mozilla-inbound/rev/49cab4f07faf
Keywords: addon-compat
Whiteboard: [leave open]
Comment 111•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ae4cfe0a9bdb
https://hg.mozilla.org/mozilla-central/rev/49cab4f07faf
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•11 years ago
|
Whiteboard: Websites tech advo bug 934107
Updated•11 years ago
|
Comment 112•11 years ago
|
||
https://developer.mozilla.org/en-US/docs/Web/API/NavigatorPlugins.plugins
https://developer.mozilla.org/en-US/Firefox/Releases/28/Site_Compatibility
Assignee | ||
Comment 113•11 years ago
|
||
bsmedberg: Do you think we should hold the changes to navigator.plugins enumeration in the Nightly and Aurora channels for a release cycle or two? Aurora users have been doing a good job of reporting site compatibility bugs, but the incoming report rate is slow (and, AFAIK, the sites' fix rate is zero). We might want extra test time before releasing this change to Beta.
Flags: needinfo?(benjamin)
Comment 114•11 years ago
|
||
Aurora doesn't have many non-English users and those are skewed highly towards European languages. So I expect that the data we want to collect would have to be from beta users in order to figure out whether we can ship this at all, and what sites we'd need to evangelize or add to the whitelist.
So I'd say we should let this ride to beta and plan to turn it off in a later beta. But that's really a call for release-drivers.
Flags: needinfo?(benjamin) → needinfo?(release-mgmt)
Comment 115•11 years ago
|
||
I agree with bsmedberg here, we should get the additional feedback on beta and then disable before ship so as to get an extra cycle on beta until we're sure this is ready to go.
Flags: needinfo?(release-mgmt)
Comment 116•11 years ago
|
||
Can you file a disabling bug and set tracking 28? on it so that we can ensure we do turn this off before shipping 28.0 to general audience?
Flags: needinfo?(cpeterson)
Updated•11 years ago
|
Target Milestone: mozilla28 → mozilla29
Comment 118•11 years ago
|
||
The target milestone here should probably stay as mozilla28 until it's actually backed out or auto-disabled for release builds so we don't accidentally think this has already happened.
Updated•11 years ago
|
Target Milestone: mozilla29 → mozilla28
Updated•11 years ago
|
Comment 119•11 years ago
|
||
As per Bug 952602, this change has been disabled for Firefox 29.
relnote-firefox: 29?
I'll update https://developer.mozilla.org/en-US/Firefox/Releases/28/Site_Compatibility
Comment 120•11 years ago
|
||
I meant: this change has been disabled for Firefox *28*
Updated•11 years ago
|
Flags: needinfo?(lsblakk)
Comment 121•11 years ago
|
||
I have moved the site compatibility notice to
https://developer.mozilla.org/en-US/Firefox/Releases/29/Site_Compatibility
Comment 122•11 years ago
|
||
Is this disabled in a way that it can be re-enabled for testing? Or is it in fact removed from the build?
Tried on 29b2 and it seems to still list all plugins when testing on https://panopticlick.eff.org
Assignee | ||
Comment 123•11 years ago
|
||
(In reply to Ralf Vitasek from comment #122)
> Is this disabled in a way that it can be re-enabled for testing? Or is it in
> fact removed from the build?
> Tried on 29b2 and it seems to still list all plugins when testing on
> https://panopticlick.eff.org
Ralf, which plugins do you have installed? The following popular plugins are whitelisted so they are still exposed in navigator.plugins[] to avoid breaking too many websites.
* Java
* Nexus Personal BankID (Swedish banking crypto plugin)
* QuickTime
* Adobe Flash Player
* Adobe Shockwave Player (Director)
You can see the plugin name keywords in the about:config pref "plugins.enumerable_names": Java,Nexus Personal,QuickTime,Shockwave
Comment 124•11 years ago
|
||
Updating flags as per Bug 952602.
status-firefox30:
--- → disabled
status-firefox31:
--- → disabled
Comment 125•11 years ago
|
||
Hum... Even after the attribute reset, plugins.enumerable_names is '*' ? (also came here after getting a very long list at EFF's Panopticlick)
Comment 126•11 years ago
|
||
I just checked and i noticed that plugins.enumerable_names was changed to '*' from my manual setting to an empty string. After clearing the value, Panopticlick again show 'undefined' in the plugin list as i was aiming for.
I hope it's just the beta channel that silently changes user modified config settings.
Comment 127•11 years ago
|
||
I'm on Firefox 29 final. plugins.enumerable_names is '*' by default.
Assignee | ||
Comment 128•11 years ago
|
||
pedrogfrancisco: Sorry your modified setting was reset. Did you switch between Firefox Beta and Release channels using the same profile?
The default value for plugins.enumerable_names in the Release channel (Firefox 29) is still '*' because we're still tracking down websites that would be broken by this plugin change. If you find any, please let us know! <:)
Comment 129•11 years ago
|
||
I did not switch at all. And just by what i can see it looks like the beta-channel was removed and the updater put me on stable without asking.
Last update in history shows as 29.0 RC1 from April 23rd.
Comment 130•11 years ago
|
||
(In reply to Ralf Vitasek from comment #129)
> I did not switch at all. And just by what i can see it looks like the
> beta-channel was removed and the updater put me on stable without asking.
>
> Last update in history shows as 29.0 RC1 from April 23rd.
You are still on beta channel (can check in about:config app.update.channel) -- the beta channel gets an update to the RC, then the next update will be beta1 from the following version which in this case will be this coming Thursday when we release Firefox 30 beta 1
Comment 131•11 years ago
|
||
Ah! Sorry, I thought the value was supposed to be '' by default.
So everything is as it was supposed to be, I had '*' prior to manually changing to ''.
I just commented it here because I thought it was a mistake. Everything is fine then.
I'll report any broken site, if any, since I'm testing '' now. Thanks!
Comment 132•11 years ago
|
||
Bug 952602 says status-firefox31:fixed so updating the flag.
The site compat info is back:
https://developer.mozilla.org/en-US/Firefox/Releases/31/Site_Compatibility#Plugins
Comment 133•11 years ago
|
||
Nope, status-firefox31:fixed in Bug 952602 means "Bug 757726 is disabled in Firefox 31, but not yet verified by QA". Removed the site compat info again...
Assignee | ||
Comment 135•11 years ago
|
||
Reenable navigator.plugin enumeration because, for now, the fallout from broken plugins outweighs the theoretical fingerprinting benefit. I would like to uplift this patch to Aurora 31 and Beta 30, too.
Attachment #8421211 -
Flags: review?(benjamin)
Updated•11 years ago
|
Attachment #8421211 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 136•11 years ago
|
||
Assignee | ||
Comment 137•11 years ago
|
||
Comment on attachment 8421211 [details] [diff] [review]
part-7-reenable-plugin-enumeration.patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug restricted navigator.plugins enumeration to reduce "browser fingerprinting" in Nightly 28. This change rode the trains to Beta 28, but was #ifdef EARLY_BETA_OR_EARLIER so the feature was never released to Firefox Release channel users.
User impact if declined: Some plugin websites will remain broken in the Nightly, Aurora, and Beta channels, but the Release channel is unaffected (because this plugin enumeration feature was never released).
Testing completed (on m-c, etc.):
Risk to taking this patch (and alternatives if risky): Low risk. This is a pref patch that just reenables navigator.plugin enumeration because, for now, the fallout from broken plugins outweighs the theoretical fingerprinting benefit.
String or IDL/UUID changes made by this patch: None.
Attachment #8421211 -
Attachment description: 757726_part-7-reenable-plugin-enumeration.patch → part-7-reenable-plugin-enumeration.patch
Attachment #8421211 -
Flags: approval-mozilla-beta?
Attachment #8421211 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #8421211 -
Flags: approval-mozilla-beta?
Attachment #8421211 -
Flags: approval-mozilla-beta+
Attachment #8421211 -
Flags: approval-mozilla-aurora?
Attachment #8421211 -
Flags: approval-mozilla-aurora+
Comment 138•11 years ago
|
||
Comment 139•11 years ago
|
||
Fixing the status flags so it actually shows up when looking for things to uplift...
Target Milestone: mozilla28 → mozilla32
Comment 140•11 years ago
|
||
Comment 141•11 years ago
|
||
Comment 142•11 years ago
|
||
Why is this bug marked FIXED while disabled everywhere?
Status: RESOLVED → REOPENED
status-firefox30:
fixed → ---
status-firefox31:
fixed → ---
status-firefox32:
fixed → ---
relnote-firefox:
29+ → ---
Resolution: FIXED → ---
Comment 143•11 years ago
|
||
Yes, I think the commits in comment 138 and 140 were in error. They wrongly took the patch from comment 137.
espressive said in bug 938885 comment 52 that we can turn *off* enumeration, and these patches turned *on* enumeration.
Comment 144•11 years ago
|
||
> the fallout from broken plugins outweighs the theoretical fingerprinting benefit
Can we have actual data on that, something that can be worked on and fixed (via tech evang or on our side)?
Comment 145•11 years ago
|
||
For future reference, a separate bug for "back out feature X" is a good idea when the backout requires uplifting. The backout bug gets marked "fixed" on the relevant brances, and the backed-out bug gets marked "disabled".
Assignee | ||
Comment 146•11 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #144)
> > the fallout from broken plugins outweighs the theoretical fingerprinting benefit
>
> Can we have actual data on that, something that can be worked on and fixed
> (via tech evang or on our side)?
I submitted the patch in comment 137 to reenable enumeration after a discussion with bsmedberg. I've been intermittently working on this feature since 2012, so my suggestion to disable this feature is not an easy one. <:\
The patch to disable enumeration (with a compatibility whitelist) landed in Nightly 28 six months ago and has been live in the Beta channel since February 2014. In that time, we've had only nine broken websites reported (covered by six plugins) in tech evangelism bug 934107. Of those nine broken websites reported, only two have responded to our contacts and fixed their plugin detection (Battlelog bug 944805 and ACE Stream bug 950445).
Nine broken websites is a surprisingly low number to me. Given that 36 plugin authors have submitted requests to be added to Firefox's "Plugin Click-To-Activate Whitelist" [1], I strongly suspect (but have no data) that we have been breaking plenty of other plugin websites. I personally emailed each of the 36 plugin authors that submitted a CTP whitelist request to alert them to the plugin enumeration changes. Most responded, but
My biggest concern is that broken plugin websites will send Firefox users to Chrome, where the plugins work as expected. We could whitelist all the plugins that Chrome supports [2], so Firefox does not support fewer plugin websites than Chrome, but Firefox users will still be upset by the loss of behavior. Additionally, we could whitelist all the plugins that are on Firefox's new CTP whitelist (because those plugin authors cared enough to submit a whitelist request).
[1] https://bugzilla.mozilla.org/buglist.cgi?product=Firefox&component=Plugin Click-To-Activate Whitelist
[2] https://support.google.com/chrome/answer/142064
Comment 147•11 years ago
|
||
cpeterson, thanks for the reply, but I still don't understand: What broke where? Do we have any data or not? We know is that 9 sites were reported broken. Do we know any more? If not, I see no reason to change plans now.
Assignee | ||
Comment 148•11 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #147)
> cpeterson, thanks for the reply, but I still don't understand: What broke
> where? Do we have any data or not? We know is that 9 sites were reported
> broken. Do we know any more? If not, I see no reason to change plans now.
I don't have any more data about broken sites, just concerns that we would frustrate Firefox users. I really would like to ship this feature. I'm not sure what we could measure to find websites with broken plugin detection, but if you have ideas, we can add Firefox Telemetry measurements to http://telemetry.mozilla.org . Perhaps we could collect telemetry on which plugins users have installed and what a website enumerates navigator.plugins but then doesn't instantiate a plugin within 1 minute.
I think user pain could be reduced by my suggestion in comment 146 to whitelist plugins supported by Chrome and Firefox's CTP whitelist. If we disabled plugin enumeration in the same Firefox release as the CTP whitelist, then users and web developers would experience the two plugin changes as "one big change" instead of repeated plugin breaking in multiple Firefox releases.
Comment 149•11 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #146)
> We could whitelist all the
> plugins that Chrome supports [2], so Firefox does not support fewer plugin
> websites than Chrome, but Firefox users will still be upset by the loss of
> behavior.
> [2] https://support.google.com/chrome/answer/142064
I'm not sure you're reading that page right: It has a list of popular plugins that are supported, but chrome does not, AFAIK, whitelist NPAPI plugins that it will load. Visiting about:plugins in chrome on linux shows the same list of plugins I get in Fx, including ones not mentioned there.
Comment 150•10 years ago
|
||
Removing relnote keyword. We are trying to get ride of this keyword.
Please use relnote-firefox once it is ready.
Keywords: relnote
Updated•10 years ago
|
Keywords: dev-doc-complete → dev-doc-needed
Comment 151•10 years ago
|
||
Stopping enumeration is a fair temporary workaround.
If plugins can be installed without exposing them to javascript or through http headers it solves this problem.
Object.defineProperty(navigator, "plugins", {
value: [
/* non-enumerable whitelist of plugins whose names are not detailed/fingerprintable */
]});
Comment hidden (obsolete) |
Updated•9 years ago
|
Whiteboard: Websites tech advo bug 934107 → [fingerprinting] Websites tech advo bug 934107
Comment 153•9 years ago
|
||
We will no longer do this before dropping the plug-in support entirely. See bug 1169945.
Honestly, I'm dubious about adding docs blindly for all dev-doc-needed bugs.
Comment 154•9 years ago
|
||
I'm not blindly adding docs for all dev-doc-needed bugs. If you don't do this (for now), you guys should update the bug and leave a comment in the Whiteboard field.
Assignee | ||
Comment 155•9 years ago
|
||
emk and kohei, thanks for the heads up. I removed the dev-doc-needed keyword and resolved this bug WONTFIX (as per my explanation in comment 146).
Status: REOPENED → RESOLVED
Closed: 11 years ago → 9 years ago
Keywords: dev-doc-needed
Resolution: --- → WONTFIX
Comment 156•9 years ago
|
||
Removed the site compat doc.
Comment 157•8 years ago
|
||
This bug has been eventually fixed with Firefox 51 in Bug 1270364 and Bug 1270366. Am I correct?
Firefox 50:
>> 'Shockwave Flash' in navigator.plugins
> true
>> navigator.plugins.length
> 1
>> for (let i in navigator.plugins) console.log(i)
> 0
> Shockwave Flash
> item
> namedItem
> refresh
> length
Firefox 51:
>> 'Shockwave Flash' in navigator.plugins
> true
>> navigator.plugins.length
> 1
>> for (let i in navigator.plugins) console.log(i)
> 0
> item
> namedItem
> refresh
> length
Flags: needinfo?(bzbarsky)
Comment 158•8 years ago
|
||
Hmm. Bug 1270366 would in fact have had that effect. If that's not web-compatible (per comment 146) then we should back it out and file a spec issue. Chris, should I do that?
But note that for privacy purposes bug 1270366 does absolutely nothing. You can do Object.getOwnPropertyNames(navigator.plugins) to get them all, including the non-enumerable ones.
Flags: needinfo?(bzbarsky) → needinfo?(cpeterson)
Assignee | ||
Comment 159•8 years ago
|
||
@ Kohei: Great catch!
@ Boris: Ironically, bug 1270366 broke Mozilla's own Plugin Check site, but we can fix that. It's not the typical use case for plugin enumeration.
https://www.mozilla.org/plugincheck/
Unminified sources:
https://github.com/mozilla/bedrock/blob/master/media/js/plugincheck/lib/plugincheck.js
https://github.com/mozilla/bedrock/blob/master/media/js/plugincheck/lib/utils.js#L46
https://github.com/mozilla/bedrock/blob/master/media/js/plugincheck/lib/version-compare.js#L33
I retested the other broken sites reported (bug 934107) against my original patch for this bug. They would not be broken by your patch because navigator.plugins.length > 0, which was not true for my original patch to prevent all enumeration.
Flags: needinfo?(cpeterson)
Comment 160•8 years ago
|
||
Should we probably postpone the fix of Bug 1270364 and Bug 1270366 until Firefox 52, excluding ESR? The next Nightly will drop the NPAPI support other than Flash anyway, so that the compatibility impact should be minimum.
Assignee | ||
Comment 161•8 years ago
|
||
(In reply to Kohei Yoshino [:kohei] from comment #160)
> Should we probably postpone the fix of Bug 1270364 and Bug 1270366 until
> Firefox 52, excluding ESR? The next Nightly will drop the NPAPI support
> other than Flash anyway, so that the compatibility impact should be minimum.
Boris: I don't think we should back out bug 1270364 and bug 1270366, but I think we should defer them to Firefox 52 (excluding ESR 52) or Firefox 53.
Kohei: That's a very good idea. We've warned web developers to anticipate plugin problems in Firefox 52 (excluding ESR 52), so batching up multiple plugin problems into 52 is a good idea.
Flags: needinfo?(bzbarsky)
Comment 162•8 years ago
|
||
> I don't think we should back out bug 1270364 and bug 1270366, but I think we should defer them to Firefox 52
That means exactly back them out (and maybe reland them later).
I expect it's not a problem in practice because these properties are not enumerable in other browsers anyway, but happy to play it safe; those fixes are not urgent.
Flags: needinfo?(bzbarsky)
Comment 163•8 years ago
|
||
Oh, is there some bug I should mark those dependent on so I know when I can reland them?
Flags: needinfo?(cpeterson)
Updated•8 years ago
|
Comment 164•8 years ago
|
||
I don't see what this bug has to do with bug 1270366 and bug 1270364. Again, Object.getOwnPropertyNames would give you the property names even after those bugs are fixed.
Assignee | ||
Comment 165•8 years ago
|
||
(In reply to Boris Zbarsky [:bz] (busy, pun intended) from comment #162)
> > I don't think we should back out bug 1270364 and bug 1270366, but I think we should defer them to Firefox 52
>
> That means exactly back them out (and maybe reland them later).
Sorry, yes. I just meant we don't need to back them our permanently.
(In reply to Boris Zbarsky [:bz] (busy, pun intended) from comment #163)
> Oh, is there some bug I should mark those dependent on so I know when I can
> reland them?
Yes. I'll make those bugs depend on bug 1269807 (npapi-eol: Remove support for all NPAPI plugins except Flash) landing instead of this WONTFIX bug.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
Flags: needinfo?(cpeterson)
Resolution: --- → WONTFIX
Comment 166•8 years ago
|
||
Is there a TL:DR summary somewhere? Does "WONTFIX" mean that we will still see plugins listed via https://panopticlick.eff.org/ and that Firefox users will still be easily tracked via list of installed plugins?
Comment 167•8 years ago
|
||
(In reply to Valent from comment #166)
> Is there a TL:DR summary somewhere? Does "WONTFIX" mean that we will still
> see plugins listed via https://panopticlick.eff.org/ and that Firefox users
> will still be easily tracked via list of installed plugins?
This bug is dropped in favor of bug 1269807, where support for NPAPI plugins other than Flash is being dropped entirely. This will result in a navigator.plugins for all Firefox users containing exactly one or zero items; enumeration becomes irrelevant. This bug could theoretically be closed as WORKSFORME after that bug lands, but the distinction isn't particularly important.
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•