diff mbox

[FFmpeg-devel] avformat/utils: Don't parse encrypted packets.

Message ID 20180828175843.15296-1-modmaker@google.com
State Superseded
Headers show

Commit Message

Jacob Trimble Aug. 28, 2018, 5:58 p.m. UTC
If a packet is full-sample encrypted, then packet data can't be parsed
without decrypting it.  So this skips the packet parsing for those
packets.  If the packet has sub-sample encryption, it is assumed that
the headers are in the clear and the parser will only need that info.

Signed-off-by: Jacob Trimble <modmaker@google.com>
---
 libavformat/utils.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer Aug. 29, 2018, 10:07 p.m. UTC | #1
On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> If a packet is full-sample encrypted, then packet data can't be parsed
> without decrypting it.  So this skips the packet parsing for those
> packets.  If the packet has sub-sample encryption, it is assumed that
> the headers are in the clear and the parser will only need that info.
> 
> Signed-off-by: Jacob Trimble <modmaker@google.com>
> ---
>  libavformat/utils.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)

Hmm, please correct me if iam wrong but IIUC
1. The demuxer has set need_parsing, indicating that it _requires_ a parser
2. Parsing cannot be done before decrypting

If all this and the set values are correct, logically, the fix would be
to decrypt the packet before the parser not to skip the parser.

Am i missing something ?

thx

[...]
James Almer Aug. 29, 2018, 10:14 p.m. UTC | #2
On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
>> If a packet is full-sample encrypted, then packet data can't be parsed
>> without decrypting it.  So this skips the packet parsing for those
>> packets.  If the packet has sub-sample encryption, it is assumed that
>> the headers are in the clear and the parser will only need that info.
>>
>> Signed-off-by: Jacob Trimble <modmaker@google.com>
>> ---
>>  libavformat/utils.c | 21 ++++++++++++++++++++-
>>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> Hmm, please correct me if iam wrong but IIUC
> 1. The demuxer has set need_parsing, indicating that it _requires_ a parser
> 2. Parsing cannot be done before decrypting
> 
> If all this and the set values are correct, logically, the fix would be
> to decrypt the packet before the parser not to skip the parser.

And if that can't be done, then the demuxer could perhaps set
st->need_parsing to 0 and skip parsing altogether?

> 
> Am i missing something ?
> 
> thx
> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Jacob Trimble Aug. 29, 2018, 10:30 p.m. UTC | #3
On Wed, Aug 29, 2018 at 3:20 PM James Almer <jamrial@gmail.com> wrote:
>
> On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> > On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> >> If a packet is full-sample encrypted, then packet data can't be parsed
> >> without decrypting it.  So this skips the packet parsing for those
> >> packets.  If the packet has sub-sample encryption, it is assumed that
> >> the headers are in the clear and the parser will only need that info.
> >>
> >> Signed-off-by: Jacob Trimble <modmaker@google.com>
> >> ---
> >>  libavformat/utils.c | 21 ++++++++++++++++++++-
> >>  1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > Hmm, please correct me if iam wrong but IIUC
> > 1. The demuxer has set need_parsing, indicating that it _requires_ a parser

There isn't a flag for "try parsing and ignore errors".  So my choice
(from an app) is to either require parsing or don't do parsing at all
(which can result in incorrect timestamps).

> > 2. Parsing cannot be done before decrypting
> >
> > If all this and the set values are correct, logically, the fix would be
> > to decrypt the packet before the parser not to skip the parser.

There are cases where the packet can't be decrypted before getting to
this point.  This point is between the demuxer creating the packet and
giving to the app.  So the only way to decrypt the frame (as of now)
is for the demuxer to do this.  There is no way for the app to handle
the decryption before this point.

From the app's perspective, I would expect the packet to remain
encrypted for as long as possible.  My app will store the packet for a
while, then decrypt it right before passing it to the decoder and
drawing the frame.  So requiring the packet to be decrypted at this
point is not ideal.

>
> And if that can't be done, then the demuxer could perhaps set
> st->need_parsing to 0 and skip parsing altogether?

I would want to still have parsing if possible. For example, a lot of
content has a clear lead where the first few seconds are clear.  So
they could be used to adjust the starting timestamps (and whatever
parsing needs) and the encrypted content later can be deduced based on
that.

