From 82951b01efefc55ab9b8353625a494f4e27dccbf Mon Sep 17 00:00:00 2001 From: "John W. Bruce" Date: Wed, 4 Mar 2020 12:11:29 -0800 Subject: [PATCH] Treat the (0,0) Pattern as 'cbcs' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (This is a merge of http://go/wvgerrit/94928.) In OEMCrypto v16, we dropped support for 'cens' and 'cbc1'. However, we did not redefine the pattern (0,0) to be a valid pattern for 'cbcs', even though it was no longer being used to signal 'cbc1'. Instead, we made the CDM reject CTR with a pattern ('cens') and CBC with a (0,0) pattern ('cbc1') to mirror the behavior of OEMCrypto v16. However, some apps have been using 'cbc1' mode to decrypt audio in 'cbcs' content. This is normally not possible but is possible for a subset of content. Furthermore, it is easy to do by accident because of the way most packagers package 'cbcs' audio and the special significance Widevine has historically given the (0,0) pattern. This patch updates the CDM to not reject CBC with a (0,0) pattern but instead treat it as 'cbcs' content. To decrypt it correctly, the pattern is treated specially inside the CDM core and converted to the recommended equivalent pattern — (10,0) — before passing the content to OEMCrypto. For more specifics, please see the design doc: http://go/vclfg Bug: 150219982 Test: ExoPlayer Demo App 'cbcs' Content Test: GTS 'cbcs' Content Change-Id: I334ff15db5f7b7d62040a036ba6d17515c3caee4 --- libwvdrmengine/cdm/core/src/crypto_session.cpp | 7 +++++++ libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp | 5 ----- .../mediacrypto/src_hidl/WVCryptoPlugin.cpp | 5 ----- .../mediacrypto/test/WVCryptoPlugin_test.cpp | 12 +----------- .../test/legacy_src/WVCryptoPlugin_test.cpp | 12 +----------- 5 files changed, 9 insertions(+), 32 deletions(-) diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index 49266886..7b0dd0e4 100644 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -1468,6 +1468,13 @@ CdmResponseType CryptoSession::Decrypt( // Convert the pattern descriptor OEMCrypto_CENCEncryptPatternDesc oec_pattern{params.pattern.encrypt_blocks, params.pattern.skip_blocks}; + // TODO(b/146581957): Remove this workaround once OEMCrypto treats (0,0) as + // 'cbcs' instead of 'cbc1'. + if (params.cipher_mode == kCipherModeCbc && oec_pattern.encrypt == 0 && + oec_pattern.skip == 0) { + // (10, 0) is the preferred pattern for decrypting every block in 'cbcs' + oec_pattern.encrypt = 10; + } // Check if a key needs to be selected if (is_any_sample_protected) { diff --git a/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp b/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp index 2fd3ecd8..8eea029f 100644 --- a/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp +++ b/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp @@ -111,11 +111,6 @@ ssize_t WVCryptoPlugin::decrypt(bool secure, const uint8_t key[KEY_ID_SIZE], errorDetailMsg->setTo( "The 'cens' schema is not supported by Widevine CDM."); return kErrorUnsupportedCrypto; - } else if (mode == kMode_AES_CBC && - (pattern.mEncryptBlocks == 0 && pattern.mSkipBlocks == 0)) { - errorDetailMsg->setTo( - "The 'cbc1' schema is not supported by Widevine CDM."); - return kErrorUnsupportedCrypto; } // Convert parameters to the form the CDM wishes to consume them in. diff --git a/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp b/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp index 57357c98..14094c73 100644 --- a/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp +++ b/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp @@ -185,11 +185,6 @@ Return WVCryptoPlugin::decrypt_1_2( _hidl_cb(Status_V1_2::BAD_VALUE, 0, "The 'cens' schema is not supported by Widevine CDM."); return Void(); - } else if (mode == Mode::AES_CBC && - (pattern.encryptBlocks == 0 && pattern.skipBlocks == 0)) { - _hidl_cb(Status_V1_2::BAD_VALUE, 0, - "The 'cbc1' schema is not supported by Widevine CDM."); - return Void(); } // Convert parameters to the form the CDM wishes to consume them in. diff --git a/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp b/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp index b74627a4..2d841bbc 100644 --- a/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp +++ b/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp @@ -339,7 +339,7 @@ TEST_F(WVCryptoPluginTest, AttemptsToDecrypt) { "WVCryptoPlugin reported a detailed error message."; } -TEST_F(WVCryptoPluginTest, RejectsCensAndCbc1) { +TEST_F(WVCryptoPluginTest, RejectsCens) { android::sp> cdm = new StrictMock(); constexpr size_t kSubSampleCount = 2; @@ -380,7 +380,6 @@ TEST_F(WVCryptoPluginTest, RejectsCensAndCbc1) { static_cast(destination->unsecurePointer())); ASSERT_NE(pDest, nullptr); - Pattern noPattern = { 0, 0 }; Pattern recommendedPattern = { 1, 9 }; // Provide the expected behavior for IsOpenSession @@ -409,15 +408,6 @@ TEST_F(WVCryptoPluginTest, RejectsCensAndCbc1) { EXPECT_EQ(status, Status::BAD_VALUE); EXPECT_EQ(bytesWritten, 0); }); - - plugin.decrypt( - false, hidl_array(keyId), hidl_array(iv), - Mode::AES_CBC, noPattern, hSubSamples, sourceBuffer, 0, hDestination, - [&](Status status, uint32_t bytesWritten, - hidl_string /* errorDetailMessage */) { - EXPECT_EQ(status, Status::BAD_VALUE); - EXPECT_EQ(bytesWritten, 0); - }); } TEST_F(WVCryptoPluginTest, CommunicatesSecureBufferRequest) { diff --git a/libwvdrmengine/mediacrypto/test/legacy_src/WVCryptoPlugin_test.cpp b/libwvdrmengine/mediacrypto/test/legacy_src/WVCryptoPlugin_test.cpp index 04f7de34..d23cc9ee 100644 --- a/libwvdrmengine/mediacrypto/test/legacy_src/WVCryptoPlugin_test.cpp +++ b/libwvdrmengine/mediacrypto/test/legacy_src/WVCryptoPlugin_test.cpp @@ -221,7 +221,7 @@ TEST_F(WVCryptoPluginTest, AttemptsToDecrypt) { "WVCryptoPlugin reported a detailed error message."; } -TEST_F(WVCryptoPluginTest, RejectsCensAndCbc1) { +TEST_F(WVCryptoPluginTest, RejectsCens) { android::sp> cdm = new StrictMock(); constexpr size_t kSubSampleCount = 2; @@ -244,7 +244,6 @@ TEST_F(WVCryptoPluginTest, RejectsCensAndCbc1) { fread(inputData, sizeof(uint8_t), kDataSize, fp); fclose(fp); - android::CryptoPlugin::Pattern noPattern = { 0, 0 }; android::CryptoPlugin::Pattern recommendedPattern = { 1, 9 }; // Provide the expected behavior for IsOpenSession @@ -267,15 +266,6 @@ TEST_F(WVCryptoPluginTest, RejectsCensAndCbc1) { "WVCryptoPlugin did not return an error for 'cens'."; EXPECT_NE(errorDetailMessage.size(), 0u) << "WVCryptoPlugin did not report a detailed error message for 'cens'."; - - res = plugin.decrypt(false, keyId, iv, CryptoPlugin::kMode_AES_CBC, - noPattern, inputData, subSamples, kSubSampleCount, - outputData, &errorDetailMessage); - - EXPECT_EQ(res, kErrorUnsupportedCrypto) << - "WVCryptoPlugin did not return an error for 'cbc1'."; - EXPECT_NE(errorDetailMessage.size(), 0u) << - "WVCryptoPlugin did not report a detailed error message for 'cbc1'."; } TEST_F(WVCryptoPluginTest, CommunicatesSecureBufferRequest) {