Multiple Renewal Keys and Logging

In this code update we add a test to ensure that the White-box API
implementation handle seeing multiple renewal keys correctly. Since
there should be no more than one renewal key in a license response, upon
seeing a second renewal key, the implementation should return a
WB_RESULT_INVALID_PARAMETER code.

Due to changes in how Chrome manages CHECKS and DCHECKS, this code has
been updated to use the new headers.

api/aead_whitebox_benchmark.cc

@@ -7,7 +7,6 @@
 
 #include "api/aead_whitebox.h"
 #include "api/test_data.h"
-#include "base/logging.h"
 #include "benchmarking/data_source.h"
 #include "benchmarking/measurements.h"
 #include "testing/gtest/include/gtest/gtest.h"

api/license_whitebox_decrypt_benchmark.cc

@@ -11,7 +11,6 @@
 #include "api/result.h"
 #include "api/test_data.h"
 #include "api/test_license_builder.h"
-#include "base/logging.h"
 #include "benchmarking/data_source.h"
 #include "benchmarking/measurements.h"
 #include "testing/gtest/include/gtest/gtest.h"

api/license_whitebox_masked_decrypt_test.cc

@@ -11,7 +11,6 @@
 #include "api/license_whitebox_test_base.h"
 #include "api/test_data.h"
 #include "api/test_license_builder.h"
-#include "base/logging.h"
 #include "crypto_utils/crypto_util.h"
 #include "crypto_utils/rsa_key.h"
 #include "testing/gtest/include/gtest/gtest.h"

api/license_whitebox_process_license_response_benchmark.cc

@@ -8,7 +8,6 @@
 #include "api/result.h"
 #include "api/test_data.h"
 #include "api/test_license_builder.h"
-#include "base/logging.h"
 #include "benchmarking/measurements.h"
 #include "testing/gtest/include/gtest/gtest.h"
 

api/license_whitebox_process_license_response_test.cc

@@ -74,6 +74,26 @@ TEST_F(LicenseWhiteboxProcessLicenseResponseTest,
       WB_RESULT_OK);
 }
 