>
> >
> > Am i missing something ?
> >
> > thx
> >
> > [...]
> >
> >
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Michael Niedermayer Aug. 29, 2018, 11:37 p.m. UTC | #4
On Wed, Aug 29, 2018 at 03:30:39PM -0700, Jacob Trimble wrote:
> On Wed, Aug 29, 2018 at 3:20 PM James Almer <jamrial@gmail.com> wrote:
> >
> > On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> > > On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> > >> If a packet is full-sample encrypted, then packet data can't be parsed
> > >> without decrypting it.  So this skips the packet parsing for those
> > >> packets.  If the packet has sub-sample encryption, it is assumed that
> > >> the headers are in the clear and the parser will only need that info.
> > >>
> > >> Signed-off-by: Jacob Trimble <modmaker@google.com>
> > >> ---
> > >>  libavformat/utils.c | 21 ++++++++++++++++++++-
> > >>  1 file changed, 20 insertions(+), 1 deletion(-)
> > >
> > > Hmm, please correct me if iam wrong but IIUC
> > > 1. The demuxer has set need_parsing, indicating that it _requires_ a parser
> 
> There isn't a flag for "try parsing and ignore errors".  So my choice
> (from an app) is to either require parsing or don't do parsing at all
> (which can result in incorrect timestamps).
> 
> > > 2. Parsing cannot be done before decrypting
> > >
> > > If all this and the set values are correct, logically, the fix would be
> > > to decrypt the packet before the parser not to skip the parser.
> 

> There are cases where the packet can't be decrypted before getting to
> this point.  

Can you elaborate on these cases ? I dont think its obvious what these
cases are, at least its not obvious to me what exactly you are thinking
of here. And i think its not helpfull if i guess what you mean and then
argue/comment on that because maybe you meant something entirely different ...


> This point is between the demuxer creating the packet and
> giving to the app.  So the only way to decrypt the frame (as of now)
> is for the demuxer to do this.  There is no way for the app to handle
> the decryption before this point.
> 

> From the app's perspective, I would expect the packet to remain
> encrypted for as long as possible.  

why ?


> My app will store the packet for a
> while, then decrypt it right before passing it to the decoder and
> drawing the frame.  So requiring the packet to be decrypted at this
> point is not ideal.
> 
> >
> > And if that can't be done, then the demuxer could perhaps set
> > st->need_parsing to 0 and skip parsing altogether?
> 
> I would want to still have parsing if possible. For example, a lot of
> content has a clear lead where the first few seconds are clear.  So
> they could be used to adjust the starting timestamps (and whatever
> parsing needs) and the encrypted content later can be deduced based on
> that.
> 
[...]
Jacob Trimble Aug. 30, 2018, 3:43 p.m. UTC | #5
On Wed, Aug 29, 2018 at 4:37 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Wed, Aug 29, 2018 at 03:30:39PM -0700, Jacob Trimble wrote:
> > On Wed, Aug 29, 2018 at 3:20 PM James Almer <jamrial@gmail.com> wrote:
> > >
> > > On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> > > > On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> > > >> If a packet is full-sample encrypted, then packet data can't be parsed
> > > >> without decrypting it.  So this skips the packet parsing for those
> > > >> packets.  If the packet has sub-sample encryption, it is assumed that
> > > >> the headers are in the clear and the parser will only need that info.
> > > >>
> > > >> Signed-off-by: Jacob Trimble <modmaker@google.com>
> > > >> ---
> > > >>  libavformat/utils.c | 21 ++++++++++++++++++++-
> > > >>  1 file changed, 20 insertions(+), 1 deletion(-)
> > > >
> > > > Hmm, please correct me if iam wrong but IIUC
> > > > 1. The demuxer has set need_parsing, indicating that it _requires_ a parser
> >
> > There isn't a flag for "try parsing and ignore errors".  So my choice
> > (from an app) is to either require parsing or don't do parsing at all
> > (which can result in incorrect timestamps).
> >
> > > > 2. Parsing cannot be done before decrypting
> > > >
> > > > If all this and the set values are correct, logically, the fix would be
> > > > to decrypt the packet before the parser not to skip the parser.
> >
>
> > There are cases where the packet can't be decrypted before getting to
> > this point.
>
> Can you elaborate on these cases ? I dont think its obvious what these
> cases are, at least its not obvious to me what exactly you are thinking
> of here. And i think its not helpfull if i guess what you mean and then
> argue/comment on that because maybe you meant something entirely different ...

Fair warning, I am working on a DRM solution.  I know many of you may
not agree with DRM, but it is a requirement imposed by content
creators.  FFmpeg doesn't have to support DRM itself, but you should
still consider its usages.

At that point in the code, the packet can only be decrypted by the
demuxer.  For example, in MP4 this can be done by passing the
-decryption_key parameter.  But that requires that FFmpeg handle the
decryption.  In my case, we are passing the packet to a CDM (content
decryption module) to handle the decryption and that might be a
hardware-secure decryptor.  In most DRM situations, we can't get the
raw key at all, all we can do is say "decrypt this block of memory".
So the only way to have the packet decrypted before this point would
be to introduce a callback to the app to decrypt the packet.

