[FFmpeg-devel] [PATCH 2/2] avformat: add protocol_whitelist

[Nicolas George]

L'octidi 8 pluviôse, an CCXXIV, Michael Niedermayer a écrit :
> a fix, good or not that isnt implemented is useless
> 
> I am not really attracted to the design you suggest, to me its worse
> in several ways but above all its alot more work. So I dont volunteer
> 
> to elaborate and avoid misuderstanding
> 
> redesigning Codec, Format and Protocol registration does make sense
> iam not against that at all
> 
> but iam not motivated to implement that, its alot of work and
> it feels alot more limited if used as the only way to whitelist
> things instead of a seperate whitelist.

I will not be annoying and oppose the new version of the patch, as it
address my most prominent concern, i.e. it does not add too much change to
the public API.

But I really really wish you would not rush implementing patches like that
public API and design changes should be discussed, and discussed before one
of the most useful developers wasted time on a 60+k patch.

And I really mean wasted: your patch fixes only one specific case of
information leak related to playlists. We knew for a long time that there
would be, and there will be more. Fixing the next one will require another
60+k patch, and the next one and the next one. If we had taken the time to
discuss a proper fix, we would probably have been able to fix most of them
in a single 65k patch, and there is even a chance we would have found a way
of doing so on 20k.

Well, since we are sure there are more information leak issues in this area
of code, we can still discuss a proper fix before the next one becomes
public.

A few unsorted thoughts about it:

- Restrictions can not be limited to just the protocol, the host name, path,
  etc., are important.

- Applications need to be able to provide complex restrictions => a
  callback is what we need.

- Restrictions need to be pervasive, there must not be a code path that
  discards them => attach them to the same data structures that allow
  accessing dangerous resources: protocol implementations.

- Global state is annoying for a library => take this occasion to get rid of
  global state too.

- If memory allocation is involved, failure is possible, checking is
  annoying => design the code to be harmless and return immediately if the
  structure are not correctly initialized. That way, only a final check is
  needed.

- Problem: the data structure belongs to what library? lavc? lavf? => merge
  the libraries so we do not have to worry about internal ABI compatibility.

Basically, the vague idea I have in mind would be a pervasive AVLibrary
structure containing all the global state in the FFmpeg libraries: list of
codecs, list of formats, list of protocols, lock manager,
dynamically-computed tables, etc. And, of course, security policy. Make it
ref-counted, make it inheritable (avlib2 = avlib1 with an extra security
restriction on top of it).

As for the specific case of avoiding information leak, I suspect we need
somehow to attach to any block of data (AVPacket, URL string, possibly
option string) a structure describing where it came from and how trustworthy
it is. But I have only a vague idea on how to do that.

> For example the string could easily be extended to support
> specific chains of protocols like "https->tls->tcp" so that the user
> is not allowed to directly pass tcp/tls urls but only https and https
> itself then is only allowed to access tls, ...

Please stop using strings as data structures. It is only convenient for the
command-line use, and not even for non-trivial cases. For API uses, real
data structures are needed. (Of course, a function to init the data
structure from a string is still welcome.)

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160128/2253f455/attachment.sig>