+TEST_F(LicenseWhiteboxProcessLicenseResponseTest,
+       InvalidParameterWithMultipleSigningKeys) {
+  TestLicenseBuilder builder;
+  builder.AddSigningKey(TestLicenseBuilder::DefaultSigningKey(),
+                        TestLicenseBuilder::PKSC8Padding());
+  builder.AddSigningKey(TestLicenseBuilder::DefaultSigningKey(),
+                        TestLicenseBuilder::PKSC8Padding());
+  builder.AddStubbedContentKey();
+  builder.Build(*public_key_, &license_);
+
+  ASSERT_EQ(
+      WB_License_ProcessLicenseResponse(
+          whitebox_, license_.core_message.data(), license_.core_message.size(),
+          license_.message.data(), license_.message.size(),
+          license_.signature.data(), license_.signature.size(),
+          license_.session_key.data(), license_.session_key.size(),
+          license_.request.data(), license_.request.size()),
+      WB_RESULT_INVALID_PARAMETER);
+}
+
 class LicenseWhiteboxProcessLicenseResponseErrorTest
     : public LicenseWhiteboxProcessLicenseResponseTest {
  protected:

api/license_whitebox_sign_benchmark.cc

@@ -11,7 +11,6 @@
 #include "api/result.h"
 #include "api/test_data.h"
 #include "api/test_license_builder.h"
-#include "base/logging.h"
 #include "benchmarking/data_source.h"
 #include "benchmarking/measurements.h"
 #include "testing/gtest/include/gtest/gtest.h"

api/license_whitebox_verify_benchmark.cc

@@ -11,7 +11,6 @@
 #include "api/result.h"
 #include "api/test_data.h"
 #include "api/test_license_builder.h"
-#include "base/logging.h"
 #include "benchmarking/data_source.h"
 #include "benchmarking/measurements.h"
 #include "testing/gtest/include/gtest/gtest.h"

api/test_license_builder.cc

@@ -4,7 +4,8 @@
 
 #include <ctime>
 
-#include "base/logging.h"
+#include "base/check.h"
+#include "base/check_op.h"
 #include "cdm/keys/certs.h"
 #include "crypto_utils/aes_cbc_encryptor.h"
 #include "crypto_utils/crypto_util.h"

chromium_deps/base/BUILD

@@ -2,7 +2,11 @@
 
 cc_library(
     name = "glog",
-    hdrs = ["logging.h"],
+    hdrs = [
+        "check.h",
+        "check_op.h",
+        "logging.h"
+    ],
     strip_include_prefix = "//chromium_deps",
     visibility = ["//visibility:public"],
     deps = [

chromium_deps/base/check.h

@@ -0,0 +1,13 @@
+// Copyright 2020 Google LLC. All Rights Reserved.
+
+#ifndef BASE_CHECK_H_
+#define BASE_CHECK_H_
+
+// Chromium has split CHECK/DCHECK out of logging.h into it's own header.
+// However, it's all still together in the GLOG header, so just include
+// it. This may cause problems when importing into Alcatraz as code using
+// anyone of these headers gets all the functions, not just the ones
+// represented by this header.
+#include "base/logging.h"
+
+#endif  // BASE_CHECK_H_

chromium_deps/base/check_op.h

@@ -0,0 +1,13 @@
+// Copyright 2020 Google LLC. All Rights Reserved.
+
+#ifndef BASE_CHECK_OP_H_
+#define BASE_CHECK_OP_H_
+
+// Chromium has split CHECK_EQ/etc. out of logging.h into it's own header.
+// However, it's all still together in the GLOG header, so just include
+// it. This may cause problems when importing into Alcatraz as code using
+// anyone of these headers gets all the functions, not just the ones
+// represented by this header.
+#include "base/logging.h"
+
+#endif  // BASE_CHECK_OP_H_

crypto_utils/aes_cbc_decryptor.cc

@@ -6,6 +6,7 @@
 #include <cstdint>
 #include <vector>
 
+#include "base/check.h"
 #include "base/logging.h"
 
 namespace widevine {

crypto_utils/aes_cbc_encryptor.cc

@@ -6,6 +6,7 @@
 #include <cstdint>
 #include <vector>
 
+#include "base/check.h"
 #include "base/logging.h"
 
 namespace widevine {

crypto_utils/aes_ctr_encryptor.cc

@@ -6,6 +6,7 @@
 #include <cstdint>
 #include <vector>
 
+#include "base/check.h"
 #include "base/logging.h"
 
 namespace widevine {

crypto_utils/crypto_util.cc

@@ -10,7 +10,7 @@
 
 #include "crypto_utils/crypto_util.h"
 
-#include "base/logging.h"
+#include "base/check.h"
 #include "third_party/boringssl/src/include/openssl/aes.h"
 #include "third_party/boringssl/src/include/openssl/cmac.h"
 #include "third_party/boringssl/src/include/openssl/evp.h"

crypto_utils/random_util.cc

@@ -8,7 +8,7 @@
 
 #include "crypto_utils/random_util.h"
 
-#include "base/logging.h"
+#include "base/check.h"
 #include "third_party/boringssl/src/include/openssl/rand.h"
 
 namespace widevine {

crypto_utils/rsa_key.cc

@@ -25,6 +25,7 @@
 
 #include "crypto_utils/rsa_key.h"
 
+#include "base/check.h"
 #include "base/logging.h"
 #include "crypto_utils/rsa_util.h"
 #include "crypto_utils/sha_util.h"

crypto_utils/rsa_util.cc

@@ -17,6 +17,7 @@
 #include <cstring>
 #include <memory>
 
+#include "base/check.h"
 #include "base/logging.h"
 #include "crypto_utils/private_key_util.h"
 #include "third_party/boringssl/src/include/openssl/pem.h"

impl/reference/aead_whitebox_impl.cc

@@ -5,6 +5,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/check_op.h"
 #include "base/logging.h"
 #include "crypto_utils/crypto_util.h"
 #include "impl/reference/memory_util.h"

impl/reference/license_whitebox_impl.cc

@@ -8,6 +8,8 @@
 #include <string>
 #include <vector>
 
+#include "base/check.h"
+#include "base/check_op.h"
 #include "base/logging.h"
 #include "cdm/protos/license_protocol.pb.h"
 #include "crypto_utils/aes_cbc_decryptor.h"
@@ -50,12 +52,12 @@ struct ContentKey {
 bool Decrypt(AesCbcDecryptor& decryptor,
              const std::string& iv,
              const std::string& encrypted,
-             std::vector<uint8_t>* decrypted) {
+             std::string* decrypted) {
   DCHECK_GE(decrypted->size(), encrypted.size());
-  return decryptor.Decrypt(reinterpret_cast<const uint8_t*>(iv.data()),
-                           iv.size(),
-                           reinterpret_cast<const uint8_t*>(encrypted.data()),
-                           encrypted.size(), decrypted->data());
+  return decryptor.Decrypt(
+      reinterpret_cast<const uint8_t*>(iv.data()), iv.size(),
+      reinterpret_cast<const uint8_t*>(encrypted.data()), encrypted.size(),
+      reinterpret_cast<uint8_t*>(&decrypted->front()));
 }
 
 bool IsOdkVersionSupported(uint16_t major_version, uint16_t minor_version) {
@@ -124,7 +126,7 @@ video_widevine::License_KeyContainer_SecurityLevel ExtractLevel(
 ContentKey CreateContentKey(
     video_widevine::License_KeyContainer_SecurityLevel level,
     bool is_hw_verified,
-    const std::vector<uint8_t>& key) {
+    const std::string& key) {
   ContentKey content_key;
 
   switch (level) {
@@ -149,7 +151,7 @@ ContentKey CreateContentKey(
   // it will only risk exposing it. We only have an entry for it so we can
   // handle errors correctly.
   if (content_key.allow_decrypt || content_key.allow_masked_decrypt) {
-    content_key.key = key;
+    content_key.key.assign(key.begin(), key.end());
   }
 
   return content_key;
@@ -515,9 +517,9 @@ WB_Result WB_License_ProcessLicenseResponse(WB_License_Whitebox* whitebox,
       decryptor.SetKey(reinterpret_cast<const uint8_t*>(decryption_key.data()),
                        decryption_key.size()));
 
-  std::string server_renewal_key;
-  std::string client_renewal_key;
   std::map<std::string, ContentKey> content_keys;
+  std::vector<std::string> server_renewal_keys;
+  std::vector<std::string> client_renewal_keys;
 
   // Even if |core_message| is provided, only ODK v16.5 and later support the
   // fields needed. If an older API is used, ignore it and use the protobuf
@@ -529,7 +531,8 @@ WB_Result WB_License_ProcessLicenseResponse(WB_License_Whitebox* whitebox,
     const std::string signing_key_iv =
         ExtractItem(parsed_license.enc_mac_keys_iv, message_str);
     if (!signing_key_encrypted.empty() && !signing_key_iv.empty()) {
-      std::vector<uint8_t> unwrapped_signing_key(signing_key_encrypted.size());
+      std::string unwrapped_signing_key(signing_key_encrypted);
+
       if (!Decrypt(decryptor, signing_key_iv, signing_key_encrypted,
                    &unwrapped_signing_key)) {
         DVLOG(1) << "Invalid parameter: Invalid enc_mac_keys.";
@@ -541,11 +544,10 @@ WB_Result WB_License_ProcessLicenseResponse(WB_License_Whitebox* whitebox,
         return WB_RESULT_INVALID_PARAMETER;
       }
 
-      const std::string signing_key(unwrapped_signing_key.begin(),
-                                    unwrapped_signing_key.end());
-      server_renewal_key = signing_key.substr(0, kSigningKeySizeBytes);
-      client_renewal_key =
-          signing_key.substr(kSigningKeySizeBytes, kSigningKeySizeBytes);
+      server_renewal_keys.push_back(
+          unwrapped_signing_key.substr(0, kSigningKeySizeBytes));
+      client_renewal_keys.push_back(unwrapped_signing_key.substr(
+          kSigningKeySizeBytes, kSigningKeySizeBytes));
     }
 
     // Now extract all the content keys.
@@ -555,7 +557,7 @@ WB_Result WB_License_ProcessLicenseResponse(WB_License_Whitebox* whitebox,
 
       const std::string iv = ExtractItem(key.key_data_iv, message_str);
       const std::string wrapped_key = ExtractItem(key.key_data, message_str);
-      std::vector<uint8_t> unwrapped_key(wrapped_key.size());
+      std::string unwrapped_key(wrapped_key);
 
       if (!Decrypt(decryptor, iv, wrapped_key, &unwrapped_key)) {
         DVLOG(1) << "Invalid parameter: Invalid key.key_data.";
@@ -604,7 +606,7 @@ WB_Result WB_License_ProcessLicenseResponse(WB_License_Whitebox* whitebox,
       }
 
       const std::string wrapped_key = key.key();
-      std::vector<uint8_t> unwrapped_key(wrapped_key.size());
+      std::string unwrapped_key(wrapped_key);
 
       if (!Decrypt(decryptor, key.iv(), wrapped_key, &unwrapped_key)) {
         // The input has to be a specific length, so if it is not, it means that
@@ -619,11 +621,10 @@ WB_Result WB_License_ProcessLicenseResponse(WB_License_Whitebox* whitebox,
           return WB_RESULT_INVALID_PARAMETER;
         }
 
-        const std::string signing_key(unwrapped_key.begin(),
-                                      unwrapped_key.end());
-        server_renewal_key = signing_key.substr(0, kSigningKeySizeBytes);
-        client_renewal_key =
-            signing_key.substr(kSigningKeySizeBytes, kSigningKeySizeBytes);
+        server_renewal_keys.push_back(
+            unwrapped_key.substr(0, kSigningKeySizeBytes));
+        client_renewal_keys.push_back(
+            unwrapped_key.substr(kSigningKeySizeBytes, kSigningKeySizeBytes));
       } else if (key.type() == KeyContainer::CONTENT) {
         constexpr size_t kContentKeySizeBytes = 16;
 
@@ -642,10 +643,19 @@ WB_Result WB_License_ProcessLicenseResponse(WB_License_Whitebox* whitebox,
     }
   }
 
+  DCHECK_EQ(server_renewal_keys.size(), client_renewal_keys.size());
+  if (server_renewal_keys.size() > 1) {
+    return WB_RESULT_INVALID_PARAMETER;
+  }
+  // Add an empty string so that we can always reference a valid string. By
+  // adding it last, a valid key will get priority over this fallback value.
+  server_renewal_keys.push_back("");
+  client_renewal_keys.push_back("");
+
   // Copy the loaded state over to the white-box instance now that we know we
   // have a valid state.
-  whitebox->server_signing_key.swap(server_renewal_key);
-  whitebox->client_signing_key.swap(client_renewal_key);
+  whitebox->server_signing_key.swap(server_renewal_keys[0]);
+  whitebox->client_signing_key.swap(client_renewal_keys[0]);
   whitebox->content_keys.swap(content_keys);
 
   return WB_RESULT_OK;