media: drop dependency on deprecated //crypto APIs [1/n]

As part of removing the deprecated crypto::SymmetricKey type, this
change:
* Adds an overload of media::AesCbcCrypto::Initialize that takes a byte
  span for the key rather than a crypto::SymmetricKey
* Migrates the AesCbcCrypto unit tests to use the new initializer rather
  than the old one, which allows significant simplification
* Migrates the production call site of AesCbcCrypto::Initialize to use a
  byte span for the key

Subsequent changes in this series will:
* Migrate the CdmAdapter unit tests to the new initializer
* Excise crypto::SymmetricKey from the rest of the CDM code

Bug: 372283556
Change-Id: I8f79741e7a2c529130428457dfffc3e183079093
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6039361
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Elly FJ <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387712}

media/cdm/aes_cbc_crypto.cc

@@ -37,14 +37,14 @@ namespace media {
 AesCbcCrypto::AesCbcCrypto() = default;
 AesCbcCrypto::~AesCbcCrypto() = default;
 
-bool AesCbcCrypto::Initialize(const crypto::SymmetricKey& key,
+bool AesCbcCrypto::Initialize(base::span<const uint8_t> key,
                               base::span<const uint8_t> iv) {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
   // This uses AES-CBC-128, so the key must be 128 bits.
   const EVP_CIPHER* cipher = EVP_aes_128_cbc();
-  const uint8_t* key_data = reinterpret_cast<const uint8_t*>(key.key().data());
-  if (key.key().length() != EVP_CIPHER_key_length(cipher)) {
+  const uint8_t* key_data = reinterpret_cast<const uint8_t*>(key.data());
+  if (key.size() != EVP_CIPHER_key_length(cipher)) {
     DVLOG(1) << "Key length is incorrect.";
     return false;
   }
@@ -68,6 +68,11 @@ bool AesCbcCrypto::Initialize(const crypto::SymmetricKey& key,
   return true;
 }
 
+bool AesCbcCrypto::Initialize(const crypto::SymmetricKey& key,
+                              base::span<const uint8_t> iv) {
+  return Initialize(base::as_byte_span(key.key()), iv);
+}
+
 bool AesCbcCrypto::Decrypt(base::span<const uint8_t> encrypted_data,
                            uint8_t* decrypted_data) {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);

media/cdm/aes_cbc_crypto.h

@@ -33,6 +33,10 @@ class MEDIA_EXPORT AesCbcCrypto {
 
   // Initializes the encryptor using |key| and |iv|. Returns false if either
   // the key or the initialization vector cannot be used.
+  bool Initialize(base::span<const uint8_t> key, base::span<const uint8_t> iv);
+
+  // Deprecated initializer that takes a crypto::SymmetricKey. Do not add new
+  // uses of this - pass the key as a byte span instead.
   bool Initialize(const crypto::SymmetricKey& key,
                   base::span<const uint8_t> iv);
 

media/cdm/aes_cbc_crypto_unittest.cc

@@ -10,8 +10,7 @@
 #include "base/containers/span.h"
 #include "base/containers/to_vector.h"
 #include "base/memory/raw_span.h"
-#include "crypto/encryptor.h"
-#include "crypto/symmetric_key.h"
+#include "crypto/aes_cbc.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/decrypt_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -26,125 +25,94 @@ namespace {
 
 // Pattern decryption uses 16-byte blocks.
 constexpr size_t kBlockSize = 16;
+constexpr size_t kKeySize = 16;
 
-// Keys and IV have to be 128 bits.
-const uint8_t kKey1[] = {0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
-                         0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13};
-static_assert(std::size(kKey1) == 128 / 8, "kKey1 must be 128 bits");
+const std::array<uint8_t, kKeySize> kKey1{0x04, 0x05, 0x06, 0x07, 0x08, 0x09,
+                                          0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+                                          0x10, 0x11, 0x12, 0x13};
 
-const uint8_t kKey2[] = {0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
-                         0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b};
-static_assert(std::size(kKey2) == 128 / 8, "kKey2 must be 128 bits");
+const std::array<uint8_t, kKeySize> kKey2{0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11,
+                                          0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+                                          0x18, 0x19, 0x1a, 0x1b};
 
-const uint8_t kIv[] = {0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
-                       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
-static_assert(std::size(kIv) == 128 / 8, "kIv must be 128 bits");
+const std::array<uint8_t, kBlockSize> kIv{0x20, 0x21, 0x22, 0x23, 0x24, 0x25,
+                                          0x26, 0x27, 0x00, 0x00, 0x00, 0x00,
+                                          0x00, 0x00, 0x00, 0x00};
 
-const uint8_t kOneBlock[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h',
-                             'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p'};
-static_assert(std::size(kOneBlock) == kBlockSize, "kOneBlock not block sized");
-
-std::string MakeString(const std::vector<uint8_t>& chars) {
-  return std::string(chars.begin(), chars.end());
-}
+const std::array<uint8_t, kBlockSize> kOneBlock{'a', 'b', 'c', 'd', 'e', 'f',
+                                                'g', 'h', 'i', 'j', 'k', 'l',
+                                                'm', 'n', 'o', 'p'};
 
 // Returns a std::vector<uint8_t> containing |count| copies of |input|.
-std::vector<uint8_t> Repeat(const std::vector<uint8_t>& input, size_t count) {
+std::vector<uint8_t> Repeat(base::span<const uint8_t> input, size_t count) {
   std::vector<uint8_t> result;
   for (size_t i = 0; i < count; ++i)
     result.insert(result.end(), input.begin(), input.end());
   return result;
 }
 
+// Encrypt |original| using AES-CBC encryption with |key| and |iv|.
+std::vector<uint8_t> Encrypt(
+    base::span<const uint8_t> plaintext,
+    base::span<const uint8_t> key,
+    base::span<const uint8_t, crypto::aes_cbc::kBlockSize> iv) {
+  auto ciphertext = crypto::aes_cbc::Encrypt(key, iv, plaintext);
+
+  // Strip off the PKCS#5 padding block.
+  ciphertext.resize(plaintext.size());
+
+  return ciphertext;
+}
+
 }  // namespace
 
-class AesCbcCryptoTest : public testing::Test {
- public:
-  AesCbcCryptoTest()
-      : key1_(crypto::SymmetricKey::Import(
-            crypto::SymmetricKey::AES,
-            std::string(std::begin(kKey1), std::end(kKey1)))),
-        key2_(crypto::SymmetricKey::Import(
-            crypto::SymmetricKey::AES,
-            std::string(std::begin(kKey2), std::end(kKey2)))),
-        iv_(kIv),
-        one_block_(base::ToVector(kOneBlock)) {}
+using AesCbcCryptoTest = ::testing::Test;
 
-  // Encrypt |original| using AES-CBC encryption with |key| and |iv|.
-  std::vector<uint8_t> Encrypt(const std::vector<uint8_t>& original,
-                               const crypto::SymmetricKey& key,
-                               base::span<const uint8_t> iv) {
-    // This code uses crypto::Encryptor to encrypt |original| rather than
-    // calling EVP_EncryptInit_ex() / EVP_EncryptUpdate() / etc. This is done
-    // for simplicity, as the crypto:: code wraps all the calls up nicely.
-    // However, for AES-CBC encryption, the crypto:: code does add padding to
-    // the output, which is simply stripped off.
-    crypto::Encryptor encryptor;
-    std::string iv_as_string(std::begin(iv), std::end(iv));
-    EXPECT_TRUE(encryptor.Init(&key, crypto::Encryptor::CBC, iv_as_string));
-
-    std::string ciphertext;
-    EXPECT_TRUE(encryptor.Encrypt(MakeString(original), &ciphertext));
-
-    // CBC encryption adds a block of padding at the end, so discard it.
-    EXPECT_GT(ciphertext.size(), original.size());
-    ciphertext.resize(original.size());
-
-    return std::vector<uint8_t>(ciphertext.begin(), ciphertext.end());
-  }
-
-  // Constants for testing.
-  std::unique_ptr<crypto::SymmetricKey> key1_;
-  std::unique_ptr<crypto::SymmetricKey> key2_;
-  base::raw_span<const uint8_t> iv_;
-  const std::vector<uint8_t> one_block_;
-};
-
-TEST_F(AesCbcCryptoTest, OneBlock) {
-  auto encrypted_block = Encrypt(one_block_, *key1_, iv_);
+TEST(AesCbcCryptoTest, OneBlock) {
+  auto encrypted_block = Encrypt(kOneBlock, kKey1, kIv);
   EXPECT_EQ(kBlockSize, encrypted_block.size());
 
   AesCbcCrypto crypto;
-  EXPECT_TRUE(crypto.Initialize(*key1_, iv_));
+  EXPECT_TRUE(crypto.Initialize(kKey1, kIv));
 
   std::vector<uint8_t> output(encrypted_block.size());
   EXPECT_TRUE(crypto.Decrypt(encrypted_block, output.data()));
-  EXPECT_EQ(output, one_block_);
+  EXPECT_EQ(base::as_byte_span(output), kOneBlock);
 }
 
-TEST_F(AesCbcCryptoTest, WrongKey) {
-  auto encrypted_block = Encrypt(one_block_, *key1_, iv_);
+TEST(AesCbcCryptoTest, WrongKey) {
+  auto encrypted_block = Encrypt(kOneBlock, kKey1, kIv);
   EXPECT_EQ(kBlockSize, encrypted_block.size());
 
   // Use |key2_| when trying to decrypt.
   AesCbcCrypto crypto;
-  EXPECT_TRUE(crypto.Initialize(*key2_, iv_));
+  EXPECT_TRUE(crypto.Initialize(kKey2, kIv));
 
   std::vector<uint8_t> output(encrypted_block.size());
   EXPECT_TRUE(crypto.Decrypt(encrypted_block, output.data()));
-  EXPECT_NE(output, one_block_);
+  EXPECT_NE(base::as_byte_span(output), kOneBlock);
 }
 
-TEST_F(AesCbcCryptoTest, WrongIV) {
-  auto encrypted_block = Encrypt(one_block_, *key1_, iv_);
+TEST(AesCbcCryptoTest, WrongIV) {
+  auto encrypted_block = Encrypt(kOneBlock, kKey1, kIv);
   EXPECT_EQ(kBlockSize, encrypted_block.size());
 
   // Use a different IV when trying to decrypt.
   AesCbcCrypto crypto;
-  std::vector<uint8_t> alternate_iv(iv_.size(), 'a');
-  EXPECT_TRUE(crypto.Initialize(*key1_, alternate_iv));
+  std::vector<uint8_t> alternate_iv(kIv.size(), 'a');
+  EXPECT_TRUE(crypto.Initialize(kKey1, alternate_iv));
 
   std::vector<uint8_t> output(encrypted_block.size());
   EXPECT_TRUE(crypto.Decrypt(encrypted_block, output.data()));
-  EXPECT_NE(output, one_block_);
+  EXPECT_NE(base::as_byte_span(output), kOneBlock);
 }
 
-TEST_F(AesCbcCryptoTest, PartialBlock) {
-  auto encrypted_block = Encrypt(one_block_, *key1_, iv_);
+TEST(AesCbcCryptoTest, PartialBlock) {
+  auto encrypted_block = Encrypt(kOneBlock, kKey1, kIv);
   EXPECT_EQ(kBlockSize, encrypted_block.size());
 
   AesCbcCrypto crypto;
-  EXPECT_TRUE(crypto.Initialize(*key2_, iv_));
+  EXPECT_TRUE(crypto.Initialize(kKey2, kIv));
 
   // Try to decrypt less than a full block.
   std::vector<uint8_t> output(encrypted_block.size());
@@ -153,25 +121,25 @@ TEST_F(AesCbcCryptoTest, PartialBlock) {
       output.data()));
 }
 
-TEST_F(AesCbcCryptoTest, MultipleBlocks) {
-  // Encrypt 10 copies of |one_block_| together.
+TEST(AesCbcCryptoTest, MultipleBlocks) {
+  // Encrypt 10 copies of |kOneBlock| together.
   constexpr size_t kNumBlocksInData = 10;
   auto encrypted_block =
-      Encrypt(Repeat(one_block_, kNumBlocksInData), *key2_, iv_);
+      Encrypt(Repeat(kOneBlock, kNumBlocksInData), kKey2, kIv);
   ASSERT_EQ(kNumBlocksInData * kBlockSize, encrypted_block.size());
 
   AesCbcCrypto crypto;
-  EXPECT_TRUE(crypto.Initialize(*key2_, iv_));
+  EXPECT_TRUE(crypto.Initialize(kKey2, kIv));
 
   std::vector<uint8_t> output(encrypted_block.size());
   EXPECT_TRUE(crypto.Decrypt(encrypted_block, output.data()));
-  EXPECT_EQ(output, Repeat(one_block_, kNumBlocksInData));
+  EXPECT_EQ(output, Repeat(kOneBlock, kNumBlocksInData));
 }
 
 // As the code in aes_cbc_crypto.cc relies on decrypting the data block by
 // block, ensure that the crypto routines work the same way whether it
 // decrypts one block at a time or all the blocks in one call.
-TEST_F(AesCbcCryptoTest, BlockDecryptionWorks) {
+TEST(AesCbcCryptoTest, BlockDecryptionWorks) {
   constexpr size_t kNumBlocksInData = 5;
   std::vector<uint8_t> data = {1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 2, 2,
                                3, 3, 3, 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4,
@@ -179,12 +147,12 @@ TEST_F(AesCbcCryptoTest, BlockDecryptionWorks) {
                                7, 7, 7, 7, 7, 7, 7, 7, 8, 8, 8, 8, 8, 8, 8, 8,
                                9, 9, 9, 9, 9, 9, 9, 9, 0, 0, 0, 0, 0, 0, 0, 0};
   ASSERT_EQ(data.size(), kNumBlocksInData * kBlockSize);
-  auto encrypted_data = Encrypt(data, *key1_, iv_);
+  auto encrypted_data = Encrypt(data, kKey1, kIv);
 
   // Decrypt |encrypted_data| in one pass.
   {
     AesCbcCrypto crypto;
-    EXPECT_TRUE(crypto.Initialize(*key1_, iv_));
+    EXPECT_TRUE(crypto.Initialize(kKey1, kIv));
 
     std::vector<uint8_t> output(kNumBlocksInData * kBlockSize);
     EXPECT_TRUE(crypto.Decrypt(encrypted_data, output.data()));
@@ -194,7 +162,7 @@ TEST_F(AesCbcCryptoTest, BlockDecryptionWorks) {
   // Repeat but call Decrypt() once for each block.
   {
     AesCbcCrypto crypto;
-    EXPECT_TRUE(crypto.Initialize(*key1_, iv_));
+    EXPECT_TRUE(crypto.Initialize(kKey1, kIv));
 
     std::vector<uint8_t> output(kNumBlocksInData * kBlockSize);
     auto input = base::make_span(encrypted_data);

media/cdm/cbcs_decryptor.cc

@@ -43,8 +43,9 @@ bool DecryptWithPattern(const crypto::SymmetricKey& key,
                         uint8_t* output_data) {
   // The AES_CBC decryption is reset for each subsample.
   AesCbcCrypto aes_cbc_crypto;
-  if (!aes_cbc_crypto.Initialize(key, iv))
+  if (!aes_cbc_crypto.Initialize(base::as_byte_span(key.key()), iv)) {
     return false;
+  }
 
   // |total_blocks| is the number of blocks in the buffer, ignoring any
   // partial block at the end. |remaining_bytes| is the number of bytes