This is why I have been working on exposing the encryption info.  My
app (or more correctly the CDM) needs to handle the decryption and we
can't just give the key to libavformat so the demuxer can decrypt the
packet.

>
>
> > This point is between the demuxer creating the packet and
> > giving to the app.  So the only way to decrypt the frame (as of now)
> > is for the demuxer to do this.  There is no way for the app to handle
> > the decryption before this point.
> >
>
> > From the app's perspective, I would expect the packet to remain
> > encrypted for as long as possible.
>
> why ?

Because the point of encryption is to ensure that nefarious parties
don't get access to the data.  Even though the packet data is stored
in memory, it could still be retrieved by a malicious user.  Usually
for protected media, the frames are only decrypted when needed and in
some cases are decrypted in a secure CPU and isn't even accessible to
the app.  There are platforms that support what is called "direct
render" where the app gives the platform the encrypted packets and a
discrete hardware unit will decrypt the packet, decode it, and render
it directly; this happens on some smart TVs.  There are also cases
which require decrypt-decode where we give the platform an encrypted
packet and it will decrypt and decode the packet, ensuring the
decrypted packet data is never in main memory.  So there are cases
where we can't even see the encoded packet and decoding is handled by
the platform.

>
>
> > My app will store the packet for a
> > while, then decrypt it right before passing it to the decoder and
> > drawing the frame.  So requiring the packet to be decrypted at this
> > point is not ideal.
> >
> > >
> > > And if that can't be done, then the demuxer could perhaps set
> > > st->need_parsing to 0 and skip parsing altogether?
> >
> > I would want to still have parsing if possible. For example, a lot of
> > content has a clear lead where the first few seconds are clear.  So
> > they could be used to adjust the starting timestamps (and whatever
> > parsing needs) and the encrypted content later can be deduced based on
> > that.
> >
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Modern terrorism, a quick summary: Need oil, start war with country that
> has oil, kill hundread thousand in war. Let country fall into chaos,
> be surprised about raise of fundamantalists. Drop more bombs, kill more
> people, be surprised about them taking revenge and drop even more bombs
> and strip your own citizens of their rights and freedoms. to be continued
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jacob Trimble Sept. 11, 2018, 6:41 p.m. UTC | #6
On Thu, Aug 30, 2018 at 8:43 AM Jacob Trimble <modmaker@google.com> wrote:
>
> On Wed, Aug 29, 2018 at 4:37 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Wed, Aug 29, 2018 at 03:30:39PM -0700, Jacob Trimble wrote:
> > > On Wed, Aug 29, 2018 at 3:20 PM James Almer <jamrial@gmail.com> wrote:
> > > >
> > > > On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> > > > > On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> > > > >> If a packet is full-sample encrypted, then packet data can't be parsed
> > > > >> without decrypting it.  So this skips the packet parsing for those
> > > > >> packets.  If the packet has sub-sample encryption, it is assumed that
> > > > >> the headers are in the clear and the parser will only need that info.
> > > > >>
> > > > >> Signed-off-by: Jacob Trimble <modmaker@google.com>
> > > > >> ---
> > > > >>  libavformat/utils.c | 21 ++++++++++++++++++++-
> > > > >>  1 file changed, 20 insertions(+), 1 deletion(-)
> > > > >
> > > > > Hmm, please correct me if iam wrong but IIUC
> > > > > 1. The demuxer has set need_parsing, indicating that it _requires_ a parser
> > >
> > > There isn't a flag for "try parsing and ignore errors".  So my choice
> > > (from an app) is to either require parsing or don't do parsing at all
> > > (which can result in incorrect timestamps).
> > >
> > > > > 2. Parsing cannot be done before decrypting
> > > > >
> > > > > If all this and the set values are correct, logically, the fix would be
> > > > > to decrypt the packet before the parser not to skip the parser.
> > >
> >
> > > There are cases where the packet can't be decrypted before getting to
> > > this point.
> >
> > Can you elaborate on these cases ? I dont think its obvious what these
> > cases are, at least its not obvious to me what exactly you are thinking
> > of here. And i think its not helpfull if i guess what you mean and then
> > argue/comment on that because maybe you meant something entirely different ...
>
> Fair warning, I am working on a DRM solution.  I know many of you may
> not agree with DRM, but it is a requirement imposed by content
> creators.  FFmpeg doesn't have to support DRM itself, but you should
> still consider its usages.
>
> At that point in the code, the packet can only be decrypted by the
> demuxer.  For example, in MP4 this can be done by passing the
> -decryption_key parameter.  But that requires that FFmpeg handle the
> decryption.  In my case, we are passing the packet to a CDM (content
> decryption module) to handle the decryption and that might be a
> hardware-secure decryptor.  In most DRM situations, we can't get the
> raw key at all, all we can do is say "decrypt this block of memory".
> So the only way to have the packet decrypted before this point would
> be to introduce a callback to the app to decrypt the packet.
>
> This is why I have been working on exposing the encryption info.  My
> app (or more correctly the CDM) needs to handle the decryption and we
> can't just give the key to libavformat so the demuxer can decrypt the
> packet.
>
> >
> >
> > > This point is between the demuxer creating the packet and
> > > giving to the app.  So the only way to decrypt the frame (as of now)
> > > is for the demuxer to do this.  There is no way for the app to handle
> > > the decryption before this point.
> > >
> >
> > > From the app's perspective, I would expect the packet to remain
> > > encrypted for as long as possible.
> >
> > why ?
>
> Because the point of encryption is to ensure that nefarious parties
> don't get access to the data.  Even though the packet data is stored
> in memory, it could still be retrieved by a malicious user.  Usually
> for protected media, the frames are only decrypted when needed and in
> some cases are decrypted in a secure CPU and isn't even accessible to
> the app.  There are platforms that support what is called "direct
> render" where the app gives the platform the encrypted packets and a
> discrete hardware unit will decrypt the packet, decode it, and render
> it directly; this happens on some smart TVs.  There are also cases
> which require decrypt-decode where we give the platform an encrypted
> packet and it will decrypt and decode the packet, ensuring the
> decrypted packet data is never in main memory.  So there are cases
> where we can't even see the encoded packet and decoding is handled by
> the platform.
>
> >
> >
> > > My app will store the packet for a
> > > while, then decrypt it right before passing it to the decoder and
> > > drawing the frame.  So requiring the packet to be decrypted at this
> > > point is not ideal.
> > >
> > > >
> > > > And if that can't be done, then the demuxer could perhaps set
> > > > st->need_parsing to 0 and skip parsing altogether?
> > >
> > > I would want to still have parsing if possible. For example, a lot of
> > > content has a clear lead where the first few seconds are clear.  So
> > > they could be used to adjust the starting timestamps (and whatever
> > > parsing needs) and the encrypted content later can be deduced based on
> > > that.
> > >
> > [...]
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Modern terrorism, a quick summary: Need oil, start war with country that
> > has oil, kill hundread thousand in war. Let country fall into chaos,
> > be surprised about raise of fundamantalists. Drop more bombs, kill more
> > people, be surprised about them taking revenge and drop even more bombs
> > and strip your own citizens of their rights and freedoms. to be continued
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Ping.
Michael Niedermayer Sept. 11, 2018, 8:48 p.m. UTC | #7
On Thu, Aug 30, 2018 at 08:43:25AM -0700, Jacob Trimble wrote:
> On Wed, Aug 29, 2018 at 4:37 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Wed, Aug 29, 2018 at 03:30:39PM -0700, Jacob Trimble wrote:
> > > On Wed, Aug 29, 2018 at 3:20 PM James Almer <jamrial@gmail.com> wrote:
> > > >
> > > > On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> > > > > On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> > > > >> If a packet is full-sample encrypted, then packet data can't be parsed
> > > > >> without decrypting it.  So this skips the packet parsing for those
> > > > >> packets.  If the packet has sub-sample encryption, it is assumed that
> > > > >> the headers are in the clear and the parser will only need that info.
> > > > >>
> > > > >> Signed-off-by: Jacob Trimble <modmaker@google.com>
> > > > >> ---
> > > > >>  libavformat/utils.c | 21 ++++++++++++++++++++-
> > > > >>  1 file changed, 20 insertions(+), 1 deletion(-)
> > > > >
> > > > > Hmm, please correct me if iam wrong but IIUC
> > > > > 1. The demuxer has set need_parsing, indicating that it _requires_ a parser
> > >
> > > There isn't a flag for "try parsing and ignore errors".  So my choice
> > > (from an app) is to either require parsing or don't do parsing at all
> > > (which can result in incorrect timestamps).
> > >
> > > > > 2. Parsing cannot be done before decrypting
> > > > >
> > > > > If all this and the set values are correct, logically, the fix would be
> > > > > to decrypt the packet before the parser not to skip the parser.
> > >
> >
> > > There are cases where the packet can't be decrypted before getting to
> > > this point.
> >
> > Can you elaborate on these cases ? I dont think its obvious what these
> > cases are, at least its not obvious to me what exactly you are thinking
> > of here. And i think its not helpfull if i guess what you mean and then
> > argue/comment on that because maybe you meant something entirely different ...
> 
> Fair warning, I am working on a DRM solution.  I know many of you may
> not agree with DRM, but it is a requirement imposed by content
> creators.  FFmpeg doesn't have to support DRM itself, but you should
> still consider its usages.
> 
> At that point in the code, the packet can only be decrypted by the
> demuxer.  For example, in MP4 this can be done by passing the
> -decryption_key parameter.  But that requires that FFmpeg handle the
> decryption.  In my case, we are passing the packet to a CDM (content
> decryption module) to handle the decryption and that might be a
> hardware-secure decryptor.  In most DRM situations, we can't get the
> raw key at all, all we can do is say "decrypt this block of memory".
> So the only way to have the packet decrypted before this point would
> be to introduce a callback to the app to decrypt the packet.

