155432 2016-03-14 05:08:26 -0700 REGRESSION (r197724): [GTK] Web Inspector: Images being blocked by CSP 2.0 2016-03-17 23:53:47 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Local Build Unspecified Unspecified RESOLVED FIXED Gtk, Regression P2 Normal --- 155585 1 cgarcia webkit-unassigned bfulgham bugs-noreply commit-queue dbates joepeck mcatanzaro mkwst oldest_to_newest 1174418 0 cgarcia 2016-03-14 05:08:26 -0700 This is similar problem to bug #155182, but in our case the web inspector uses GResources, that are still being blocked. I guess we could add resource scheme to the list of schemes allowed by the web inspector, but I think we really want to always allow GResources by the CSP, since they are in practice like a data URL. 1174423 1 273962 cgarcia 2016-03-14 05:14:00 -0700 Created attachment 273962 Patch This patch fixes the problem, but I'm not happy with the platform ifdefs there. I wonder if we could use a existing method like SchemeRegistry::shouldTreatURLSchemeAsSecure(), but I'm not sure we also want to allow the other "secure" schemes, or add a new one specific for this to SchemeRegistry. 1174453 2 mcatanzaro 2016-03-14 07:49:42 -0700 I think this makes sense, let's wait for Dan Bates to look at it as well. 1174507 3 273962 darin 2016-03-14 10:00:11 -0700 Comment on attachment 273962 Patch OK. A cleaner way to do this would be to make a function that does the protocolIsData check that also does the procotolIs("resource") check on GTK and use it here. We’d want to audit all the other call sites of protocolIsData. 1174918 4 cgarcia 2016-03-14 23:59:53 -0700 (In reply to comment #3) > Comment on attachment 273962 [details] > Patch > > OK. A cleaner way to do this would be to make a function that does the > protocolIsData check that also does the procotolIs("resource") check on GTK > and use it here. We’d want to audit all the other call sites of > protocolIsData. Yes, probably better as follow up patch. I also wonder if "applewebdata" is similar to a GResource and should be considered as well. 1174919 5 cgarcia 2016-03-15 00:01:38 -0700 Committed r198201: <http://trac.webkit.org/changeset/198201> 1174978 6 273962 dbates 2016-03-15 08:51:36 -0700 Comment on attachment 273962 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273962&action=review > Source/WebCore/ChangeLog:8 > + The GTK+ port Web Inspector uses GResources for all internal Is this only necessary for the Web Inspector? If so, how did you come to the decision to allow the interpretation of source * for resource URLs for images and audio/video sub resources on all web pages as opposed to modifying the img-src and media-src directives in the Web Inspector's CSP policy to allow GResources. > Source/WebCore/ChangeLog:9 > + resources (images, fonts, scripts, etc.) that are now blocked by Does this issue affect JavaScripts scripts? I mean, the proposed change only affects the interpretation of source * for images and audio/video subresources. 1174980 7 cgarcia 2016-03-15 08:55:39 -0700 (In reply to comment #6) > Comment on attachment 273962 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=273962&action=review > > > Source/WebCore/ChangeLog:8 > > + The GTK+ port Web Inspector uses GResources for all internal > > Is this only necessary for the Web Inspector? If so, how did you come to the > decision to allow the interpretation of source * for resource URLs for > images and audio/video sub resources on all web pages as opposed to > modifying the img-src and media-src directives in the Web Inspector's CSP > policy to allow GResources. Because GResources are like a data URL in practice, so if we allow data URLs I don't see why not allowing GResources. They are always safe, so I don't think they should be blocked in any case. > > Source/WebCore/ChangeLog:9 > > + resources (images, fonts, scripts, etc.) that are now blocked by > > Does this issue affect JavaScripts scripts? I mean, the proposed change only > affects the interpretation of source * for images and audio/video > subresources. No, scripts were not affected. 1175630 8 dbates 2016-03-16 20:42:40 -0700 (In reply to comment #7) > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=273962&action=review > > > > > Source/WebCore/ChangeLog:8 > > > + The GTK+ port Web Inspector uses GResources for all internal > > > > Is this only necessary for the Web Inspector? If so, how did you come to the > > decision to allow the interpretation of source * for resource URLs for > > images and audio/video sub resources on all web pages as opposed to > > modifying the img-src and media-src directives in the Web Inspector's CSP > > policy to allow GResources. > > Because GResources are like a data URL in practice, so if we allow data URLs > I don't see why not allowing GResources. They are always safe, so I don't > think they should be blocked in any case. > Unless we know that there are popular web sites that make use of resource URLs and define a CSP that depends on * allowing such URLs then we should revert <http://trac.webkit.org/changeset/198201> and take a similar approach as in the fix for bug 155182 to add resource: to the image-src and media-src directives in the CSP policy for the Web Inspector. Additional remarks: We should not be making a exception for resource URLs in ContentSecurityPolicySourceList::isProtocolAllowedByStar(). Source * should be interpreted as strictly as possible so as to more closely match (2) of section Matching Source Expressions of the Content Security Policy Level 2 spec. [1] and avoid surprising web developers. Following from the spec. we do not want * to match data URLs. As mentioned in the ChangeLog description for changeset 197724,<http://trac.webkit.org/changeset/197724>, I chose to make an exception for the image-src and media-src directives and allow data URLs to mitigate web compatibility issues (see remark [2]). [1] <https://www.w3.org/TR/2015/CR-CSP2-20150721/#match-source-expression> [2] This decision was influenced in part by Mozilla's experience in restricting the interpretation of source * as remarked in bug 154122, comment 2. 1175651 9 cgarcia 2016-03-16 23:38:20 -0700 (In reply to comment #8) > (In reply to comment #7) > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=273962&action=review > > > > > > > Source/WebCore/ChangeLog:8 > > > > + The GTK+ port Web Inspector uses GResources for all internal > > > > > > Is this only necessary for the Web Inspector? If so, how did you come to the > > > decision to allow the interpretation of source * for resource URLs for > > > images and audio/video sub resources on all web pages as opposed to > > > modifying the img-src and media-src directives in the Web Inspector's CSP > > > policy to allow GResources. > > > > Because GResources are like a data URL in practice, so if we allow data URLs > > I don't see why not allowing GResources. They are always safe, so I don't > > think they should be blocked in any case. > > > > Unless we know that there are popular web sites that make use of resource > URLs and define a CSP that depends on * allowing such URLs then we should > revert <http://trac.webkit.org/changeset/198201> and take a similar approach > as in the fix for bug 155182 to add resource: to the image-src and media-src > directives in the CSP policy for the Web Inspector. No there isn't any website using resource URLs, because GResources are something internal to the application in the client side. We use GResources inside WebKit itself to compile all the resources (inspector files, but also webcire icons) in the shared library. That way we don't need to install the resources and find them in the file system at runtime, they are always available to any application linking to the library. GTK+ applications also compile their own GResources in their injected bundle library to make their own resources available to the web process. It's typically used for user scripts, custom error pages, about: pages, etc. So, GResources shouldn't be affected by the CSP at all, because they are never used by websites, but by applications as an implementation detail. > Additional remarks: > > We should not be making a exception for resource URLs in > ContentSecurityPolicySourceList::isProtocolAllowedByStar(). Source * should > be interpreted as strictly as possible so as to more closely match (2) of > section Matching Source Expressions of the Content Security Policy Level 2 > spec. [1] and avoid surprising web developers. Following from the spec. we > do not want * to match data URLs. As mentioned in the ChangeLog description > for changeset 197724,<http://trac.webkit.org/changeset/197724>, I chose to > make an exception for the image-src and media-src directives and allow data > URLs to mitigate web compatibility issues (see remark [2]). > > [1] <https://www.w3.org/TR/2015/CR-CSP2-20150721/#match-source-expression> > [2] This decision was influenced in part by Mozilla's experience in > restricting the interpretation of source * as remarked in bug 154122, > comment 2. 1175722 10 mcatanzaro 2016-03-17 06:29:10 -0700 See also: https://blogs.gnome.org/alexl/2012/01/26/resources-in-glib/ 1175729 11 dbates 2016-03-17 07:34:34 -0700 (In reply to comment #9) > > > Unless we know that there are popular web sites that make use of resource > > URLs and define a CSP that depends on * allowing such URLs then we should > > revert <http://trac.webkit.org/changeset/198201> and take a similar approach > > as in the fix for bug 155182 to add resource: to the image-src and media-src > > directives in the CSP policy for the Web Inspector. > > No there isn't any website using resource URLs, because GResources are > something internal to the application in the client side. We use GResources > inside WebKit itself to compile all the resources (inspector files, but also > webcire icons) in the shared library. That way we don't need to install the > resources and find them in the file system at runtime, they are always > available to any application linking to the library. GTK+ applications also > compile their own GResources in their injected bundle library to make their > own resources available to the web process. It's typically used for user > scripts, custom error pages, about: pages, etc. So, GResources shouldn't be > affected by the CSP at all, because they are never used by websites, but by > applications as an implementation detail. > Then please revert <http://trac.webkit.org/changeset/198201> and add resource: to the list of schemes allowed by the web inspector. 1175730 12 cgarcia 2016-03-17 07:39:51 -0700 (In reply to comment #11) > (In reply to comment #9) > > > > > Unless we know that there are popular web sites that make use of resource > > > URLs and define a CSP that depends on * allowing such URLs then we should > > > revert <http://trac.webkit.org/changeset/198201> and take a similar approach > > > as in the fix for bug 155182 to add resource: to the image-src and media-src > > > directives in the CSP policy for the Web Inspector. > > > > No there isn't any website using resource URLs, because GResources are > > something internal to the application in the client side. We use GResources > > inside WebKit itself to compile all the resources (inspector files, but also > > webcire icons) in the shared library. That way we don't need to install the > > resources and find them in the file system at runtime, they are always > > available to any application linking to the library. GTK+ applications also > > compile their own GResources in their injected bundle library to make their > > own resources available to the web process. It's typically used for user > > scripts, custom error pages, about: pages, etc. So, GResources shouldn't be > > affected by the CSP at all, because they are never used by websites, but by > > applications as an implementation detail. > > > > Then please revert <http://trac.webkit.org/changeset/198201> and add > resource: to the list of schemes allowed by the web inspector. My concern is whether that could affect an applications using for example a user style sheet with url(resource://). 1175749 13 dbates 2016-03-17 09:18:23 -0700 (In reply to comment #12) > My concern is whether that could affect an applications using for example a > user style sheet with url(resource://). I suggest that we revert <http://trac.webkit.org/changeset/198201>, add resource: to the list of schemes allowed by the Web Inspector, and wait for bug reports to come in with respect to third-party GTK applications that make use of a user stylesheet and reference resource URLs (or do you know of any bug reports?). 1175756 14 commit-queue 2016-03-17 09:39:52 -0700 Re-opened since this is blocked by bug 155585 1175761 15 274297 cgarcia 2016-03-17 09:45:23 -0700 Created attachment 274297 Patch 1175793 16 274297 dbates 2016-03-17 11:47:12 -0700 Comment on attachment 274297 Patch Thank you for reverting <http://trac.webkit.org/changeset/198201> and updating the CSP policy of the Web Inspector. 1176080 17 cgarcia 2016-03-17 23:53:47 -0700 Committed r198382: <http://trac.webkit.org/changeset/198382> 273962 2016-03-14 05:14:00 -0700 2016-03-17 09:45:23 -0700 Patch wk2-gtk-inspector-resources.diff text/plain 2169 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAyYWVjYTJhLi4zMjEzYzIyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTYtMDMtMTQgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFJFR1JFU1NJT04gKHIxOTc3MjQpOiBbR1RLXSBXZWIgSW5zcGVjdG9yOiBJ bWFnZXMgYmVpbmcgYmxvY2tlZCBieSBDU1AgMi4wCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTU0MzIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgR1RLKyBwb3J0IFdlYiBJbnNwZWN0b3IgdXNlcyBH UmVzb3VyY2VzIGZvciBhbGwgaW50ZXJuYWwKKyAgICAgICAgcmVzb3VyY2VzIChpbWFnZXMsIGZv bnRzLCBzY3JpcHRzLCBldGMuKSB0aGF0IGFyZSBub3cgYmxvY2tlZCBieQorICAgICAgICB0aGUg Q1NQLiBHUmVzb3VjZXMgYXJlIGxpa2UgZGF0YSBVUkxzIGluIHByYWN0aWNlLCBzbyB3ZSBzaG91 bGQKKyAgICAgICAgYWx3YXlzIGFsbG93IHRoZW0uCisKKyAgICAgICAgKiBwYWdlL2NzcC9Db250 ZW50U2VjdXJpdHlQb2xpY3lTb3VyY2VMaXN0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNvbnRl bnRTZWN1cml0eVBvbGljeVNvdXJjZUxpc3Q6OmlzUHJvdG9jb2xBbGxvd2VkQnlTdGFyKToKKwog MjAxNi0wMy0wOCAgQW50b25pbyBHb21lcyAgPHRvbmlraXRvb0B3ZWJraXQub3JnPgogCiAgICAg ICAgIFNjcm9sbGluZyBkb2VzIG5vdCB3b3JrIHdoZW4gdGhlIG1vdXNlIGRvd24gaXMgaGFuZGxl ZCBieSBhIG5vZGUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvY3NwL0NvbnRlbnRT ZWN1cml0eVBvbGljeVNvdXJjZUxpc3QuY3BwIGIvU291cmNlL1dlYkNvcmUvcGFnZS9jc3AvQ29u dGVudFNlY3VyaXR5UG9saWN5U291cmNlTGlzdC5jcHAKaW5kZXggNDA4YjQwZS4uMzUzOTNmZSAx MDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGFnZS9jc3AvQ29udGVudFNlY3VyaXR5UG9saWN5 U291cmNlTGlzdC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGFnZS9jc3AvQ29udGVudFNlY3Vy aXR5UG9saWN5U291cmNlTGlzdC5jcHAKQEAgLTEwMiwxMCArMTAyLDE3IEBAIGJvb2wgQ29udGVu dFNlY3VyaXR5UG9saWN5U291cmNlTGlzdDo6aXNQcm90b2NvbEFsbG93ZWRCeVN0YXIoY29uc3Qg VVJMJiB1cmwpIGNvCiAgICAgLy8gRklYTUU6IFdlIHNob3VsZCBub3QgaGFyZGNvZGUgdGhlIGRp cmVjdGl2ZSBuYW1lcy4gV2Ugc2hvdWxkIG1ha2UgdXNlIG9mIHRoZSBjb25zdGFudHMgaW4gQ29u dGVudFNlY3VyaXR5UG9saWN5RGlyZWN0aXZlTGlzdC5jcHAuCiAgICAgLy8gU2VlIDxodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU1MTMzPi4KICAgICBib29sIGlzQWxs b3dlZCA9IHVybC5wcm90b2NvbElzSW5IVFRQRmFtaWx5KCk7Ci0gICAgaWYgKGVxdWFsTGV0dGVy c0lnbm9yaW5nQVNDSUlDYXNlKG1fZGlyZWN0aXZlTmFtZSwgImltZy1zcmMiKSkKKyAgICBpZiAo ZXF1YWxMZXR0ZXJzSWdub3JpbmdBU0NJSUNhc2UobV9kaXJlY3RpdmVOYW1lLCAiaW1nLXNyYyIp KSB7CiAgICAgICAgIGlzQWxsb3dlZCB8PSB1cmwucHJvdG9jb2xJc0RhdGEoKTsKLSAgICBlbHNl IGlmIChlcXVhbExldHRlcnNJZ25vcmluZ0FTQ0lJQ2FzZShtX2RpcmVjdGl2ZU5hbWUsICJtZWRp YS1zcmMiKSkKKyNpZiBQTEFURk9STShHVEspCisgICAgICAgIGlzQWxsb3dlZCB8PSB1cmwucHJv dG9jb2xJcygicmVzb3VyY2UiKTsKKyNlbmRpZgorICAgIH0gZWxzZSBpZiAoZXF1YWxMZXR0ZXJz SWdub3JpbmdBU0NJSUNhc2UobV9kaXJlY3RpdmVOYW1lLCAibWVkaWEtc3JjIikpIHsKICAgICAg ICAgaXNBbGxvd2VkIHw9IHVybC5wcm90b2NvbElzRGF0YSgpIHx8IHVybC5wcm90b2NvbElzQmxv YigpOworI2lmIFBMQVRGT1JNKEdUSykKKyAgICAgICAgaXNBbGxvd2VkIHw9IHVybC5wcm90b2Nv bElzKCJyZXNvdXJjZSIpOworI2VuZGlmCisgICAgfQogICAgIHJldHVybiBpc0FsbG93ZWQ7CiB9 CiAK 274297 2016-03-17 09:45:23 -0700 2016-03-17 11:47:12 -0700 Patch winspector-images.diff text/plain 1531 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IDNmMWEyZDguLmMwNzRhMGIgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkluc3BlY3Rv clVJL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDE2LTAzLTE3ICBDYXJsb3MgR2FyY2lh IENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OIChyMTk3 NzI0KTogW0dUS10gV2ViIEluc3BlY3RvcjogSW1hZ2VzIGJlaW5nIGJsb2NrZWQgYnkgQ1NQIDIu MAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU1NDMy CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWxsb3cg V2ViIEluc3BlY3RvciB0byBsb2FkIHJlc291cmNlOiBpbWFnZSByZXNvdXJjZXMuCisKKyAgICAg ICAgKiBVc2VySW50ZXJmYWNlL01haW4uaHRtbDoKKwogMjAxNi0wMy0wOCAgSm9zZXBoIFBlY29y YXJvICA8cGVjb3Jhcm9AYXBwbGUuY29tPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IEltYWdl cyBiZWluZyBibG9ja2VkIGJ5IENTUCAyLjAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0 b3JVSS9Vc2VySW50ZXJmYWNlL01haW4uaHRtbCBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2Vy SW50ZXJmYWNlL01haW4uaHRtbAppbmRleCBjYzE1ZjZhLi5hODZhM2I0IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9NYWluLmh0bWwKKysrIGIvU291cmNl L1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvTWFpbi5odG1sCkBAIC0yNiw3ICsyNiw3IEBA CiA8aHRtbD4KIDxoZWFkPgogICAgIDxtZXRhIGh0dHAtZXF1aXY9IkNvbnRlbnQtVHlwZSIgY29u dGVudD0idGV4dC9odG1sOyBjaGFyc2V0PXV0Zi04Ij4KLSAgICA8bWV0YSBodHRwLWVxdWl2PSJD b250ZW50LVNlY3VyaXR5LVBvbGljeSIgY29udGVudD0iZGVmYXVsdC1zcmMgJ3NlbGYnOyBpbWct c3JjICogZmlsZTogYmxvYjo7IGNvbm5lY3Qtc3JjICo7IG1lZGlhLXNyYyAqIGJsb2I6OyBmb250 LXNyYyAqIGJsb2I6OyBzdHlsZS1zcmMgJ3NlbGYnICd1bnNhZmUtaW5saW5lJzsgc2NyaXB0LXNy YyAnc2VsZicgJ3Vuc2FmZS1pbmxpbmUnIj4KKyAgICA8bWV0YSBodHRwLWVxdWl2PSJDb250ZW50 LVNlY3VyaXR5LVBvbGljeSIgY29udGVudD0iZGVmYXVsdC1zcmMgJ3NlbGYnOyBpbWctc3JjICog ZmlsZTogYmxvYjogcmVzb3VyY2U6OyBjb25uZWN0LXNyYyAqOyBtZWRpYS1zcmMgKiBibG9iOjsg Zm9udC1zcmMgKiBibG9iOjsgc3R5bGUtc3JjICdzZWxmJyAndW5zYWZlLWlubGluZSc7IHNjcmlw dC1zcmMgJ3NlbGYnICd1bnNhZmUtaW5saW5lJyI+CiAKICAgICA8bGluayByZWw9InN0eWxlc2hl ZXQiIGhyZWY9IkV4dGVybmFsL0NvZGVNaXJyb3IvY29kZW1pcnJvci5jc3MiPgogCg==