diff mbox

[FFmpeg-devel] libavutil/encryption_info: Allow multiple init info.

Message ID CAO7y9i-O5=oxUGTa5YM+fNrc9g1pmFayjvcehh1JFvw-2XEk7A@mail.gmail.com
State Superseded
Headers show

Commit Message

Jacob Trimble April 28, 2018, 12:30 a.m. UTC 
On Fri, Apr 27, 2018 at 10:33 AM, Jacob Trimble <modmaker@google.com> wrote:
> On Mon, Apr 23, 2018 at 11:03 AM, Jacob Trimble <modmaker@google.com> wrote:
>> While integrating my encryption info changes, I noticed a problem with
>> the init info structs.  I implemented them as side-data on the Stream.
>> But this means there can only be one per stream.  However, there can
>> be multiple 'pssh' atoms in a single stream (e.g. for key rotation or
>> multiple key systems). (sorry for not noticing sooner)
>>
>> Attached is a patch to fix this by making the init info a
>> singly-linked-list.  It is ABI compatible and is still easy to use and
>> understand.  The alternative would be to break ABI and have the
>> side-data methods return an array of pointers.  I could do that
>> instead if that is preferable.
>
> Ping

Noticed a bug, fixed.

Comments

Jacob Trimble May 7, 2018, 4:34 p.m. UTC | #1 
On Fri, Apr 27, 2018 at 5:30 PM, Jacob Trimble <modmaker@google.com> wrote:
> On Fri, Apr 27, 2018 at 10:33 AM, Jacob Trimble <modmaker@google.com> wrote:
>> On Mon, Apr 23, 2018 at 11:03 AM, Jacob Trimble <modmaker@google.com> wrote:
>>> While integrating my encryption info changes, I noticed a problem with
>>> the init info structs.  I implemented them as side-data on the Stream.
>>> But this means there can only be one per stream.  However, there can
>>> be multiple 'pssh' atoms in a single stream (e.g. for key rotation or
>>> multiple key systems). (sorry for not noticing sooner)
>>>
>>> Attached is a patch to fix this by making the init info a
>>> singly-linked-list.  It is ABI compatible and is still easy to use and
>>> understand.  The alternative would be to break ABI and have the
>>> side-data methods return an array of pointers.  I could do that
>>> instead if that is preferable.
>>
>> Ping
>
> Noticed a bug, fixed.

Ping.
diff mbox

Patch

From 50ec8ed80687134ecb9e74c45336e920d9e28666 Mon Sep 17 00:00:00 2001
From: Jacob Trimble <modmaker@google.com>
Date: Mon, 23 Apr 2018 10:33:58 -0700
Subject: [PATCH] libavutil/encryption_info: Allow multiple init info.

It is possible for there to be multiple encryption init info structure.
For example, to support multiple key systems or in key rotation.  This
changes the AVEncryptionInitInfo struct to be a linked list so there
can be multiple structs without breaking ABI.

Signed-off-by: Jacob Trimble <modmaker@google.com>
---
 libavutil/encryption_info.c | 156 +++++++++++++++++++++++-------------
 libavutil/encryption_info.h |   5 ++
 2 files changed, 107 insertions(+), 54 deletions(-)

diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
index 20a752d6b4..2329d94f12 100644
--- a/libavutil/encryption_info.c
+++ b/libavutil/encryption_info.c
@@ -160,13 +160,16 @@  uint8_t *av_encryption_info_add_side_data(const AVEncryptionInfo *info, size_t *
 }
 
 // The format of the AVEncryptionInitInfo side data:
-// u32be system_id_size
-// u32be num_key_ids
-// u32be key_id_size
-// u32be data_size
-// u8[system_id_size] system_id
-// u8[key_id_size][num_key_id] key_ids
-// u8[data_size] data
+// u32be init_info_count
+// {
+//   u32be system_id_size
+//   u32be num_key_ids
+//   u32be key_id_size
+//   u32be data_size
+//   u8[system_id_size] system_id
+//   u8[key_id_size][num_key_id] key_ids
+//   u8[data_size] data
+// }[init_info_count]
 
 #define FF_ENCRYPTION_INIT_INFO_EXTRA 16
 
@@ -215,6 +218,7 @@  void av_encryption_init_info_free(AVEncryptionInitInfo *info)
         for (i = 0; i < info->num_key_ids; i++) {
             av_free(info->key_ids[i]);
         }
+        av_encryption_init_info_free(info->next);
         av_free(info->system_id);
         av_free(info->key_ids);
         av_free(info->data);