1. If the demuxer sets need_parsing it requires parsing
2. If the demuxer requires parsing then parsing should be done
3. If parsing cannot be done but is required thats a error and should produce
   some warning or error message. It should not be part of some intended 
   design


> 
> This is why I have been working on exposing the encryption info.  My
> app (or more correctly the CDM) needs to handle the decryption and we
> can't just give the key to libavformat so the demuxer can decrypt the
> packet.
> 
> >
> >
> > > This point is between the demuxer creating the packet and
> > > giving to the app.  So the only way to decrypt the frame (as of now)
> > > is for the demuxer to do this.  There is no way for the app to handle
> > > the decryption before this point.
> > >
> >
> > > From the app's perspective, I would expect the packet to remain
> > > encrypted for as long as possible.
> >
> > why ?
> 
> Because the point of encryption is to ensure that nefarious parties

you call your customers, "nefarious" ? I guess its none of my buisness
but i dont think this is a good mindset.


> don't get access to the data.  Even though the packet data is stored
> in memory, it could still be retrieved by a malicious user.  Usually
> for protected media, the frames are only decrypted when needed and in
> some cases are decrypted in a secure CPU and isn't even accessible to
> the app.  There are platforms that support what is called "direct
> render" where the app gives the platform the encrypted packets and a
> discrete hardware unit will decrypt the packet, decode it, and render
> it directly; this happens on some smart TVs.  There are also cases
> which require decrypt-decode where we give the platform an encrypted
> packet and it will decrypt and decode the packet, ensuring the
> decrypted packet data is never in main memory.  So there are cases
> where we can't even see the encoded packet and decoding is handled by
> the platform.

