From 30a3da1b838607e1789743f5549b8dc0649ed2e1 Mon Sep 17 00:00:00 2001
From: "John \"Juce\" Bruce" <juce@google.com>
Date: Fri, 29 Apr 2022 11:19:35 -0700
Subject: [PATCH] Skip key padding better

(This is a merge of http://go/wvgerrit/151112.)

The Widevine CDMs have never validated the padding on AES keys. However,
the code to ignore the padding was unusual and based on the assumption
the keys would always have either 0 or 16 bytes of padding and did not
handle other cases correctly. This patch updates the padding-ignoring
code to just do the obvious thing: Reject keys that are too small and
ignore all extra bytes regardless of count.

Bug: 114159862
Test: x86-64
Change-Id: Ic48010477e4cb5f7d2afbde25cf2f098e3470089
---
 .../cdm/core/include/wv_cdm_constants.h       |  1 +
 libwvdrmengine/cdm/core/src/license.cpp       | 37 ++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/libwvdrmengine/cdm/core/include/wv_cdm_constants.h b/libwvdrmengine/cdm/core/include/wv_cdm_constants.h
index a66f3eb3..2e1a1d5d 100644
--- a/libwvdrmengine/cdm/core/include/wv_cdm_constants.h
+++ b/libwvdrmengine/cdm/core/include/wv_cdm_constants.h
@@ -15,6 +15,7 @@ static const size_t KEY_PAD_SIZE = 16;
 static const size_t CONTENT_KEY_SIZE = 16;
 static const size_t SERVICE_KEY_SIZE = 16;
 static const size_t MAC_KEY_SIZE = 32;
+static const size_t ENTITLEMENT_KEY_SIZE = 32;
 static const size_t KEYBOX_KEY_DATA_SIZE = 72;
 static const size_t SRM_REQUIREMENT_SIZE = 12;
 
diff --git a/libwvdrmengine/cdm/core/src/license.cpp b/libwvdrmengine/cdm/core/src/license.cpp
index 6ecd3413..5e8d477c 100644
--- a/libwvdrmengine/cdm/core/src/license.cpp
+++ b/libwvdrmengine/cdm/core/src/license.cpp
@@ -55,17 +55,19 @@ std::vector<CryptoKey> ExtractEntitlementKeys(const License& license) {
 
   for (int i = 0; i < license.key_size(); ++i) {
     CryptoKey key;
-    size_t length = 0;
     switch (license.key(i).type()) {
       case License_KeyContainer::ENTITLEMENT: {
-        // Strip off PKCS#5 padding - since we know the key is 32 or 48 bytes,
-        // the padding will always be 16 bytes.
-        if (license.key(i).key().size() > 32) {
-          length = license.key(i).key().size() - 16;
-        } else {
-          length = 0;
+        // We always take the first ENTITLEMENT_KEY_SIZE bytes and ignore the
+        // rest in order to ignore any padding.
+        if (license.key(i).key().size() < ENTITLEMENT_KEY_SIZE) {
+          LOGE(
+              "Skipping key %s because it is too small. Expected: %zu vs. "
+              "Actual: %zu",
+              license.key(i).id().c_str(), ENTITLEMENT_KEY_SIZE,
+              license.key(i).key().size());
+          continue;
         }
-        key.set_key_data(license.key(i).key().substr(0, length));
+        key.set_key_data(license.key(i).key().substr(0, ENTITLEMENT_KEY_SIZE));
         key.set_key_data_iv(license.key(i).iv());
         key.set_key_id(license.key(i).id());
         key.set_track_label(license.key(i).track_label());
@@ -111,20 +113,21 @@ std::vector<CryptoKey> ExtractContentKeys(const License& license) {
   // Extract content key(s)
   for (int i = 0; i < license.key_size(); ++i) {
     CryptoKey key;
-    size_t length;
     switch (license.key(i).type()) {
       case License_KeyContainer::CONTENT:
       case License_KeyContainer::OPERATOR_SESSION: {
         key.set_key_id(license.key(i).id());
-        // Strip off PKCS#5 padding - since we know the key is 16 or 32 bytes,
-        // the padding will always be 16 bytes.
-        // TODO(b/111069024): remove this!
-        if (license.key(i).key().size() > 16) {
-          length = license.key(i).key().size() - 16;
-        } else {
-          length = 0;
+        // We always take the first CONTENT_KEY_SIZE bytes and ignore the rest
+        // in order to ignore any padding.
+        if (license.key(i).key().size() < CONTENT_KEY_SIZE) {
+          LOGE(
+              "Skipping key %s because it is too small. Expected: %zu vs. "
+              "Actual: %zu",
+              license.key(i).id().c_str(), CONTENT_KEY_SIZE,
+              license.key(i).key().size());
+          continue;
         }
-        key.set_key_data(license.key(i).key().substr(0, length));
+        key.set_key_data(license.key(i).key().substr(0, CONTENT_KEY_SIZE));
         key.set_key_data_iv(license.key(i).iv());
         if (license.key(i).has_key_control()) {
           key.set_key_control(license.key(i).key_control().key_control_block());