<video> and <audio> elements do not obey Content Security Policy on redirect

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Daniel Bates]

The HTML audio and video element should obey the Content Security Policy of the page.

[Daniel Bates]

<rdar://problem/10234844>

[Daniel Bates]

(In reply to comment #0)
> The HTML audio and video element should obey the Content Security Policy of
> the page.

I meant to write:

HTML audio and video elements should obey the Content Security Policy of the page on redirect.

[Daniel Bates]

Created attachment 274130 [details]
Patch and Layout Tests

[Daniel Bates]

Created attachment 274132 [details]
Patch and Layout Tests

[Build Bot]

Comment on attachment 274132 [details]
Patch and Layout Tests

Attachment 274132 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/985318

New failing tests:
http/tests/security/contentSecurityPolicy/svg-font-redirect-blocked.html

[Build Bot]

Created attachment 274139 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 274132 [details]
Patch and Layout Tests

Attachment 274132 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/985406

New failing tests:
http/tests/security/contentSecurityPolicy/svg-font-redirect-blocked.html

[Build Bot]

Created attachment 274147 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Daniel Bates]

(In reply to comment #5)
> Comment on attachment 274132 [details]
> Patch and Layout Tests
> 
> Attachment 274132 [details] did not pass mac-ews (mac):
> Output: http://webkit-queues.webkit.org/results/985318
> 
> New failing tests:
> http/tests/security/contentSecurityPolicy/svg-font-redirect-blocked.html

The test loads the font with URL <http://127.0.0.1:8000/resources/redirect.php?code=307&url=http%3A%2F%2Flocalhost%3A8000/security/contentSecurityPolicy/resources/ABCFont.svg#ABCFont>. Notice that this URL contains a fragment, #ABCFont. This test will fail on OS X Yosemite because it is affected by CFNetwork bug <rdar://problem/3225447> - fragments are not preserved on redirection. For the purposes of this test bug it is sufficient to remove the fragment, #ABCFont, from the URL so as to make this test pass on both OS X Yosemite and OS X El Capitan.

[Daniel Bates]

Created attachment 274163 [details]
Patch and Layout Tests

Remove fragment from URL used in test http/tests/security/contentSecurityPolicy/svg-font-redirect-blocked.html to avoid <rdar://problem/3225447>. Also rebased patch.

[Alex Christensen]

Comment on attachment 274163 [details]
Patch and Layout Tests

View in context: https://bugs.webkit.org/attachment.cgi?id=274163&action=review

r=me

> Source/WebCore/loader/MediaResourceLoader.cpp:59
> +    // FIXME: Skip Content Security Policy check if the element that inititated this request
> +    // is in a user-agent shadow tree. See <https://bugs.webkit.org/show_bug.cgi?id=155505>.

Why do we want to do this?  Luckily this patch doesn't deal with user-agent shadow trees, and that's the part I'm not sure about.

> Source/WebCore/loader/ResourceLoadInfo.cpp:58
> +    case CachedResource::MediaResource:

can we get rid of the default in this switch statement?

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:458
> +    case CachedResource::MediaResource:

This is the functional change, right?  We're not breaking here.

[Alex Christensen]

Comment on attachment 274163 [details]
Patch and Layout Tests

View in context: https://bugs.webkit.org/attachment.cgi?id=274163&action=review

> Source/WebCore/ChangeLog:28
> +               http/tests/security/contentSecurityPolicy/video-redirect-blocked.html

Are there tests for xsl stylesheets, svg documents, LinkPrefetch and LinkSubresource?

[Daniel Bates]

(In reply to comment #11)
> [...]
> 
> > Source/WebCore/loader/MediaResourceLoader.cpp:59
> > +    // FIXME: Skip Content Security Policy check if the element that inititated this request
> > +    // is in a user-agent shadow tree. See <https://bugs.webkit.org/show_bug.cgi?id=155505>.
> 
> Why do we want to do this?

See <rdar://problem/25169452> for more details.

> > Source/WebCore/loader/ResourceLoadInfo.cpp:58
> > +    case CachedResource::MediaResource:
> 
> can we get rid of the default in this switch statement?
> 

Will remove default case statement and add the following to the end of the switch block:

#if ENABLE(LINK_PREFETCH)
    case CachedResource::LinkPrefetch:
    case CachedResource::LinkSubresource:
        ASSERT_NOT_REACHED();
#endif

> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:458
> > +    case CachedResource::MediaResource:
> 
> This is the functional change, right?  We're not breaking here.

Yes, this is the meaningful part of the patch. We explicitly want to fall through to the same logic as we do for a <track> and check the Content Security Policy if the URL of the media resource is allowed to be requested.

[Daniel Bates]

(In reply to comment #12)
> Are there tests for xsl stylesheets, svg documents, LinkPrefetch and
> LinkSubresource?

Will add tests for an XSL stylesheet and an SVG document embedded using an HTML image element before landing. I am not adding tests for cached resources of types CachedResource::LinkPrefetch and CachedResource::LinkSubresource because such tests are not meaningful given that the Content Security Policy does not apply to prefetched resources as we do not know the purpose of the prefetch resource at the time it is requested to be prefetched and hence do not know which CSP directive to enforce (if any).

[Daniel Bates]

Committed r198292: <http://trac.webkit.org/changeset/198292>