you arent yet using the cortex decryptor module with neuralink?

seriously, i think without this your nefarious customers will still be able to
easily record the data if they want. All this fancy stuff doesnt really
provide any real protection. And it makes people hate you/your company.

Anyway, if you want to skip the parser on random subsets of packets (encrypted) 
i think this requires more care. For example i think the parser could
buffer some data, if some random set of packets bypass the parser
this could loose data at the point of switchover.
For which parsers is that working, do they need flushing, ...
it should not just be done while totally ignoring if the code can handle
being switched like this
also the interrested user who hits a problem (aka failure due to lack of
requested parsing) and increases verbosity should have a chance to 
find the cause. There should be some sort of debug or warning message
that requested parsing is not done.

Still, personally i think it would be better to decrypt before the
parser, its not really going to make a difference for someone who
wants to access the data but it makes the code cleaner and more robust

Thanks

[...]
Jacob Trimble Sept. 11, 2018, 10:50 p.m. UTC | #8
On Tue, Sep 11, 2018 at 1:48 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Thu, Aug 30, 2018 at 08:43:25AM -0700, Jacob Trimble wrote:
> > On Wed, Aug 29, 2018 at 4:37 PM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Wed, Aug 29, 2018 at 03:30:39PM -0700, Jacob Trimble wrote:
> > > > On Wed, Aug 29, 2018 at 3:20 PM James Almer <jamrial@gmail.com> wrote:
> > > > >
> > > > > On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> > > > > > On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> > > > > >> If a packet is full-sample encrypted, then packet data can't be parsed
> > > > > >> without decrypting it.  So this skips the packet parsing for those
> > > > > >> packets.  If the packet has sub-sample encryption, it is assumed that
> > > > > >> the headers are in the clear and the parser will only need that info.
> > > > > >>
> > > > > >> Signed-off-by: Jacob Trimble <modmaker@google.com>
> > > > > >> ---
> > > > > >>  libavformat/utils.c | 21 ++++++++++++++++++++-
> > > > > >>  1 file changed, 20 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > Hmm, please correct me if iam wrong but IIUC
> > > > > > 1. The demuxer has set need_parsing, indicating that it _requires_ a parser
> > > >
> > > > There isn't a flag for "try parsing and ignore errors".  So my choice
> > > > (from an app) is to either require parsing or don't do parsing at all
> > > > (which can result in incorrect timestamps).
> > > >
> > > > > > 2. Parsing cannot be done before decrypting
> > > > > >
> > > > > > If all this and the set values are correct, logically, the fix would be
> > > > > > to decrypt the packet before the parser not to skip the parser.
> > > >
> > >
> > > > There are cases where the packet can't be decrypted before getting to
> > > > this point.
> > >
> > > Can you elaborate on these cases ? I dont think its obvious what these
> > > cases are, at least its not obvious to me what exactly you are thinking
> > > of here. And i think its not helpfull if i guess what you mean and then
> > > argue/comment on that because maybe you meant something entirely different ...
> >
> > Fair warning, I am working on a DRM solution.  I know many of you may
> > not agree with DRM, but it is a requirement imposed by content
> > creators.  FFmpeg doesn't have to support DRM itself, but you should
> > still consider its usages.
> >
> > At that point in the code, the packet can only be decrypted by the
> > demuxer.  For example, in MP4 this can be done by passing the
> > -decryption_key parameter.  But that requires that FFmpeg handle the
> > decryption.  In my case, we are passing the packet to a CDM (content
> > decryption module) to handle the decryption and that might be a
> > hardware-secure decryptor.  In most DRM situations, we can't get the
> > raw key at all, all we can do is say "decrypt this block of memory".
> > So the only way to have the packet decrypted before this point would
> > be to introduce a callback to the app to decrypt the packet.
>
> 1. If the demuxer sets need_parsing it requires parsing
> 2. If the demuxer requires parsing then parsing should be done
> 3. If parsing cannot be done but is required thats a error and should produce
>    some warning or error message. It should not be part of some intended
>    design
>

