[Bug 765275] matroska: implement reading & writing ContentEncryption headers

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=765275

--- Comment #21 from y.bandou <bandou.yacine at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #19)
> Review of attachment 354562 [details] [review]:
> 
> Generally looks OK to me, but this needs to be generalized a bit and like I
> wrote in the previous comment: it needs to be documented. How does the
> Matroska/WebM spec map to buffer content, caps, events and GstMetas?
> 
> ::: gst/matroska/matroska-read-common.c
> @@ +354,3 @@
> + * Set those parsed information into protection info structure
> @info_protect which
> + * will be added in protection metadata of the Gstbuffer.
> + * The subsamples format follows the same pssh box format in Common
> Encryption spec:
> 
> This shouldn't really follow CENC but just parse the things from the
> Matroska container the way it is stored there. It's the job of the decryptor
> element to make sense of it, and it needs to understand the
> application/matroska-enc caps anyway.
> 

Yes I agree with you.

But I would like to have the same processing in the Decryptor and Parser
element when managing the sub-sample map.

> @@ +379,3 @@
> +  size = *size_out;
> +  /* For more detail see the WbeM's spec
> https://www.webmproject.org/docs/webm-encryption/#47-signal-byte-format */
> +  signal_byte = *data++;
> 
> Please use a GstByteReader here. All the manual byte parsing we do
> everywhere was already the reason for various CVEs and more harmless but
> annoying rashes :)
> 

I agree, I'll do it.

> @@ +421,3 @@
> +      /* For more details see
> https://www.webmproject.org/docs/webm-encryption/#46-subsample-encrypted-
> block-format */
> +      subsamples =
> +          g_malloc (nb_subsample * (sizeof (guint16) + sizeof (guint32)));
> 
> This calculation can overflow on specially crafted files. Also various other
> calculations here
> 
 guint8 nb_part = *data;
 ...  
 nb_subsample = (nb_part + 2) >> 1;

 I think nb_subsample can't be great than  127,  because nb_part is unsigned
byte.

> @@ +2970,3 @@
> +                GST_WARNING_OBJECT (common->sinkpad,
> +                    "Unexpected to have ContentEncAESSettings because
> Content Encryption Algo different to 5-AES");
> +                ret = gst_ebml_read_skip (ebml);
> 
> We can probably still parse that and output the encrypted stream, just that
> downstream will have an unsupported enc algo in the caps and can't handle
> it. Unless it can handle something else than AES. Or not?

The Webm support only AES algorithm.

The spec: https://www.webmproject.org/docs/container/

The encryption algorithm used. The value '0' means that the contents have not
been encrypted but only signed. Predefined values: 1 - DES; 2 - 3DES; 3 -
Twofish; 4 - Blowfish; 5 - AES. WebM only supports a value of 5 (AES).

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.