Closed Bug 757726 Opened 12 years ago Closed 8 years ago

disallow enumeration of navigator.plugins

Categories

(Core Graveyard :: Plug-ins, defect, P2)

defect

Tracking

(firefox28 disabled, firefox29 disabled, firefox30 disabled, firefox31 disabled)

RESOLVED WONTFIX
Tracking Status
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+
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.
OS: Linux → All
Hardware: x86_64 → All
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.
Seems reasonable to limit the ability to enumerate navigator.plugins. Confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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/
(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
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.
(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.
Attached patch WIP-prevent-plugin-enumeration.patch (obsolete) — — Splinter Review
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)
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
(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.
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.
> 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.
(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?
> 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".
(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?".
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.
(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?
(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.
No longer depends on: 793978
> 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 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+
Attached patch part-2-fix-about-plugins.patch (obsolete) — — Splinter Review
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)
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.
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 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+
Blocks: 566423
No longer blocks: 566423
Attached patch part-1-about-plugins-v2.patch — — Splinter Review
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)
Attached patch part-2-prevent-plugin-enumeration-v2.patch (obsolete) — — Splinter Review
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 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.
> * Replace document.write() with document.createElement() Your new patch resolves comment 23. Thank you!
Attached patch part-2-prevent-plugin-enumeration-v3.patch (obsolete) — — Splinter Review
(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)
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?
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
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...)
(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/
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.
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.
(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 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+
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]
(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.
Depends on: 831188
Depends on: 835969
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.
(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.
(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.
Why relnote-, given the potential to break existing websites?
Flags: needinfo?(bbajaj)
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.
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?
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.
(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
(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
Flags: needinfo?(bbajaj)
(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 :)
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.
Depends on: 876988
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?
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).
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
I have submitted a chromium issue detailing this privacy problem: https://code.google.com/p/chromium/issues/detail?id=271772
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)
Attachment #820124 - Attachment description: 757726-part-2-fix-Preferences-Applications.patch → part-2-fix-Preferences-Applications.patch
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)
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)
Attached patch part-5-hide-plugins.patch (obsolete) — — Splinter Review
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)
Attached patch part-6-unhide-PluginCheck-plugins.patch (obsolete) — — Splinter Review
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)
Attached patch part-7-fix-Second_Test_Plug-in-tests.patch (obsolete) — — Splinter Review
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)
> * 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 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 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 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)
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.
> * 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.
(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/
See Also: → jsplugins-base
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. <:)
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). <:)
Attachment #820124 - Flags: review?(benjamin) → review+
Depends on: 933124
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)
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)
Attachment #820127 - Attachment is obsolete: true
Attached patch part-5-v2-hide-plugins.patch (obsolete) — — Splinter Review
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)
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.
Attachment #825124 - Flags: review?(jschoenick)
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");
(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)
Attachment #825121 - Flags: review?(jschoenick) → review+
Attachment #825122 - Flags: review?(jschoenick) → review+
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.
(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.
(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.
(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.
John: in comment 81, you suggested whitelisting MIME type instead of plugin names.
... 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)
(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)
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.)
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.
Will Shumway return the exact same string as genuine Flash Player when bug 867626 is fixed?
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)".
re comment 89: I'm not using the mplayer plugin myself. Comment 91 is convincing (thank you) and I retract my comment 88.
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.
Attached patch part-5-add-mEnsuredPlugins-flag.patch (obsolete) — — Splinter Review
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)
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)
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)
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)
(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)
(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".
Attachment #829988 - Flags: review?(jschoenick) → review+
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+
Part 5: Fix test_refresh_navigator_plugins to not recycle variables. r=johns https://hg.mozilla.org/integration/mozilla-inbound/rev/4ecd8d06586b
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)
Why don't you rewrite plugincheck so that it works without enumeration?
I filed bug 938885 to fix the plugin check. If somebody could please assign this to the right person? Or any volunteers here?
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+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 942587
Whiteboard: Websites tech advo bug 934107
See Also: → 934107
Depends on: 944957
Depends on: 944805
Depends on: 950445
No longer depends on: 950445
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)
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)
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)
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)
Depends on: 952602
I filed disabling bug 952602.
Flags: needinfo?(cpeterson)
Target Milestone: mozilla28 → mozilla29
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.
Target Milestone: mozilla29 → mozilla28
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
Flags: needinfo?(lsblakk)
I meant: this change has been disabled for Firefox *28*
Flags: needinfo?(lsblakk)
Depends on: 972052
No longer depends on: 972052
Depends on: 985968
No longer depends on: 944805
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
(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
Hum... Even after the attribute reset, plugins.enumerable_names is '*' ? (also came here after getting a very long list at EFF's Panopticlick)
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.
I'm on Firefox 29 final. plugins.enumerable_names is '*' by default.
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! <:)
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.
(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
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!
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
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...
So the current flag should be...
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)
Attachment #8421211 - Flags: review?(benjamin) → review+
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?
Attachment #8421211 - Flags: approval-mozilla-beta?
Attachment #8421211 - Flags: approval-mozilla-beta+
Attachment #8421211 - Flags: approval-mozilla-aurora?
Attachment #8421211 - Flags: approval-mozilla-aurora+
Fixing the status flags so it actually shows up when looking for things to uplift...
Target Milestone: mozilla28 → mozilla32
Why is this bug marked FIXED while disabled everywhere?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
> 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)?
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".
Target Milestone: mozilla32 → ---
(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
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.
(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.
(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.
Removing relnote keyword. We are trying to get ride of this keyword. Please use relnote-firefox once it is ready.
Keywords: relnote
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 */ ]});
Blocks: 1186948
Whiteboard: Websites tech advo bug 934107 → [fingerprinting] Websites tech advo bug 934107
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.
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.
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 ago9 years ago
Keywords: dev-doc-needed
Resolution: --- → WONTFIX
Removed the site compat doc.
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)
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)
@ 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)
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.
(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)
> 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)
Oh, is there some bug I should mark those dependent on so I know when I can reland them?
Flags: needinfo?(cpeterson)
Status: RESOLVED → REOPENED
Depends on: 1303076, 1270366, 1270364
Resolution: WONTFIX → ---
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.
(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 ago8 years ago
Flags: needinfo?(cpeterson)
Resolution: --- → WONTFIX
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?
(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.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: