Bug 91919 - Implement the plugin-types Content Security Policy directive.
: Implement the plugin-types Content Security Policy directive.
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
: WebExposed
:
: 85558 93949 94252
  Show dependency treegraph
 
Reported: 2012-07-20 22:34 PST by
Modified: 2012-08-16 14:19 PST (History)


Attachments
WIP (19.48 KB, patch)
2012-07-20 23:43 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
First pass. (26.04 KB, patch)
2012-07-21 13:41 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
Patch (46.31 KB, patch)
2012-08-07 06:53 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
Adam's feedback. (46.25 KB, patch)
2012-08-07 14:15 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-06 (315.18 KB, application/zip)
2012-08-07 14:57 PST, WebKit Review Bot
no flags Details
Reset test result. (46.26 KB, patch)
2012-08-07 15:05 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
Rebased onto ToT. (46.79 KB, patch)
2012-08-13 11:43 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-07-20 22:34:09 PST
The CSP 1.1 editor's draft defines the `plugin-types` directive, which limits the plugins that can load in a protected resource by whitelisting specific MIME types[1]. As this directive is still experimental, we should implement it behind the CSP_NEXT flag.

[1]: https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#plugin-types--experimental
------- Comment #1 From 2012-07-20 23:43:12 PST -------
Created an attachment (id=153652) [details]
WIP
------- Comment #2 From 2012-07-20 23:44:02 PST -------
Uploaded WIP patch. Can you verify that I'm going in the right direction, Adam, while I fiddle with the FIXMEs? :)
------- Comment #3 From 2012-07-20 23:47:37 PST -------
(In reply to comment #2)
> Uploaded WIP patch. Can you verify that I'm going in the right direction, Adam, while I fiddle with the FIXMEs? :)

(Note the `applet` hard-coding in particular.)
------- Comment #4 From 2012-07-21 13:41:50 PST -------
Created an attachment (id=153676) [details]
First pass.
------- Comment #5 From 2012-07-22 20:41:58 PST -------
(From update of attachment 153676 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=153676&action=review

> Source/WebCore/loader/SubframeLoader.cpp:127
> -        if (!m_frame->document()->contentSecurityPolicy()->allowObjectFromSource(url))
> +        if (!m_frame->document()->contentSecurityPolicy()->allowObjectFromSource(url)
> +            || !m_frame->document()->contentSecurityPolicy()->allowPluginType(mimeType, url))

I'm not sure this is sufficient.  The actual type that gets instantiated for a plugin is really complicated.  Hixie has a crazy diagram that shows how it all works.  As an example, we should test <object data="..."></object> with a response that has a Content-Type header that specifies a plugin.

If you look through the Chromium code, you'll find an implementation of the "content settings" feature that blocks plugins by type.  It might be worth looking at how that code works.

> Source/WebCore/page/ContentSecurityPolicy.cpp:820
> +    return checkPluginTypeAndReportViolation(type, "Refused to load '" + url.string() + "' (MIME type `" + type + "`) because it violates the following Content Security Policy Directive: ");

I'm not sure whether we should use `.  Maybe just ' would be better?

> Source/WebCore/page/ContentSecurityPolicy.cpp:1008
> +    m_pluginTypesDirective = String(begin, end - begin);

Isn't this just |value|?  If we just store |value| here rather than calling String(), we can avoid a malloc and a memcpy because we'll just take another reference to value.

> Source/WebCore/page/ContentSecurityPolicy.cpp:1019
> +    while (position < end) {

This probably isn't the right way to handle errors here.  We need to make the spec more precise as to how to parse this directive.  What we should do is first split on whitespace and then process each non-whitespace sequence.  For example, that will parse "image/jpg boo text/html" as ["image/jpg", "text/html"] rather than ["image/jpg"], whcih is what this code will do.

This code also has a bug in that it empties out m_pluginTypesDirective even if it makes m_pluginTypes non-empty.
------- Comment #6 From 2012-07-22 20:42:26 PST -------
jochen knows about blocking plugins by type.  It would be good to get his input on this patch.
------- Comment #7 From 2012-07-22 21:25:20 PST -------
(In reply to comment #5)
> (From update of attachment 153676 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153676&action=review
> 
> > Source/WebCore/loader/SubframeLoader.cpp:127
> > -        if (!m_frame->document()->contentSecurityPolicy()->allowObjectFromSource(url))
> > +        if (!m_frame->document()->contentSecurityPolicy()->allowObjectFromSource(url)
> > +            || !m_frame->document()->contentSecurityPolicy()->allowPluginType(mimeType, url))
> 
> I'm not sure this is sufficient.  The actual type that gets instantiated for a plugin is really complicated.  Hixie has a crazy diagram that shows how it all works.  As an example, we should test <object data="..."></object> with a response that has a Content-Type header that specifies a plugin.
> 
> If you look through the Chromium code, you'll find an implementation of the "content settings" feature that blocks plugins by type.  It might be worth looking at how that code works.

Alright. I knew it seemed too easy. :)

I'll poke the Munich folks about content settings code this week when I'm back in town.

> 
> > Source/WebCore/page/ContentSecurityPolicy.cpp:820
> > +    return checkPluginTypeAndReportViolation(type, "Refused to load '" + url.string() + "' (MIME type `" + type + "`) because it violates the following Content Security Policy Directive: ");
> 
> I'm not sure whether we should use `.  Maybe just ' would be better?

You're right. The backtic is an artifact of my obsession with Markdown; single quotes are probably appropriate here.

> 
> > Source/WebCore/page/ContentSecurityPolicy.cpp:1008
> > +    m_pluginTypesDirective = String(begin, end - begin);
> 
> Isn't this just |value|?  If we just store |value| here rather than calling String(), we can avoid a malloc and a memcpy because we'll just take another reference to value.

Good point. I'll fix this.

> 
> > Source/WebCore/page/ContentSecurityPolicy.cpp:1019
> > +    while (position < end) {
> 
> This probably isn't the right way to handle errors here.  We need to make the spec more precise as to how to parse this directive.  What we should do is first split on whitespace and then process each non-whitespace sequence.  For example, that will parse "image/jpg boo text/html" as ["image/jpg", "text/html"] rather than ["image/jpg"], whcih is what this code will do.
> 
> This code also has a bug in that it empties out m_pluginTypesDirective even if it makes m_pluginTypes non-empty.

`m_pluginTypesDirective` being empty is sufficient to make the current implementation fail to load any plugin. This implementation takes a draconian approach: given `image/jpg boo text/html`, it would block all plugins, as `boo` isn't recognizably in the form of a MIME type. See the check in CSPDirectiveList::checkPluginTypeAndReportViolation for detail. That's arguably the wrong approach, but it seemed consistent with the route we took regarding `script-nonce` (which boiled down to "That's not a nonce; no script for you.").

If that is the route we go down, then you're still right: we should clear out `m_pluginTypes`, which would remove the necessity of emptying the directive text. That's a much better implementation.
------- Comment #8 From 2012-07-22 21:52:10 PST -------
> `m_pluginTypesDirective` being empty is sufficient to make the current implementation fail to load any plugin. This implementation takes a draconian approach: given `image/jpg boo text/html`, it would block all plugins, as `boo` isn't recognizably in the form of a MIME type. See the check in CSPDirectiveList::checkPluginTypeAndReportViolation for detail. That's arguably the wrong approach, but it seemed consistent with the route we took regarding `script-nonce` (which boiled down to "That's not a nonce; no script for you.").

For source-list, we skip over sources that we don't understand, which gives more scope for future additions to the syntax...

> If that is the route we go down, then you're still right: we should clear out `m_pluginTypes`, which would remove the necessity of emptying the directive text. That's a much better implementation.

It's not clear to me which behavior is better.  We should think for a bit about whether there are future additions we might make to the syntax...
------- Comment #9 From 2012-07-22 22:13:06 PST -------
(In reply to comment #8)
> > `m_pluginTypesDirective` being empty is sufficient to make the current implementation fail to load any plugin. This implementation takes a draconian approach: given `image/jpg boo text/html`, it would block all plugins, as `boo` isn't recognizably in the form of a MIME type. See the check in CSPDirectiveList::checkPluginTypeAndReportViolation for detail. That's arguably the wrong approach, but it seemed consistent with the route we took regarding `script-nonce` (which boiled down to "That's not a nonce; no script for you.").
> 
> For source-list, we skip over sources that we don't understand, which gives more scope for future additions to the syntax...
> 
> > If that is the route we go down, then you're still right: we should clear out `m_pluginTypes`, which would remove the necessity of emptying the directive text. That's a much better implementation.
> 
> It's not clear to me which behavior is better.  We should think for a bit about whether there are future additions we might make to the syntax...

I edited the draft 1.1 spec to embody this behavior in https://dvcs.w3.org/hg/content-security-policy/rev/5e6fdb226239, but even though I can't think of any good reason to allow non-MIME-style types in the future, it's certainly a possibility I hadn't considered. I'll drop a note to the list.
------- Comment #10 From 2012-07-23 11:31:12 PST -------
(From update of attachment 153676 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=153676&action=review

>>> Source/WebCore/loader/SubframeLoader.cpp:127
>>> +            || !m_frame->document()->contentSecurityPolicy()->allowPluginType(mimeType, url))
>> 
>> I'm not sure this is sufficient.  The actual type that gets instantiated for a plugin is really complicated.  Hixie has a crazy diagram that shows how it all works.  As an example, we should test <object data="..."></object> with a response that has a Content-Type header that specifies a plugin.
>> 
>> If you look through the Chromium code, you'll find an implementation of the "content settings" feature that blocks plugins by type.  It might be worth looking at how that code works.
> 
> Alright. I knew it seemed too easy. :)
> 
> I'll poke the Munich folks about content settings code this week when I'm back in town.

I'm not sure that matching against the mime type of the plugin that ends up being loaded is achievable at all - we currently just take whether WebKit gives us, and then decide how to render the plugin, but never actually tell webkit what we did (i.e. it might by a click2play place holder that creates a render view to displays its contents).
------- Comment #11 From 2012-07-23 13:54:49 PST -------
(From update of attachment 153676 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=153676&action=review

>>>> Source/WebCore/loader/SubframeLoader.cpp:127
>>>> +            || !m_frame->document()->contentSecurityPolicy()->allowPluginType(mimeType, url))
>>> 
>>> I'm not sure this is sufficient.  The actual type that gets instantiated for a plugin is really complicated.  Hixie has a crazy diagram that shows how it all works.  As an example, we should test <object data="..."></object> with a response that has a Content-Type header that specifies a plugin.
>>> 
>>> If you look through the Chromium code, you'll find an implementation of the "content settings" feature that blocks plugins by type.  It might be worth looking at how that code works.
>> 
>> Alright. I knew it seemed too easy. :)
>> 
>> I'll poke the Munich folks about content settings code this week when I'm back in town.
> 
> I'm not sure that matching against the mime type of the plugin that ends up being loaded is achievable at all - we currently just take whether WebKit gives us, and then decide how to render the plugin, but never actually tell webkit what we did (i.e. it might by a click2play place holder that creates a render view to displays its contents).

Right, neither the actual MIME type nor the actual WebPlugin instance might be what we expect it to be; for example the MIME type could be empty here, in which case we'd try to find a matching plug-in based on the URL extension.

However, if the CSP is only used for *whitelisting* certain plug-ins, going by MIME type shouldn't be a problem: If a page specifies a certain MIME type, we refuse to load a plug-in with a different MIME type (to avoid confusion attacks; see AllowMimeTypeMismatch() in Chromium plugin_list.cc). That means, if we have e.g. a CSP that only allows the "application/shockwave-flash" MIME type, we can block all other MIME types here at the Webkit level, and our plug-in implementation already blocks all other MIME types. It's true that we can't tell here which WebPlugin is actually going to handle it (could be a placeholder, or this or that version of Flash), but that's not something that this layer knows about anyway.

The only thing we'd need to make sure is that we don't allow a MIME type of application/octet-stream in the CSP, which is the only non-empty MIME type that can be overridden (because that would effectively allow any plug-in again).

Adam, I'd be interested in seeing that crazy diagram sometime ;-)
------- Comment #12 From 2012-07-23 14:00:24 PST -------
> Adam, I'd be interested in seeing that crazy diagram sometime ;-)

I looked around a bit and couldn't lay my hands on it.  Someone in #whatwg might be able to find it.

Another possibility here is to pass the list of allowed MIME types to the embedder and have it do the work of making sure we don't violate the policy.
------- Comment #13 From 2012-07-23 16:35:09 PST -------
(In reply to comment #11)
> (From update of attachment 153676 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153676&action=review
> 
> >>>> Source/WebCore/loader/SubframeLoader.cpp:127
> >>>> +            || !m_frame->document()->contentSecurityPolicy()->allowPluginType(mimeType, url))
> >>> 
> >>> I'm not sure this is sufficient.  The actual type that gets instantiated for a plugin is really complicated.  Hixie has a crazy diagram that shows how it all works.  As an example, we should test <object data="..."></object> with a response that has a Content-Type header that specifies a plugin.
> >>> 
> >>> If you look through the Chromium code, you'll find an implementation of the "content settings" feature that blocks plugins by type.  It might be worth looking at how that code works.
> >> 
> >> Alright. I knew it seemed too easy. :)
> >> 
> >> I'll poke the Munich folks about content settings code this week when I'm back in town.
> > 
> > I'm not sure that matching against the mime type of the plugin that ends up being loaded is achievable at all - we currently just take whether WebKit gives us, and then decide how to render the plugin, but never actually tell webkit what we did (i.e. it might by a click2play place holder that creates a render view to displays its contents).
> 
> Right, neither the actual MIME type nor the actual WebPlugin instance might be what we expect it to be; for example the MIME type could be empty here, in which case we'd try to find a matching plug-in based on the URL extension.
> 
> However, if the CSP is only used for *whitelisting* certain plug-ins, going by MIME type shouldn't be a problem: If a page specifies a certain MIME type, we refuse to load a plug-in with a different MIME type (to avoid confusion attacks; see AllowMimeTypeMismatch() in Chromium plugin_list.cc). 

If I understand you correctly, this would cover the case where a developer includes an explicit MIME type in the `object` or `embed` element. Is that correct? If so, I like it.

I wouldn't be at all averse to making an explicit declaration of the expected MIME type on the element itself mandatory in the CSP spec (similar to the mandatory `nonce` attribute), as that has nice security properties at a minimal cost to the developer.

If possible, I'd prefer not to hand off the MIME types to the embedder. The implementation seems cleaner conceptually if the CSP object retains responsibility for a thumbs up/down on resources it asserts control over. Delegating that responsibility feels wrong.
------- Comment #14 From 2012-07-23 17:21:53 PST -------
Ok.  I'm sold on coupling this with the strict "type" behavior.  Would you be willing to email the working group and edit the spec.  :-P
------- Comment #15 From 2012-07-23 17:22:28 PST -------
Note: There's already a mechanism in the HTML spec for requiring this strict checking.  We just need to tell the HTML spec to require it.
------- Comment #16 From 2012-07-23 18:28:20 PST -------
(In reply to comment #14)
> Ok.  I'm sold on coupling this with the strict "type" behavior.  Would you be willing to email the working group and edit the spec.  :-P

Sure. I'll take a look at this tonight, assuming that the tiny baby keeps me up till 2 again. :)
------- Comment #17 From 2012-07-23 18:31:42 PST -------
(In reply to comment #15)
> Note: There's already a mechanism in the HTML spec for requiring this strict checking.  We just need to tell the HTML spec to require it.

You're talking about 'typemustmatch', right? (http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#attr-object-typemustmatch)
------- Comment #18 From 2012-07-24 10:05:33 PST -------
(In reply to comment #17)
> (In reply to comment #15)
> > Note: There's already a mechanism in the HTML spec for requiring this strict checking.  We just need to tell the HTML spec to require it.
> 
> You're talking about 'typemustmatch', right? (http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#attr-object-typemustmatch)

Yes, exactly.
------- Comment #19 From 2012-07-27 02:07:24 PST -------
(From update of attachment 153676 [details])
This patch isn't quite right, as discussed.
------- Comment #20 From 2012-08-07 06:53:49 PST -------
Created an attachment (id=156932) [details]
Patch
------- Comment #21 From 2012-08-07 06:56:58 PST -------
The attached patch implements the check against the declared type of the plugin, as we discussed here. I'll drop a line to the public-webappsec@ group now to weigh out the impacts of that suggestion.

Two caveats:

1. Since WebKit doesn't seem to natively support the `typemustmatch` mechanism, I'm working around it by doing the comparison inside of CSP. It's not perfect, but I hope it's good enough for the moment.

2. I'm hardcoding the MIME type of comparisons for the `applet` element to `application/x-java-applet`, which seemed reasonable.
------- Comment #22 From 2012-08-07 07:16:44 PST -------
(In reply to comment #21)
> The attached patch implements the check against the declared type of the plugin, as we discussed here. I'll drop a line to the public-webappsec@ group now to weigh out the impacts of that suggestion.

http://lists.w3.org/Archives/Public/public-webappsec/2012Aug/0000.html

I think it's worth reviewing this patch now so that I can clean up any defects, but (assuming there's debate) I'd like to wait to land it until that discussion reaches consensus.
------- Comment #23 From 2012-08-07 11:39:47 PST -------
(From update of attachment 156932 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=156932&action=review

> Source/WebCore/loader/SubframeLoader.cpp:133
> +        if (!document()->contentSecurityPolicy()->allowObjectFromSource(url)
> +            || !m_frame->document()->contentSecurityPolicy()->allowPluginType(mimeType, declaredMimeType, url)) {

Why do these two lines use a different way to find the document?

> Source/WebCore/loader/SubframeLoader.cpp:301
> +            || !element->document()->contentSecurityPolicy()->allowPluginType("application/x-java-applet", "application/x-java-applet", codeBaseURL))

Can you make this string into a named constant?

> Source/WebCore/page/ContentSecurityPolicy.cpp:600
> +    bool checkPluginType(const String& type, const String& typeAttr) const;

typeAttr -> typeAttribute (please use complete words in variable names)

> Source/WebCore/page/ContentSecurityPolicy.cpp:628
> +    HashSet<String> m_pluginTypes;
> +    String m_pluginTypesDirective;

Should we create a PluginTypesDirective object to hold these pieces of state?

> Source/WebCore/page/ContentSecurityPolicy.cpp:729
> +        message = makeString("`plugin-types` Content Security Policy directive is empty; all plugins will be blocked.\n");

You don't need to call makeString if you've just got a single string.  You can just use the assignment operator.

Also, please don't use ` in console messages.  We prefer '.

> Source/WebCore/page/ContentSecurityPolicy.cpp:1088
> +    // `plugin-types ____;` OR `plugin-types;`

No ` please.

> Source/WebCore/page/ContentSecurityPolicy.cpp:1089
> +    if (value.isNull()) {

isNull -> isEmpty ?

> Source/WebCore/page/ContentSecurityPolicy.cpp:1297
> +template<bool (CSPDirectiveList::*allowed)(const String&, const String&, const KURL&, ContentSecurityPolicy::ReportingStatus) const>

I'm not sure its worth having this whole template given that we're probably only going to have one function with this signature.
------- Comment #24 From 2012-08-07 12:45:15 PST -------
(In reply to comment #23)
> (From update of attachment 156932 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156932&action=review
> 
> > Source/WebCore/loader/SubframeLoader.cpp:133
> > +        if (!document()->contentSecurityPolicy()->allowObjectFromSource(url)
> > +            || !m_frame->document()->contentSecurityPolicy()->allowPluginType(mimeType, declaredMimeType, url)) {
> 
> Why do these two lines use a different way to find the document?

'document()' is the same as 'm_frame->document()'; we should be using the former. Not sure why I didn't here...

> 
> > Source/WebCore/loader/SubframeLoader.cpp:301
> > +            || !element->document()->contentSecurityPolicy()->allowPluginType("application/x-java-applet", "application/x-java-applet", codeBaseURL))
> 
> Can you make this string into a named constant?

Sure thing. DEFINE_STATIC_LOCAL, or something publicly accessible on some relevant class?

> 
> > Source/WebCore/page/ContentSecurityPolicy.cpp:600
> > +    bool checkPluginType(const String& type, const String& typeAttr) const;
> 
> typeAttr -> typeAttribute (please use complete words in variable names)

Done.

> > Source/WebCore/page/ContentSecurityPolicy.cpp:628
> > +    HashSet<String> m_pluginTypes;
> > +    String m_pluginTypesDirective;
> 
> Should we create a PluginTypesDirective object to hold these pieces of state?

We certainly could. Should we do the same for script-nonce?

> > Source/WebCore/page/ContentSecurityPolicy.cpp:729
> > +        message = makeString("`plugin-types` Content Security Policy directive is empty; all plugins will be blocked.\n");
> 
> You don't need to call makeString if you've just got a single string.  You can just use the assignment operator.

Done.

> Also, please don't use ` in console messages.  We prefer '.

I'm trying to wean myself off them in WebKit. Muscle memory. :(

> > Source/WebCore/page/ContentSecurityPolicy.cpp:1089
> > +    if (value.isNull()) {
> 
> isNull -> isEmpty ?

Done.

> > Source/WebCore/page/ContentSecurityPolicy.cpp:1297
> > +template<bool (CSPDirectiveList::*allowed)(const String&, const String&, const KURL&, ContentSecurityPolicy::ReportingStatus) const>
> 
> I'm not sure its worth having this whole template given that we're probably only going to have one function with this signature.

Easy enough. I'd done it for consistency, but you're probably right.
------- Comment #25 From 2012-08-07 12:58:30 PST -------
(In reply to comment #24)
> > > Source/WebCore/page/ContentSecurityPolicy.cpp:628
> > > +    HashSet<String> m_pluginTypes;
> > > +    String m_pluginTypesDirective;
> > 
> > Should we create a PluginTypesDirective object to hold these pieces of state?
> 
> We certainly could. Should we do the same for script-nonce?

Actually, now that I look at the code, I'd prefer not to do this until something like https://bugs.webkit.org/show_bug.cgi?id=93205 is in. If I pull the parsing out into a subclass of CSPDirective, then I won't be able to log many of the errors that are currently being generated (as only CSPDirectiveList can directly log errors, etc.).
------- Comment #26 From 2012-08-07 13:21:47 PST -------
> > > Source/WebCore/loader/SubframeLoader.cpp:301
> > > +            || !element->document()->contentSecurityPolicy()->allowPluginType("application/x-java-applet", "application/x-java-applet", codeBaseURL))
> > 
> > Can you make this string into a named constant?
> 
> Sure thing. DEFINE_STATIC_LOCAL, or something publicly accessible on some relevant class?

I would just use a const char[] rather than a static.

> > > > Source/WebCore/page/ContentSecurityPolicy.cpp:628
> > > > +    HashSet<String> m_pluginTypes;
> > > > +    String m_pluginTypesDirective;
> > > 
> > > Should we create a PluginTypesDirective object to hold these pieces of state?
> > 
> > We certainly could. Should we do the same for script-nonce?
> 
> Actually, now that I look at the code, I'd prefer not to do this until something like https://bugs.webkit.org/show_bug.cgi?id=93205 is in. If I pull the parsing out into a subclass of CSPDirective, then I won't be able to log many of the errors that are currently being generated (as only CSPDirectiveList can directly log errors, etc.).

Ok.
------- Comment #27 From 2012-08-07 14:15:52 PST -------
Created an attachment (id=157001) [details]
Adam's feedback.
------- Comment #28 From 2012-08-07 14:57:22 PST -------
(From update of attachment 157001 [details])
Attachment 157001 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13456233

New failing tests:
http/tests/security/contentSecurityPolicy/1.1/plugintypes-url-02.html
------- Comment #29 From 2012-08-07 14:57:25 PST -------
Created an attachment (id=157015) [details]
Archive of layout-test-results from gce-cr-linux-06

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
------- Comment #30 From 2012-08-07 15:05:20 PST -------
Created an attachment (id=157017) [details]
Reset test result.
------- Comment #31 From 2012-08-13 11:43:30 PST -------
Created an attachment (id=158062) [details]
Rebased onto ToT.
------- Comment #32 From 2012-08-13 11:44:58 PST -------
(In reply to comment #22)
> (In reply to comment #21)
> > The attached patch implements the check against the declared type of the plugin, as we discussed here. I'll drop a line to the public-webappsec@ group now to weigh out the impacts of that suggestion.
> 
> http://lists.w3.org/Archives/Public/public-webappsec/2012Aug/0000.html
> 
> I think it's worth reviewing this patch now so that I can clean up any defects, but (assuming there's debate) I'd like to wait to land it until that discussion reaches consensus.

New patch rebases this onto ToT.

If there continues to be no feedback through tomorrow, I'd like to CQ this. We can always argue about/change the implementation after the fact. :)
------- Comment #33 From 2012-08-13 11:54:02 PST -------
> If there continues to be no feedback through tomorrow, I'd like to CQ this. We can always argue about/change the implementation after the fact. :)

I think it's fine to land this whenever you like.  We should just be careful not to ship CSP_NEXT until we'll happy with the progress in the working group.

On that topic, we should probably make sure CSP_NEXT is disabled on the beta channel.
------- Comment #34 From 2012-08-13 11:58:46 PST -------
(In reply to comment #33)
> > If there continues to be no feedback through tomorrow, I'd like to CQ this. We can always argue about/change the implementation after the fact. :)
> 
> I think it's fine to land this whenever you like.  We should just be careful not to ship CSP_NEXT until we'll happy with the progress in the working group.
> 
> On that topic, we should probably make sure CSP_NEXT is disabled on the beta channel.

Kerz@ is release manager for Chrome 22. Want me to ping him?
------- Comment #35 From 2012-08-13 12:06:57 PST -------
> Kerz@ is release manager for Chrome 22. Want me to ping him?

Yeah.
------- Comment #36 From 2012-08-14 01:56:24 PST -------
(In reply to comment #35)
> > Kerz@ is release manager for Chrome 22. Want me to ping him?
> 
> Yeah.

Covered in http://crbug.com/142415

In the meantime, I'll find someone to CQ this. :)
------- Comment #37 From 2012-08-14 02:17:07 PST -------
(From update of attachment 158062 [details])
Clearing flags on attachment: 158062

Committed r125531: <http://trac.webkit.org/changeset/125531>
------- Comment #38 From 2012-08-14 02:17:14 PST -------
All reviewed patches have been landed.  Closing bug.