From 82951b01efefc55ab9b8353625a494f4e27dccbf Mon Sep 17 00:00:00 2001 From: "John W. Bruce" Date: Wed, 4 Mar 2020 12:11:29 -0800 Subject: [PATCH 1/4] 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) { From 1f1ba94a61a76a92dd8a18419f3abfde950a7ce3 Mon Sep 17 00:00:00 2001 From: "John W. Bruce" Date: Wed, 4 Mar 2020 12:20:58 -0800 Subject: [PATCH 2/4] Cache Max Subsample Size (This is a merge of http://go/wvgerrit/95003.) To reduce the number of OEMCrypto calls on the decrypt path, the maximum subsample size will now be cached after the first call to retrieve it. Bug: 150018606 Test: Android Unit Tests Test: CE CDM Unit Tests Test: ExoPlayer high-bitrate playback on OEC v15 Change-Id: I0b5d38d8a082c0a127d2a47f112b76c64085bddb --- .../cdm/core/include/crypto_session.h | 3 +- .../cdm/core/src/crypto_session.cpp | 42 ++++++++++++------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/crypto_session.h b/libwvdrmengine/cdm/core/include/crypto_session.h index 747c9ec5..ee891692 100644 --- a/libwvdrmengine/cdm/core/include/crypto_session.h +++ b/libwvdrmengine/cdm/core/include/crypto_session.h @@ -300,7 +300,7 @@ class CryptoSession { CdmResponseType GetSystemIdInternal(uint32_t* system_id); CdmResponseType GenerateRsaSignature(const std::string& message, std::string* signature); - bool GetMaxSubsampleRegionSize(size_t* max); + size_t GetMaxSubsampleRegionSize(); bool SetDestinationBufferType(); @@ -429,6 +429,7 @@ class CryptoSession { static std::atomic request_id_index_source_; uint32_t api_version_; + size_t max_subsample_region_size_; // Stores the most recent error code returned from a call to // OEMCrypto_DecryptCENC. This is used to reduce the total number of diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index 49266886..ede25310 100644 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -80,7 +80,7 @@ static_assert(ArraySize(kMaxSubsampleRegionSizes) == "The kMaxSubsampleRegionSizes table needs to be updated to " "reflect the supported range of resource rating tiers."); -constexpr size_t kDefaultMaximumChunkSize = 100 * KiB; +constexpr size_t kDefaultMaxSubsampleRegionSize = kMaxSubsampleRegionSizes[0]; // This maps a few common OEMCryptoResult to CdmResponseType. Many mappings // are not universal but are OEMCrypto method specific. Those will be @@ -217,7 +217,8 @@ CryptoSession::CryptoSession(metrics::CryptoMetrics* metrics) requested_security_level_(kLevelDefault), usage_support_type_(kUnknownUsageSupport), usage_table_header_(nullptr), - api_version_(0) { + api_version_(0), + max_subsample_region_size_(0) { assert(metrics); Init(); life_span_.Start(); @@ -1331,14 +1332,28 @@ CdmResponseType CryptoSession::GenerateRsaSignature(const std::string& message, "OEMCrypto_GenerateRSASignature"); } -bool CryptoSession::GetMaxSubsampleRegionSize(size_t* max) { - uint32_t tier = 0; - if (!GetResourceRatingTier(&tier)) return false; - // Subtract RESOURCE_RATING_TIER_MIN to get a 0-based index into the table. - const uint32_t index = tier - RESOURCE_RATING_TIER_MIN; - if (index >= ArraySize(kMaxSubsampleRegionSizes)) return false; - *max = kMaxSubsampleRegionSizes[index]; - return true; +size_t CryptoSession::GetMaxSubsampleRegionSize() { + // If we haven't cached the answer yet, fetch it from OEMCrypto. + if (max_subsample_region_size_ == 0) { + uint32_t tier = 0; + if (GetResourceRatingTier(&tier)) { + // Subtract RESOURCE_RATING_TIER_MIN to get a 0-based index into the + // table. + const uint32_t index = tier - RESOURCE_RATING_TIER_MIN; + if (index < ArraySize(kMaxSubsampleRegionSizes)) { + max_subsample_region_size_ = kMaxSubsampleRegionSizes[index]; + } + } + + // If something went wrong, use the default. + if (max_subsample_region_size_ == 0) { + LOGW("Unable to get maximum subsample region size. Defaulting to %zu.", + kDefaultMaxSubsampleRegionSize); + max_subsample_region_size_ = kDefaultMaxSubsampleRegionSize; + } + } + + return max_subsample_region_size_; } CdmResponseType CryptoSession::Decrypt( @@ -2723,12 +2738,7 @@ OEMCryptoResult CryptoSession::DecryptSample( OEMCryptoResult CryptoSession::LegacyDecrypt( const OEMCrypto_SampleDescription& sample, CdmCipherMode cipher_mode, const OEMCrypto_CENCEncryptPatternDesc& pattern) { - size_t max_chunk_size; - if (!GetMaxSubsampleRegionSize(&max_chunk_size)) { - LOGW("Unable to get maximum subsample region size. Defaulting to 100 KiB."); - max_chunk_size = kDefaultMaximumChunkSize; - } - + const size_t max_chunk_size = GetMaxSubsampleRegionSize(); OEMCryptoResult sts = OEMCrypto_ERROR_NOT_IMPLEMENTED; // We can be sure this is only called with one subsample containing one From 96dc665cd5b0043903adae6a03416eaf55534b0e Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Wed, 4 Mar 2020 15:14:01 -0800 Subject: [PATCH 3/4] Remove ODK from unused makefiles Merge from Widevine repo of http://go/wvgerrit/95087 The ODK library is used in OEMCrypto, and in test code, but it is not needed in the CDM layer. As such, it can be removed from the Android.mk for cdm libraries. Bug: 150809634 Test: unit tests Change-Id: If29458e7d3d940f9a383d77e5082e7388e19c32f --- libwvdrmengine/Android.mk | 2 -- libwvdrmengine/cdm/Android.bp | 1 - libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp | 1 - 3 files changed, 4 deletions(-) diff --git a/libwvdrmengine/Android.mk b/libwvdrmengine/Android.mk index 31f2e751..6f258625 100644 --- a/libwvdrmengine/Android.mk +++ b/libwvdrmengine/Android.mk @@ -168,7 +168,6 @@ LOCAL_STATIC_LIBRARIES := \ libwvdrmcryptoplugin \ libwvdrmdrmplugin \ libwvlevel3 \ - libwv_odk \ LOCAL_SHARED_LIBRARIES := \ libbase \ @@ -230,7 +229,6 @@ LOCAL_STATIC_LIBRARIES := \ libwvdrmcryptoplugin_hidl \ libwvdrmdrmplugin_hidl \ libwvlevel3 \ - libwv_odk \ LOCAL_SHARED_LIBRARIES := \ android.hardware.drm@1.0 \ diff --git a/libwvdrmengine/cdm/Android.bp b/libwvdrmengine/cdm/Android.bp index a095eec5..4ad49ab4 100644 --- a/libwvdrmengine/cdm/Android.bp +++ b/libwvdrmengine/cdm/Android.bp @@ -17,7 +17,6 @@ cc_library_static { "vendor/widevine/libwvdrmengine/cdm/util/include", "vendor/widevine/libwvdrmengine/cdm/include", "vendor/widevine/libwvdrmengine/oemcrypto/include", - "vendor/widevine/libwvdrmengine/oemcrypto/odk/include", "external/jsmn", "external/protobuf/src", ], diff --git a/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp b/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp index 6d6baea2..e35c163d 100644 --- a/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp +++ b/libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp @@ -33,7 +33,6 @@ #include "license_protocol.pb.h" #include "log.h" #include "metrics_collections.h" -#include "odk.h" #include "properties.h" #include "wv_cdm_constants.h" From 037918aa493ef6b2b664c6fb4461ccb43fb94224 Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Mon, 9 Mar 2020 13:14:56 -0700 Subject: [PATCH 4/4] Update tests for license release Merge from Widevine repo of http://go/wvgerrit/95403 This updates the test code CreateDefaultResponse to make sure that license releases do not have a core message, and that the key control block is correctly set for renewals. Also, the unit test OEMCryptoUsageTableTest.TimingTest is changed to only a license release when the license is inactive. If the license is still active, then the license is loaded before generating a usage report. Test: Ran full unit tests Bug: 151092673 Change-Id: I7c01fd17f9b66e88ab3c57aa0f3d40740f13507c --- .../oemcrypto/test/oec_session_util.cpp | 11 +++----- .../oemcrypto/test/oemcrypto_test.cpp | 28 ++++++++----------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp index 1dd11dc0..b3844e66 100644 --- a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp +++ b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp @@ -844,7 +844,7 @@ void RenewalRoundTrip::FillAndVerifyCoreRequest( } void RenewalRoundTrip::CreateDefaultResponse() { - if (license_messages_->api_version() < kCoreMessagesAPI) { + if (license_messages_->api_version() < kCoreMessagesAPI || is_release_) { uint32_t control = 0; uint32_t nonce = 0; // If this is a v15 device, and a v15 license, and the license used a nonce, @@ -859,12 +859,9 @@ void RenewalRoundTrip::CreateDefaultResponse() { constexpr size_t index = 0; response_data_.keys[index].key_id_length = 0; response_data_.keys[index].key_id[0] = '\0'; - std::string kcVersion = - "kc" + std::to_string(core_request_.api_major_version); - if (global_features.api_version < kCoreMessagesAPI) { - // For v15 or earlier devices, we use the api of the device. - kcVersion = "kc" + std::to_string(global_features.api_version); - } + const uint32_t renewal_api = + std::max(core_request_.api_major_version, 15u); + std::string kcVersion = "kc" + std::to_string(renewal_api); memcpy(response_data_.keys[index].control.verification, kcVersion.c_str(), 4); const uint32_t duration = static_cast( diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp b/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp index 0f86912e..81450489 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp @@ -1694,15 +1694,15 @@ class OEMCryptoSessionTestDecryptWithHDCP : public OEMCryptoSessionTests, // reported if OEMCrypto_WARNING_MIXED_OUTPUT_PROTECTION is expected. ASSERT_NO_FATAL_FAILURE( s.TestDecryptCTR(true, OEMCrypto_WARNING_MIXED_OUTPUT_PROTECTION)) - << "Failed when current HDCP = " << HDCPCapabilityAsString(current) - << ", maximum HDCP = " << HDCPCapabilityAsString(maximum) - << ", license HDCP = " << HDCPCapabilityAsString(version); + << "Failed when current HDCP = " << HDCPCapabilityAsString(current) + << ", maximum HDCP = " << HDCPCapabilityAsString(maximum) + << ", license HDCP = " << HDCPCapabilityAsString(version); } else { ASSERT_NO_FATAL_FAILURE( s.TestDecryptCTR(true, OEMCrypto_ERROR_INSUFFICIENT_HDCP)) - << "Failed when current HDCP = " << HDCPCapabilityAsString(current) - << ", maximum HDCP = " << HDCPCapabilityAsString(maximum) - << ", license HDCP = " << HDCPCapabilityAsString(version); + << "Failed when current HDCP = " << HDCPCapabilityAsString(current) + << ", maximum HDCP = " << HDCPCapabilityAsString(maximum) + << ", license HDCP = " << HDCPCapabilityAsString(version); } } else { ASSERT_NO_FATAL_FAILURE(s.TestDecryptCTR(true, OEMCrypto_SUCCESS)) @@ -2804,7 +2804,8 @@ TEST_F(OEMCryptoLoadsCertificate, CertificateProvisionBadRSAKey) { // Test that RewrapDeviceRSAKey verifies the RSA key is valid. // TODO(b/144186970): This test should also run on Prov 3.0 devices. -TEST_F(OEMCryptoLoadsCertificate, CertificateProvisionBadRSAKeyKeyboxTestAPI16) { +TEST_F(OEMCryptoLoadsCertificate, + CertificateProvisionBadRSAKeyKeyboxTestAPI16) { Session s; ProvisioningRoundTrip provisioning_messages(&s, encoded_rsa_key_); provisioning_messages.PrepareSession(keybox_); @@ -6065,17 +6066,10 @@ TEST_P(OEMCryptoUsageTableTest, TimingTest) { ASSERT_NO_FATAL_FAILURE(s2.close()); ASSERT_NO_FATAL_FAILURE(s1.open()); - ASSERT_NO_FATAL_FAILURE(s2.open()); - ASSERT_NO_FATAL_FAILURE(s3.open()); ASSERT_NO_FATAL_FAILURE(entry1.ReloadUsageEntry()); - ASSERT_NO_FATAL_FAILURE(entry2.ReloadUsageEntry()); - ASSERT_NO_FATAL_FAILURE(entry3.ReloadUsageEntry()); - // Sending a release from an offline license that has been deactivate will - // only work if the license server can handle v16 licenses. This is a rare - // condition, so it is OK to break it during the transition months. - entry1.license_messages().set_api_version(global_features.api_version); - entry2.license_messages().set_api_version(global_features.api_version); - entry3.license_messages().set_api_version(global_features.api_version); + ASSERT_NO_FATAL_FAILURE(entry2.OpenAndReload(this)); + ASSERT_NO_FATAL_FAILURE(entry3.OpenAndReload(this)); + wvcdm::TestSleep::Sleep(kLongSleep); ASSERT_NO_FATAL_FAILURE(s1.UpdateUsageEntry(&encrypted_usage_header_)); ASSERT_NO_FATAL_FAILURE(entry1.GenerateVerifyReport(kInactiveUsed));