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

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #354562|none                        |needs-work
             status|                            |

--- Comment #19 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 354562:
 --> (https://bugzilla.gnome.org/review?bug=765275&attachment=354562)

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.

@@ +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
:)

@@ +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

@@ +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?

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