[FFmpeg-devel] [PATCH 10/18] avformat/hls: add support for byte-ranged segments

[Anssi Hannula]

01.01.2014 03:42, Michael Niedermayer kirjoitti:
> On Wed, Jan 01, 2014 at 03:28:58AM +0200, Anssi Hannula wrote:
>> 01.01.2014 01:34, Michael Niedermayer kirjoitti:
>>> On Wed, Jan 01, 2014 at 12:32:10AM +0200, Anssi Hannula wrote:
>>>> 31.12.2013 10:20, Clément Bœsch kirjoitti:
>>>>> On Tue, Dec 31, 2013 at 09:28:05AM +0200, Anssi Hannula wrote:
>>>>>> 31.12.2013 08:30, Clément Bœsch kirjoitti:
>>>>>>> On Tue, Dec 31, 2013 at 05:13:03AM +0200, Anssi Hannula wrote:
>>>>>>>> 31.12.2013 05:05, Michael Niedermayer kirjoitti:
>>>>>>>>> On Mon, Dec 30, 2013 at 01:14:24PM +0200, Anssi Hannula wrote:
>>>>>>>>>> Add support for EXT-X-BYTERANGE added in HLS protocol v4.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Anssi Hannula <anssi.hannula at iki.fi>
>>>>>>>>>> ---
>>>>>>>>>>  libavformat/hls.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>>>>>>>>>  1 file changed, 61 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>> [...]
>>>>>>>>>> @@ -635,8 +671,22 @@ static int open_input(HLSContext *c, struct playlist *pls)
>>>>>>>>>>      else
>>>>>>>>>>        ret = AVERROR(ENOSYS);
>>>>>>>>>>  
>>>>>>>>>> +    /* Seek to the requested position. If this was a HTTP request,
>>>>>>>>>> +     * the offset should already be there. */
>>>>>>>>>> +    if (ret == 0) {
>>>>>>>>>> +        int seekret = ffurl_seek(pls->input, seg->url_offset, SEEK_SET);
>>>>>>>>>> +        if (seekret < 0) {
>>>>>>>>>> +            av_log(pls->parent, AV_LOG_ERROR, "Unable to seek to offset %"PRId64" of HLS segment '%s'\n", seg->url_offset, seg->url);
>>>>>>>>>> +            ret = seekret;
>>>>>>>>>> +            ffurl_close(pls->input);
>>>>>>>>>> +            pls->input = NULL;
>>>>>>>>>> +        }
>>>>>>>>>> +    }
>>>>>>>>>
>>>>>>>>> maybe i misunderstand something but why do you need to seek if the
>>>>>>>>> offset was already provided to the protocol ?
>>>>>>>>
>>>>>>>> Well, in case the protocol is not HTTP. Of course that is indeed a bit
>>>>>>>> far-fetched as it is "HTTP Live Streaming" ;)
>>>>>>>>
>>>>>>>> This was useful for local testing without needing a HTTP server, though.
>>>>>>>> I'm not too heavily against removing it, but it doesn't really hurt
>>>>>>>> either so I kept it...
>>>>>>>
>>>>>>> Does that mean a remote server can make you play local files?
>>>>>>
>>>>>> Apparently yes, hls uses just ff_make_absolute_url() so the playlist can
>>>>>> just contain "file://local/file.ts".
>>>>>>
>>>>>
>>>>> I can't think of a way to communicate back information, but typically this
>>>>> means the remote server can make you open sensitive files client side. A
>>>>> blind attack might allow to guess the existence of files. If a sensible
>>>>> file is somehow probed as a random playlist supported by FFmpeg, some url
>>>>> in it might be fetched leaking some information.
>>>>>
>>>>> Of course this is just theory, but the point is, I'd better have this code
>>>>> surrounded by #if DEBUG, or enabled through a disabled by default option,
>>>>> such as -allow_local_file or something.
>>>>
>>>> Right, it is not a normal use case so we could restrict it to http/https
>>>> by default and add "allow_any_protocol" (any proto allowed) or
>>>> "allow_local_file" (http(s): and file: protos). Both is overkill, not
>>>> sure which one to add yet.
>>>>
>>>> However, personally I'm much more concerned about doing
>>>> $ someplayer random_file.mp4
>>>> when random_file.mp4 is actually a HLS playlist file and FFmpeg starts
>>>> making HTTP connections to URLs in the file, which I would not really
>>>> expect...
>>>> I'm not sure what we could do about that, though... Maybe only have
>>>> .probe() succeed if filename extension is .m3u(8)?
>>>> The HLS standard specifies, though, that either the playlist extension
>>>> should be .m3u(8) and/_or_ HTTP Content-Type is one of several strings.
>>>> So theoretically playlist files delivered by HTTP could have random names...
>>>>
>>>> Or WDYT?
>>>
>>> one could enforce that the playlist protocol matches the
>>> playlist entries protocolls
>>> like a local playlist can access files while a http playlist is
>>> limited to http
>>
>> Sure, that would indeed seem a pretty sensible way in theory, but how
>> would we determine the playlist protocol, from AVFormatContext.filename?
>> And what if that is empty or just contains filename without protocol?
> 
> if its empty then the av_class of the AVIOContext could be looked at
> to see if it allows determining the protocol
> if that fails too then i guess the only obvious option left is to
> print a warning and continue

What about the case of AVFormatContext.filename containing simply
"random_file.mp4" (no proto or path), which I guess is common for local
files, but which I guess is also common for remote files accessed by
custom AVIOContext (since the parameter name is "filename", not "path")?

Assuming it is local would break HLS in e.g. XBMC and others that use
custom AVIOContext for HTTP with filename being just filename part.

Assuming it is remote would make the check quite ineffective (unless
maybe coupled with file extension check?).


>>
>>> stricter "same origin" checks like same server or domain could be
>>> tried too, i dont know if these are possible with real world hls
>>> though or if this would break some streams
-- 
Anssi Hannula