So would a log be enough?  That's what happens if the parser fails to
be created or can't be found.

>
> >
> > This is why I have been working on exposing the encryption info.  My
> > app (or more correctly the CDM) needs to handle the decryption and we
> > can't just give the key to libavformat so the demuxer can decrypt the
> > packet.
> >
> > >
> > >
> > > > This point is between the demuxer creating the packet and
> > > > giving to the app.  So the only way to decrypt the frame (as of now)
> > > > is for the demuxer to do this.  There is no way for the app to handle
> > > > the decryption before this point.
> > > >
> > >
> > > > From the app's perspective, I would expect the packet to remain
> > > > encrypted for as long as possible.
> > >
> > > why ?
> >
> > Because the point of encryption is to ensure that nefarious parties
>
> you call your customers, "nefarious" ? I guess its none of my buisness
> but i dont think this is a good mindset.
>

The people who bought the movie are our customers; the people trying
to get the decrypted frames without paying, or trying to distribute
the movie for free are the nefarious ones.

>
> > don't get access to the data.  Even though the packet data is stored
> > in memory, it could still be retrieved by a malicious user.  Usually
> > for protected media, the frames are only decrypted when needed and in
> > some cases are decrypted in a secure CPU and isn't even accessible to
> > the app.  There are platforms that support what is called "direct
> > render" where the app gives the platform the encrypted packets and a
> > discrete hardware unit will decrypt the packet, decode it, and render
> > it directly; this happens on some smart TVs.  There are also cases
> > which require decrypt-decode where we give the platform an encrypted
> > packet and it will decrypt and decode the packet, ensuring the
> > decrypted packet data is never in main memory.  So there are cases
> > where we can't even see the encoded packet and decoding is handled by
> > the platform.
>
> you arent yet using the cortex decryptor module with neuralink?
>
> seriously, i think without this your nefarious customers will still be able to
> easily record the data if they want. All this fancy stuff doesnt really
> provide any real protection. And it makes people hate you/your company.
>
> Anyway, if you want to skip the parser on random subsets of packets (encrypted)
> i think this requires more care. For example i think the parser could
> buffer some data, if some random set of packets bypass the parser
> this could loose data at the point of switchover.
> For which parsers is that working, do they need flushing, ...
> it should not just be done while totally ignoring if the code can handle
> being switched like this
> also the interrested user who hits a problem (aka failure due to lack of
> requested parsing) and increases verbosity should have a chance to
> find the cause. There should be some sort of debug or warning message
> that requested parsing is not done.

So how about, when we see an encrypted frame, we flush the parser
before skipping the frame?  Can we just flush the parser and then
reuse it later?  Or would we need to create a new parser if we saw
clear frames later?

>
> Still, personally i think it would be better to decrypt before the
> parser, its not really going to make a difference for someone who
> wants to access the data but it makes the code cleaner and more robust

Like I said, in some cases, this is impossible.  If we are running on
an Android device or a smart TV, we'll use a hardware secure decoder
where the OS will decrypt and decode the video in a secure chip to
avoid having the encoded frame in the clear at all.  So we literally
can't decrypt the frames ourselves.  Even if we could decrypt (for
software decoding), we can't under the current design since the app
needs to decrypt the data and the app can't get to the data before
this point.

