Summary: | Hide all plugin names except Flash, Java, and QuickTime | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||||||
Component: | WebCore Misc. | Assignee: | Brent Fulgham <bfulgham> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | andersca, ap, bfulgham, buildbot, dbates, oliver, rmondello, rniwa, sam, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=151782 | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 153657 | ||||||||||||
Attachments: |
|
Description
Brent Fulgham
2015-09-09 15:45:19 PDT
This bug attempts to implement the recommendation from the EFF regarding decreasing the attack surface for Fingerprinting (see <https://panopticlick.eff.org/browser-uniqueness.pdf>). "There are probably stronger ease-of-development arguments for making plu- gins enumeratable, but the example of IE shows that it is not strictly necessary. We recommend that browsers switch to confirm-only testing for fonts and plu- gins, with an exponential backoff to prevent exhaustive searches by malicious javascript." See <https://panopticlick.eff.org> for some simple fingerprinting measurements. Created attachment 260885 [details]
Patch
Comment on attachment 260885 [details] Patch Attachment 260885 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/155532 New failing tests: plugins/plugin-javascript-access.html plugins/navigator-mimeTypes-length.html Created attachment 260887 [details]
Archive of layout-test-results from ews103 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 260885 [details] Patch Attachment 260885 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/155551 New failing tests: plugins/plugin-javascript-access.html plugins/navigator-mimeTypes-length.html Created attachment 260888 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
These errors are happening because the tests iterate over the installed plugins until they find the desired entry. We should just query for existence, rather than do this iteration. Created attachment 260932 [details]
Patch
Comment on attachment 260932 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260932&action=review > Source/WebCore/plugins/PluginData.cpp:61 > + if ((plugin.name.findIgnoringCase("Shockwave") == notFound) > + && (plugin.name.findIgnoringCase("QuickTime") == notFound) > + && (plugin.name.findIgnoringCase("Java") == notFound)) { > + continue; Need a comment explaining the rationale behind this list. Otherwise it’s too mysterious in the future. I suggest putting this predicate into a separate named function by itself. The correct function to use is containsIgnoringASCIICase rather than findIgnoringCase != notFound. static bool shouldBePubliclyVisible(const PluginInfo& plugin) { // For practical website compatibility there are a few plug-ins that need to be visible ... <good explanation> return plugin.name.containsIgnoringASCIICase("Shockwave") || ... } ... if (shouldBePubliclyVisible(plugin)) plugins.append(WTF::move(plugin)); > Source/WebCore/plugins/PluginData.cpp:68 > + return codePointCompare(a.name, b.name) < 0; Consider using codePointCompareLessThan? Is there a reason we are sorting in a case sensitive way rather than a case folding way? > Source/WebCore/plugins/PluginData.h:93 > Vector<PluginInfo> webVisiblePlugins() const; > + Vector<PluginInfo> publiclyVisiblePlugins() const; Very unclear naming here. How is "web visible" different from "publicly visible". We should use more precise terminology. > Source/WebCore/testing/Internals.cpp:3095 > + Document* document = contextDocument(); > + if (!document) > + return; > + > + if (Page* page = document->page()) > + page->setShowAllPlugins(show); Slightly strange combination of idioms. Early return for one null check, but nested if for the other. Comment on attachment 260932 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260932&action=review >> Source/WebCore/plugins/PluginData.cpp:61 >> + continue; > > Need a comment explaining the rationale behind this list. Otherwise it’s too mysterious in the future. > > I suggest putting this predicate into a separate named function by itself. > > The correct function to use is containsIgnoringASCIICase rather than findIgnoringCase != notFound. > > static bool shouldBePubliclyVisible(const PluginInfo& plugin) > { > // For practical website compatibility there are a few plug-ins that need to be visible ... <good explanation> > return plugin.name.containsIgnoringASCIICase("Shockwave") || ... > } > > ... > > if (shouldBePubliclyVisible(plugin)) > plugins.append(WTF::move(plugin)); Great ideas! I'll incorporate them. >> Source/WebCore/plugins/PluginData.cpp:68 >> + return codePointCompare(a.name, b.name) < 0; > > Consider using codePointCompareLessThan? > > Is there a reason we are sorting in a case sensitive way rather than a case folding way? Neat -- I didn't know about codePointCompareLessThan! I'll switch. No. I just want a consistent ordering returned to the client to minimize differences from systems to system. Is there a preferred sorting criteria besides codePointCompareLessThan? >> Source/WebCore/plugins/PluginData.h:93 >> + Vector<PluginInfo> publiclyVisiblePlugins() const; > > Very unclear naming here. How is "web visible" different from "publicly visible". We should use more precise terminology. The "web visible" plugins are the set of things that are allowed to be accessed from the web based on the "WebPlatformStrategies" settings. The "publiclyVisiblePlugins" are the subset of the "webVisiblePlugins" that we want to publicly advertise. All others should only be findable using a specific existence test. Maybe "advertisedWebVisiblePlugins" or "publiclyVisibleWebVisiblePlugins"? >> Source/WebCore/testing/Internals.cpp:3095 >> + page->setShowAllPlugins(show); > > Slightly strange combination of idioms. Early return for one null check, but nested if for the other. I'll revise this to be consistent. Committed r191211: <http://trac.webkit.org/changeset/191211> |