@@ -225,71 +229,115 @@  void av_encryption_init_info_free(AVEncryptionInitInfo *info)
 AVEncryptionInitInfo *av_encryption_init_info_get_side_data(
     const uint8_t *side_data, size_t side_data_size)
 {
-    AVEncryptionInitInfo *info;
-    uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
-
-    if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
-        return NULL;
-
-    system_id_size = AV_RB32(side_data);
-    num_key_ids = AV_RB32(side_data + 4);
-    key_id_size = AV_RB32(side_data + 8);
-    data_size = AV_RB32(side_data + 12);
-
-    // UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
-    if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
-        return NULL;
+    AVEncryptionInitInfo *ret = NULL, *info;
+    uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
+    uint64_t init_info_count;
 
-    info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
-    if (!info)
+    if (!side_data || side_data_size < 4)
         return NULL;
 
-    memcpy(info->system_id, side_data + 16, system_id_size);
-    side_data += system_id_size + 16;
-    for (i = 0; i < num_key_ids; i++) {
-      memcpy(info->key_ids[i], side_data, key_id_size);
-      side_data += key_id_size;
+    init_info_count = AV_RB32(side_data);
+    side_data += 4;
+    side_data_size -= 4;
+    for (i = 0; i < init_info_count; i++) {
+      if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
+          av_encryption_init_info_free(ret);
+          return NULL;
+      }
+
+      system_id_size = AV_RB32(side_data);
+      num_key_ids = AV_RB32(side_data + 4);
+      key_id_size = AV_RB32(side_data + 8);
+      data_size = AV_RB32(side_data + 12);
+
+      // UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
+      if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
+          av_encryption_init_info_free(ret);
+          return NULL;
+      }
+      side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
+      side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
+
+      info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
+      if (!info) {
+          av_encryption_init_info_free(ret);
+          return NULL;
+      }
+      info->next = ret;
+      ret = info;
+
+      memcpy(info->system_id, side_data, system_id_size);
+      side_data += system_id_size;
+      side_data_size -= system_id_size;
+      for (j = 0; j < num_key_ids; j++) {
+        memcpy(info->key_ids[j], side_data, key_id_size);
+        side_data += key_id_size;
+        side_data_size -= key_id_size;
+      }
+      memcpy(info->data, side_data, data_size);
+      side_data += data_size;
+      side_data_size -= data_size;
     }
-    memcpy(info->data, side_data, data_size);
 
-    return info;
+    return ret;
 }
 
 uint8_t *av_encryption_init_info_add_side_data(const AVEncryptionInitInfo *info, size_t *side_data_size)
 {
+    const AVEncryptionInitInfo *cur_info, *cur_info_2;
     uint8_t *buffer, *cur_buffer;
-    uint32_t i, max_size;
-
-    if (UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA < info->system_id_size ||
-        UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA - info->system_id_size < info->data_size) {
-        return NULL;
-    }
-
-    if (info->num_key_ids) {
-        max_size = UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA - info->system_id_size - info->data_size;
-        if (max_size / info->num_key_ids < info->key_id_size)
+    uint32_t i, init_info_count;
+
+    *side_data_size = 4;
+    init_info_count = 0;
+    for (cur_info = info; cur_info; cur_info = cur_info->next) {
+      // Detect a circular list.
+      for (cur_info_2 = info; cur_info_2 != cur_info; cur_info_2 = cur_info_2->next) {
+          if (cur_info->next == cur_info_2)
+              return NULL;
+      }
+      if (cur_info->next == cur_info)
+          return NULL;
+
+      if (UINT32_MAX == init_info_count ||
+          UINT32_MAX - *side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA ||
+          UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < info->system_id_size ||
+          UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA - info->system_id_size < info->data_size) {
+          return NULL;
+      }
+      *side_data_size += FF_ENCRYPTION_INIT_INFO_EXTRA + info->system_id_size + info->data_size;
+      init_info_count++;
+
+      if (info->num_key_ids) {
+        if ((UINT32_MAX - *side_data_size) / info->num_key_ids < info->key_id_size) {
             return NULL;
+        }
+        *side_data_size += info->num_key_ids * info->key_id_size;
+      }
     }
 
-    *side_data_size = FF_ENCRYPTION_INIT_INFO_EXTRA + info->system_id_size +
-                      info->data_size + (info->num_key_ids * info->key_id_size);
     cur_buffer = buffer = av_malloc(*side_data_size);
     if (!buffer)
         return NULL;
 
-    AV_WB32(cur_buffer,      info->system_id_size);
-    AV_WB32(cur_buffer +  4, info->num_key_ids);
-    AV_WB32(cur_buffer +  8, info->key_id_size);
-    AV_WB32(cur_buffer + 12, info->data_size);
-    cur_buffer += 16;
-
-    memcpy(cur_buffer, info->system_id, info->system_id_size);
-    cur_buffer += info->system_id_size;
-    for (i = 0; i < info->num_key_ids; i++) {
-        memcpy(cur_buffer, info->key_ids[i], info->key_id_size);
-        cur_buffer += info->key_id_size;
+    AV_WB32(cur_buffer, init_info_count);
+    cur_buffer += 4;
+    for (cur_info = info; cur_info; cur_info = cur_info->next) {
+      AV_WB32(cur_buffer,      info->system_id_size);
+      AV_WB32(cur_buffer +  4, info->num_key_ids);
+      AV_WB32(cur_buffer +  8, info->key_id_size);
+      AV_WB32(cur_buffer + 12, info->data_size);
+      cur_buffer += 16;
+
+      memcpy(cur_buffer, info->system_id, info->system_id_size);
+      cur_buffer += info->system_id_size;
+      for (i = 0; i < info->num_key_ids; i++) {
+          memcpy(cur_buffer, info->key_ids[i], info->key_id_size);
+          cur_buffer += info->key_id_size;
+      }
+      memcpy(cur_buffer, info->data, info->data_size);
+      cur_buffer += info->data_size;
     }
-    memcpy(cur_buffer, info->data, info->data_size);
 
     return buffer;
 }
diff --git a/libavutil/encryption_info.h b/libavutil/encryption_info.h
index ec5501aac7..9140968fde 100644
--- a/libavutil/encryption_info.h
+++ b/libavutil/encryption_info.h
@@ -115,6 +115,11 @@  typedef struct AVEncryptionInitInfo {
      */
     uint8_t* data;
     uint32_t data_size;
+
+    /**
+     * An optional pointer to the next initialization info in the list.
+     */
+    struct AVEncryptionInitInfo *next;
 } AVEncryptionInitInfo;
 
 /**
-- 
2.17.0.441.gb46fe60e1d-goog