[vlc-devel] [PATCH 2/2] playlist/fetcher: do not ignore metadata scope when downloading art

[Filip Roséen]

Hi Pierre,

On 2016-12-11 05:28, Pierre wrote:

> > I cannot see any other way to interpret the flag, and the intent has
> > always been to block metadata lookup and access for the desribed set
> > of entities (when the flag is enabled).
> 
> I disagree, the intent has been to prevent privacy leaks through
> metadata lookup and access. Metadata lookup and access that don't
> cause privacy leaks (e.g. getting the associated art from the same
> video website that is already getting queried) are a grey area of
> interpretation that you're now denying.

 - Exactly what is it that I am *"now denying"*?

I honestly cannot read `--no-network-metadata-access` and see your
point, if it was meant to block *metadata lookups*; why is it not
named `--no-network-metadata-lookup`?

> There's at least me considering the feature that way, I sure as hell
> don't want my local media to start leaking queries to the internet,
> but I do appreciate getting the associated art from youtube video or
> soundcloud tracks I'm playing.

> If you can't see how I can think that, then you might as well call
> me stupid.

I would never resort to such replies, not do I wish anyone to do the
same towards anyone on `vlc-devel`. Hopefully those that do will stop
soon enough.

> Do I need to download art from within lua scripts and pass it as an
> inline data URL to get my point across?
> 
> > The `m3u`-example is simply an example, the same would happen for
> > entities that has a remote resource embedded directly within the
> > media; the reason I choose `m3u` to demonstrate the issue is that it
> > is easily edited, and easily reasoned about.
> 
> Granted, but that's also part of the grey area; although metadata URLs
> embedded in video file containers would rather be on the side of what
> you don't want to automatically lookup.
> 
> However this is starting to be a different issue from privacy and
> querying metadata, it's an issue of blocking untrusted input and attack
> vectors. And I don't think that this was the concern in mind when the
> feature was designed, so one could say that you're diverting the real
> purpose of a loosely related feature to fix a proof-of-concept exploit.
> It also seems that the patch is wrongly coupling the file caching
> technical optimization concern with the privacy concern.

Could you elaborate this point?

I do not want to reply to something which I am not sure I completely
understand (in terms of what is being expressed), as that will
probably just make us walk down a discussion path that might be
irrelevant to the underlying point.

 
> file:// implies neither local, nor private, nor trusted. The attack
> vector you highlighted is still exploitable after your patch, whether
> --metadata-network-access is enabled or not.


> Meta lua scripts have an explicit privacy scope, and their output can be
> more or less trusted. The output of lua website demuxers can be trusted
> and have no privacy impact. I'm not aware of any protection in other
> demuxers from untrusted art URLs, nor from privacy leaks they might
> incur.

Demuxers generally do not download any metadata, that happens in the
affected code-path which the original patch modified.
 
As far as I know, we have no such demuxer.

> The playlist core correctly handles meta fetchers, but seems to
> provide only a poor, coupled handling of the other technical, trust and
> privacy concerns. I think this patch is a misaimed attempt at improving
> things.

No, it is not a `100%` fix related to the privacy concern; but as I
think I expressed earlier, or at least on `#videolan @ freenode`; my
opinion is that it narrows the concern to a more acceptable level.

Related to "correctly handling meta fetcher", that is only somewhat
true; I will try to create tickets for the issues I found during next
week.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161211/487029ea/attachment.html>