Bug 33539 - [GTK] handle media redirections
Summary: [GTK] handle media redirections
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-12 11:01 PST by Philippe Normand
Modified: 2010-02-01 05:41 PST (History)
2 users (show)

See Also:


Attachments
media redirections support (7.51 KB, patch)
2010-01-13 07:41 PST, Philippe Normand
oliver: review-
Details | Formatted Diff | Diff
media redirections support (7.97 KB, patch)
2010-01-14 01:31 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
media redirections support (9.91 KB, patch)
2010-01-19 10:13 PST, Philippe Normand
oliver: review-
Details | Formatted Diff | Diff
media redirections support (9.79 KB, patch)
2010-01-20 00:04 PST, Philippe Normand
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2010-01-12 11:01:29 PST
With current player http://movies.apple.com/movies/us/apple/ipoditunes/2007/touch/ads/apple_ipodtouch_touch_r640-9cie.mov fails to load. We need to react on redirect messages sent by qtdemux.
Comment 1 Philippe Normand 2010-01-13 07:41:59 PST
Created attachment 46459 [details]
media redirections support
Comment 2 Oliver Hunt 2010-01-13 22:55:31 PST
Comment on attachment 46459 [details]
media redirections support

For security reasons you need to disallow cross-origin redirects from media, otherwise canvas origin security checks may be bypassed
Comment 3 Philippe Normand 2010-01-14 01:31:26 PST
Created attachment 46547 [details]
media redirections support
Comment 4 Oliver Hunt 2010-01-14 01:51:10 PST
Comment on attachment 46547 [details]
media redirections support

Okay this origin check stuff looks sane to me, but i'll need to defer to a gtk/gstreamer type person for review proper sorry :(
Comment 5 Sebastian Dröge (slomo) 2010-01-19 03:00:19 PST
Looks good in general, some notes:
- In MediaPlayerPrivate::loadNextLocation(), new-location could actually be a absolute URI. Simple to check. Also use gst_value_get_structure() as this does type checks.
- The redirect message might have a "new-location" field but not "locations" field. The "new-location" field would be of type string. Both can exist too, in the case "new-location" would have preference. Should be handled
- When changing the location of playbin2, only reset it to READY, not NULL. That should be done at other places too. Going back to NULL and then back PLAYING takes a lot of time.
Comment 6 Philippe Normand 2010-01-19 05:38:23 PST
(In reply to comment #4)
> (From update of attachment 46547 [details])
> Okay this origin check stuff looks sane to me, but i'll need to defer to a
> gtk/gstreamer type person for review proper sorry :(

Oliver I am not sure I did the right origin check:

- if I do GtkLauncher <uri-of-the-mov>, it works fine
- but if I use the video-player.html the origin check fails because the SecurityOrigin of the Document is for the file:// uri  and the mov uri is an http uri.

In the second case it should work too, no?
Comment 7 Philippe Normand 2010-01-19 10:13:06 PST
Created attachment 46923 [details]
media redirections support
Comment 8 Oliver Hunt 2010-01-19 12:33:18 PST
Comment on attachment 46923 [details]
media redirections support

Ah right, i see what you're doing, you're doing an origin check of the document versus the media.  That's not what we want to do.  What we want to do is make any redirect in the media that goes to a different origin from the current origin be forbidden

Eg. if i have

http://example.com/foo.mov and that media file contains a redirect to http://example.com/foo2.mov that's okay, if i instead it redirects to http://evil.com/foo.mov then the media should be blocked.  This has nothing to do with the containing document.
Comment 9 Philippe Normand 2010-01-20 00:04:07 PST
Created attachment 46987 [details]
media redirections support
Comment 10 Oliver Hunt 2010-01-20 11:01:52 PST
Comment on attachment 46987 [details]
media redirections support

This looks good, but you should see if you can make a testcase tp ensure an unsafe redirect is blocked, and a safe one is not. r-'ing simply due to the absence of testcases
Comment 11 Oliver Hunt 2010-01-21 00:58:14 PST
Comment on attachment 46987 [details]
media redirections support

r=me, as eric carlson did not like the idea of testing for the cross origin blocking behaviour.
Comment 12 Philippe Normand 2010-01-21 01:44:41 PST
Landed as r53617. Thanks!
Comment 13 Philippe Normand 2010-02-01 05:41:19 PST
(In reply to comment #8)
> (From update of attachment 46923 [details])
> Ah right, i see what you're doing, you're doing an origin check of the document
> versus the media.  That's not what we want to do.  What we want to do is make
> any redirect in the media that goes to a different origin from the current
> origin be forbidden
> 
> Eg. if i have
> 
> http://example.com/foo.mov and that media file contains a redirect to
> http://example.com/foo2.mov that's okay, if i instead it redirects to
> http://evil.com/foo.mov then the media should be blocked.  This has nothing to
> do with the containing document.

Hi Oliver,

Sorry to come again on this origin check stuff but I found one example where the redirect is done to a totally different server.

http://stream.qtv.apple.com/events/jan/1001q3f8hhr/1001908r5ft6dswz_1_350_ref.mov

redirects to:

rtsp://a2047.v1412b.c1412.g.vq.akamaistream.net/5/2047/1412/1_h264_350/1a1a1ae555c531960166df4dbc3095c327960d7be756b71b49aa1576e344addb3ead1a497aaedf11/1001908r5ft6dswz_1_350.mov

So I am not sure anymore we should do that origin check, what do you think?