>
> Thanks
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The greatest way to live with honor in this world is to be what we pretend
> to be. -- Socrates
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Michael Niedermayer Sept. 12, 2018, 6:50 p.m. UTC | #9
On Tue, Sep 11, 2018 at 03:50:57PM -0700, Jacob Trimble wrote:
> On Tue, Sep 11, 2018 at 1:48 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Thu, Aug 30, 2018 at 08:43:25AM -0700, Jacob Trimble wrote:
> > > On Wed, Aug 29, 2018 at 4:37 PM Michael Niedermayer
> > > <michael@niedermayer.cc> wrote:
> > > >
> > > > On Wed, Aug 29, 2018 at 03:30:39PM -0700, Jacob Trimble wrote:
> > > > > On Wed, Aug 29, 2018 at 3:20 PM James Almer <jamrial@gmail.com> wrote:
> > > > > >
> > > > > > On 8/29/2018 7:07 PM, Michael Niedermayer wrote:
> > > > > > > On Tue, Aug 28, 2018 at 10:58:43AM -0700, Jacob Trimble wrote:
> > > > > > >> If a packet is full-sample encrypted, then packet data can't be parsed
> > > > > > >> without decrypting it.  So this skips the packet parsing for those
> > > > > > >> packets.  If the packet has sub-sample encryption, it is assumed that
> > > > > > >> the headers are in the clear and the parser will only need that info.
> > > > > > >>
> > > > > > >> Signed-off-by: Jacob Trimble <modmaker@google.com>
> > > > > > >> ---
> > > > > > >>  libavformat/utils.c | 21 ++++++++++++++++++++-
> > > > > > >>  1 file changed, 20 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > Hmm, please correct me if iam wrong but IIUC
> > > > > > > 1. The demuxer has set need_parsing, indicating that it _requires_ a parser
> > > > >
> > > > > There isn't a flag for "try parsing and ignore errors".  So my choice
> > > > > (from an app) is to either require parsing or don't do parsing at all
> > > > > (which can result in incorrect timestamps).
> > > > >
> > > > > > > 2. Parsing cannot be done before decrypting
> > > > > > >
> > > > > > > If all this and the set values are correct, logically, the fix would be
> > > > > > > to decrypt the packet before the parser not to skip the parser.
> > > > >
> > > >
> > > > > There are cases where the packet can't be decrypted before getting to
> > > > > this point.
> > > >
> > > > Can you elaborate on these cases ? I dont think its obvious what these
> > > > cases are, at least its not obvious to me what exactly you are thinking
> > > > of here. And i think its not helpfull if i guess what you mean and then
> > > > argue/comment on that because maybe you meant something entirely different ...
> > >
> > > Fair warning, I am working on a DRM solution.  I know many of you may
> > > not agree with DRM, but it is a requirement imposed by content
> > > creators.  FFmpeg doesn't have to support DRM itself, but you should
> > > still consider its usages.
> > >
> > > At that point in the code, the packet can only be decrypted by the
> > > demuxer.  For example, in MP4 this can be done by passing the
> > > -decryption_key parameter.  But that requires that FFmpeg handle the
> > > decryption.  In my case, we are passing the packet to a CDM (content
> > > decryption module) to handle the decryption and that might be a
> > > hardware-secure decryptor.  In most DRM situations, we can't get the
> > > raw key at all, all we can do is say "decrypt this block of memory".
> > > So the only way to have the packet decrypted before this point would
> > > be to introduce a callback to the app to decrypt the packet.
> >
> > 1. If the demuxer sets need_parsing it requires parsing
> > 2. If the demuxer requires parsing then parsing should be done
> > 3. If parsing cannot be done but is required thats a error and should produce
> >    some warning or error message. It should not be part of some intended
> >    design
> >
> 
> So would a log be enough?  That's what happens if the parser fails to
> be created or can't be found.

if a parser is required and it fails to be created thats the same issue
it should produce something an interrested user or developer can see
to understand where a potential problem comes from


> 
> >
> > >
> > > This is why I have been working on exposing the encryption info.  My
> > > app (or more correctly the CDM) needs to handle the decryption and we
> > > can't just give the key to libavformat so the demuxer can decrypt the
> > > packet.
> > >
> > > >
> > > >
> > > > > This point is between the demuxer creating the packet and
> > > > > giving to the app.  So the only way to decrypt the frame (as of now)
> > > > > is for the demuxer to do this.  There is no way for the app to handle
> > > > > the decryption before this point.
> > > > >
> > > >
> > > > > From the app's perspective, I would expect the packet to remain
> > > > > encrypted for as long as possible.
> > > >
> > > > why ?
> > >
> > > Because the point of encryption is to ensure that nefarious parties
> >
> > you call your customers, "nefarious" ? I guess its none of my buisness
> > but i dont think this is a good mindset.
> >
> 
> The people who bought the movie are our customers; the people trying
> to get the decrypted frames without paying, or trying to distribute
> the movie for free are the nefarious ones.

wanting to access the decrypted frames doesnt imply that they want to
distribute anything.
How many people have a hw player or something which doesnt support every
kind of DRM? Its not unreasonable to want to make a backup of material one
spend good money on and not unreasonable one wants to watch it with a device
different from what the seller intended. Theres nothing nefarious in any of
this.
Also more rarely one may want to comment, critique or otherwise refer to
a small part of a copyrighted movie. Again this is not hurting your sales or
anything. If anything it might help from extra "advertisment"
People reading a paper or watching a 10min critique of a movie arent going to
rush out and buy that movie if that paper or critique doesnt exist in the
first place.

The people who want to distribute the movie for free tend to not be affected
much by DRM or law. Its the average person who has no bad intend this hits.


> 
> >
> > > don't get access to the data.  Even though the packet data is stored
> > > in memory, it could still be retrieved by a malicious user.  Usually
> > > for protected media, the frames are only decrypted when needed and in
> > > some cases are decrypted in a secure CPU and isn't even accessible to
> > > the app.  There are platforms that support what is called "direct
> > > render" where the app gives the platform the encrypted packets and a
> > > discrete hardware unit will decrypt the packet, decode it, and render
> > > it directly; this happens on some smart TVs.  There are also cases
> > > which require decrypt-decode where we give the platform an encrypted
> > > packet and it will decrypt and decode the packet, ensuring the
> > > decrypted packet data is never in main memory.  So there are cases
> > > where we can't even see the encoded packet and decoding is handled by
> > > the platform.
> >
> > you arent yet using the cortex decryptor module with neuralink?
> >
> > seriously, i think without this your nefarious customers will still be able to
> > easily record the data if they want. All this fancy stuff doesnt really
> > provide any real protection. And it makes people hate you/your company.
> >
> > Anyway, if you want to skip the parser on random subsets of packets (encrypted)
> > i think this requires more care. For example i think the parser could
> > buffer some data, if some random set of packets bypass the parser
> > this could loose data at the point of switchover.
> > For which parsers is that working, do they need flushing, ...
> > it should not just be done while totally ignoring if the code can handle
> > being switched like this
> > also the interrested user who hits a problem (aka failure due to lack of
> > requested parsing) and increases verbosity should have a chance to
> > find the cause. There should be some sort of debug or warning message
> > that requested parsing is not done.
> 
> So how about, when we see an encrypted frame, we flush the parser
> before skipping the frame?  Can we just flush the parser and then
> reuse it later?  Or would we need to create a new parser if we saw
> clear frames later?

try/test it and review the code. only way to know for sure what works

thanks

[...]
diff mbox

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index b0b5e164a6..1107787eae 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -27,6 +27,7 @@ 
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
 #include "libavutil/dict.h"
+#include "libavutil/encryption_info.h"
 #include "libavutil/internal.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
@@ -1576,6 +1577,9 @@  static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
     while (!got_packet && !s->internal->parse_queue) {
         AVStream *st;
         AVPacket cur_pkt;
+        uint8_t *enc_side_data;
+        int enc_side_data_size;
+        int is_full_encrypted = 0;
 
         /* read next packet */
         ret = ff_read_packet(s, &cur_pkt);
@@ -1659,7 +1663,22 @@  FF_ENABLE_DEPRECATION_WARNINGS
                 st->parser->flags |= PARSER_FLAG_USE_CODEC_TS;
         }
 
-        if (!st->need_parsing || !st->parser) {
+        /* if the packet is full-sample encrypted, we can't parse it.  If the
+         * packet uses sub-sample encryption, assume the headers are clear and
+         * can still be parsed.
+         */
+        enc_side_data = av_packet_get_side_data(
+            &cur_pkt, AV_PKT_DATA_ENCRYPTION_INFO, &enc_side_data_size);
+        if (enc_side_data) {
+            AVEncryptionInfo *enc_info =
+                av_encryption_info_get_side_data(enc_side_data, enc_side_data_size);
+            if (enc_info) {
+                is_full_encrypted = enc_info->subsample_count == 0;
+                av_encryption_info_free(enc_info);
+            }
+        }
+
+        if (!st->need_parsing || !st->parser || is_full_encrypted) {
             /* no parsing needed: we just output the packet as is */
             *pkt = cur_pkt;
             compute_pkt_fields(s, st, NULL, pkt, AV_NOPTS_VALUE, AV_NOPTS_VALUE);