From 71fce8f352d461d01c2c82874daf4da49adb3e13 Mon Sep 17 00:00:00 2001 From: Alex Dale Date: Tue, 8 Oct 2024 15:29:47 -0700 Subject: [PATCH 1/3] getOfflineLicenseKeySetIds() respects plugin security level. [ Merge of http://go/wvgerrit/208430 ] The MediaDrm plugin API getOfflineLicenseKeySetIds() was listing both L1 and L3 offline licenses. While this is generally acceptable, apps might force set L3 via the setStringProperty(), which should cause the DRM plugin to behave as if it is L3 only. This change will cause the WVDrmPlugin list L3 only if the app had set the security level to L3. Bug: 357863269 Bug: 372105842 Test: DRM Compliance ATP via ABTD Test: libwvdrmdrmplugin_hal_test on Oriole Change-Id: I1a6e10b7eb880eef4ba36ed31b12ebfe8617f002 (cherry picked from commit 26b888b0944ea72e1a8c29786867d02b921e92ec) --- libwvdrmengine/mediadrm/include/WVDrmPlugin.h | 2 +- libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp | 24 ++++--- .../mediadrm/test/WVDrmPlugin_hal_test.cpp | 70 +++++++++++++++++++ 3 files changed, 85 insertions(+), 11 deletions(-) diff --git a/libwvdrmengine/mediadrm/include/WVDrmPlugin.h b/libwvdrmengine/mediadrm/include/WVDrmPlugin.h index b5e14891..4d374ac1 100644 --- a/libwvdrmengine/mediadrm/include/WVDrmPlugin.h +++ b/libwvdrmengine/mediadrm/include/WVDrmPlugin.h @@ -82,7 +82,7 @@ class WVDrmPlugin : public ::aidl::android::hardware::drm::BnDrmPlugin, ::ndk::ScopedAStatus getNumberOfSessions( ::aidl::android::hardware::drm::NumberOfSessions* _aidl_return) override; ::ndk::ScopedAStatus getOfflineLicenseKeySetIds( - std::vector<::aidl::android::hardware::drm::KeySetId>* _aidl_return) + std::vector<::aidl::android::hardware::drm::KeySetId>* keySetIds) override; ::ndk::ScopedAStatus getOfflineLicenseState( const ::aidl::android::hardware::drm::KeySetId& in_keySetId, diff --git a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp index 5ad1bc7b..636694c7 100644 --- a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp +++ b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp @@ -965,21 +965,27 @@ Status WVDrmPlugin::unprovisionDevice() { } ::ndk::ScopedAStatus WVDrmPlugin::getOfflineLicenseKeySetIds( - vector<::aidl::android::hardware::drm::KeySetId>* _aidl_return) { - _aidl_return->clear(); + vector<::aidl::android::hardware::drm::KeySetId>* keySetIds) { + keySetIds->clear(); CdmIdentifier identifier; const auto status = mCdmIdentifierBuilder.getCdmIdentifier(&identifier); if (status != Status::OK) { return toNdkScopedAStatus(status); } - const std::vector levels = {wvcdm::kSecurityLevelL1, - wvcdm::kSecurityLevelL3}; + std::vector levelsToList; + if (mPropertySet.security_level() != wvcdm::QUERY_VALUE_SECURITY_LEVEL_L3) { + // Do not list L1 offline licenses if the DRM plugin is in + // L3-only mode. + levelsToList.push_back(wvcdm::kSecurityLevelL1); + } + // Always list L3, as "default" may imply either. + levelsToList.push_back(wvcdm::kSecurityLevelL3); std::vector allKeySetIds; CdmResponseType res(wvcdm::UNKNOWN_ERROR); bool success = false; - for (auto level : levels) { + for (const auto& level : levelsToList) { std::vector levelKeySetIds; res = mCDM->ListStoredLicenses(level, identifier, &levelKeySetIds); @@ -1004,15 +1010,13 @@ Status WVDrmPlugin::unprovisionDevice() { // Filter out key sets based on ATSC mode. const auto isAllowedKeySetId = mPropertySet.use_atsc_mode() ? IsAtscKeySetId : IsNotAtscKeySetId; - std::vector keySetIds; + keySetIds->reserve(allKeySetIds.size()); for (const CdmKeySetId& keySetId : allKeySetIds) { if (isAllowedKeySetId(keySetId)) { - keySetIds.push_back(KeySetId{StrToVector(keySetId)}); + keySetIds->push_back(KeySetId{StrToVector(keySetId)}); } } - - *_aidl_return = std::move(keySetIds); - return toNdkScopedAStatus(mapCdmResponseType(wvcdm::NO_ERROR)); + return ::ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus WVDrmPlugin::getOfflineLicenseState( diff --git a/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp b/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp index 226fc0d4..260f7c7f 100644 --- a/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp +++ b/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp @@ -2884,6 +2884,8 @@ TEST_F(WVDrmPluginHalTest, GetOfflineLicenseKeySetIds_NonAtscMode) { std::vector offlineKeySetIds; const auto ret = mPlugin->getOfflineLicenseKeySetIds(&offlineKeySetIds); ASSERT_TRUE(ret.isOk()); + + // Transform back into CDM types. std::vector offlineCdmKeySetIds; for (const auto &keySetId : offlineKeySetIds) { offlineCdmKeySetIds.emplace_back(keySetId.keySetId.begin(), @@ -2929,6 +2931,8 @@ TEST_F(WVDrmPluginHalTest, GetOfflineLicenseKeySetIds_AtscMode) { std::vector offlineKeySetIds; const auto ret = mPlugin->getOfflineLicenseKeySetIds(&offlineKeySetIds); ASSERT_TRUE(ret.isOk()); + + // Transform back into CDM types. std::vector offlineCdmKeySetIds; for (const auto &keySetId : offlineKeySetIds) { offlineCdmKeySetIds.emplace_back(keySetId.keySetId.begin(), @@ -2939,6 +2943,72 @@ TEST_F(WVDrmPluginHalTest, GetOfflineLicenseKeySetIds_AtscMode) { EXPECT_EQ(expectedCdmKeySetIds, offlineCdmKeySetIds); } +TEST_F(WVDrmPluginHalTest, GetOfflineLicenseKeySetIds_L1AndL3) { + const std::vector cdmKeySetIdsL1 = {"ksid1111", "ksid2222", + "ksid3333", "ksid4444"}; + const std::vector cdmKeySetIdsL3 = {"ksid5555", "ksid6666", + "ksid7777", "ksid8888"}; + // Expected key set IDs are the combination of both L1 and L3. + std::vector cdmKeySetIds = cdmKeySetIdsL1; + cdmKeySetIds.insert(cdmKeySetIds.end(), cdmKeySetIdsL3.begin(), + cdmKeySetIdsL3.end()); + + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL1, _, NotNull())) + .WillOnce(DoAll(SetArgPointee<2>(cdmKeySetIdsL1), + testing::Return(CdmResponseType(wvcdm::NO_ERROR)))); + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL3, _, NotNull())) + .WillOnce(DoAll(SetArgPointee<2>(cdmKeySetIdsL3), + testing::Return(CdmResponseType(wvcdm::NO_ERROR)))); + + // In if security level is default, then both L1 and L3 + // offline licenses should be returned. + mPlugin->setPropertyString("securityLevel", + wvcdm::QUERY_VALUE_SECURITY_LEVEL_DEFAULT); + + std::vector offlineKeySetIds; + const auto ret = mPlugin->getOfflineLicenseKeySetIds(&offlineKeySetIds); + ASSERT_TRUE(ret.isOk()); + + // Transform back into CDM types. + std::vector offlineCdmKeySetIds; + for (const auto &keySetId : offlineKeySetIds) { + offlineCdmKeySetIds.emplace_back(keySetId.keySetId.begin(), + keySetId.keySetId.end()); + } + + EXPECT_EQ(cdmKeySetIds.size(), offlineCdmKeySetIds.size()); + EXPECT_EQ(cdmKeySetIds, offlineCdmKeySetIds); +} + +TEST_F(WVDrmPluginHalTest, GetOfflineLicenseKeySetIds_L3Only) { + const std::vector cdmKeySetIdsL3 = {"ksid1111", "ksid2222", + "ksid3333", "ksid4444"}; + + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL1, _, NotNull())) + .Times(0); + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL3, _, NotNull())) + .WillOnce(DoAll(SetArgPointee<2>(cdmKeySetIdsL3), + testing::Return(CdmResponseType(wvcdm::NO_ERROR)))); + + // After setting L3 mode, only L3 key set IDs should be returned. + mPlugin->setPropertyString("securityLevel", + wvcdm::QUERY_VALUE_SECURITY_LEVEL_L3); + + std::vector offlineKeySetIds; + const auto ret = mPlugin->getOfflineLicenseKeySetIds(&offlineKeySetIds); + ASSERT_TRUE(ret.isOk()); + + // Transform back into CDM types. + std::vector offlineCdmKeySetIds; + for (const auto &keySetId : offlineKeySetIds) { + offlineCdmKeySetIds.emplace_back(keySetId.keySetId.begin(), + keySetId.keySetId.end()); + } + + EXPECT_EQ(cdmKeySetIdsL3.size(), offlineCdmKeySetIds.size()); + EXPECT_EQ(cdmKeySetIdsL3, offlineCdmKeySetIds); +} + TEST_F(WVDrmPluginHalTest, GetOfflineLicenseState) { EXPECT_CALL(*mCdm, QueryStatus(_, QUERY_KEY_SECURITY_LEVEL, _)) .WillRepeatedly(DoAll(SetArgPointee<2>(QUERY_VALUE_SECURITY_LEVEL_L1), From f1da7b637df0f9b4fa66b7b3f43c61d3090bded5 Mon Sep 17 00:00:00 2001 From: Alex Dale Date: Tue, 8 Oct 2024 15:36:39 -0700 Subject: [PATCH 2/3] removeOfflineLicense() respects plugin security level. [ Merge of http://go/wvgerrit/208470 ] The MediaDrm plugin API removeOfflineLicense() would check both L1 and L3 for the offline license. While this is generally acceptable, apps might force set L3 via the setStringProperty(), which should cause the DRM plugin to behave as if it is L3 only. This change will cause the WVDrmPlugin only remove L3 key set IDs while in L3 mode. L1 key set IDs in this case will be treated as non-existing. Bug: 357863269 Bug: 372105842 Test: DRM Compliance ATP via ABTD Test: libwvdrmdrmplugin_hal_test on Oriole Change-Id: I81dddbacaee28da6c0a94527b0e390e86f55f81f (cherry picked from commit 0aa6aad1af5dcfaed03895c26bcfd08ab8782d1c) --- libwvdrmengine/mediadrm/include/WVDrmPlugin.h | 3 +- libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp | 21 ++++--- .../mediadrm/test/WVDrmPlugin_hal_test.cpp | 56 +++++++++++++++++++ 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/libwvdrmengine/mediadrm/include/WVDrmPlugin.h b/libwvdrmengine/mediadrm/include/WVDrmPlugin.h index 4d374ac1..63a428d4 100644 --- a/libwvdrmengine/mediadrm/include/WVDrmPlugin.h +++ b/libwvdrmengine/mediadrm/include/WVDrmPlugin.h @@ -134,7 +134,8 @@ class WVDrmPlugin : public ::aidl::android::hardware::drm::BnDrmPlugin, ::ndk::ScopedAStatus removeKeys( const std::vector& in_sessionId) override; ::ndk::ScopedAStatus removeOfflineLicense( - const ::aidl::android::hardware::drm::KeySetId& in_keySetId) override; + const ::aidl::android::hardware::drm::KeySetId& in_keySetIdPackage) + override; ::ndk::ScopedAStatus removeSecureStop( const ::aidl::android::hardware::drm::SecureStopId& in_secureStopId) override; diff --git a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp index 636694c7..1249a6bd 100644 --- a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp +++ b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp @@ -1071,8 +1071,9 @@ Status WVDrmPlugin::unprovisionDevice() { } ::ndk::ScopedAStatus WVDrmPlugin::removeOfflineLicense( - const ::aidl::android::hardware::drm::KeySetId& in_keySetId) { - if (in_keySetId.keySetId.empty()) { + const ::aidl::android::hardware::drm::KeySetId& keySetIdPackage) { + const auto& keySetId = keySetIdPackage.keySetId; + if (keySetId.empty()) { return toNdkScopedAStatus(Status::BAD_VALUE); } @@ -1082,12 +1083,18 @@ Status WVDrmPlugin::unprovisionDevice() { return toNdkScopedAStatus(status); } - const std::vector levels = {wvcdm::kSecurityLevelL1, - wvcdm::kSecurityLevelL3}; - const CdmKeySetId cdmKeySetId(in_keySetId.keySetId.begin(), - in_keySetId.keySetId.end()); + std::vector levelsToCheck; + if (mPropertySet.security_level() != wvcdm::QUERY_VALUE_SECURITY_LEVEL_L3) { + // Do not attempt to remove an L1 offline licenses if the DRM plugin + // is in L3-only mode. + levelsToCheck.push_back(wvcdm::kSecurityLevelL1); + } + // Always check L3, as "default" may imply either. + levelsToCheck.push_back(wvcdm::kSecurityLevelL3); - for (const CdmSecurityLevel level : levels) { + const CdmKeySetId cdmKeySetId(keySetId.begin(), keySetId.end()); + + for (const CdmSecurityLevel level : levelsToCheck) { std::vector keySetIds; const CdmResponseType res = mCDM->ListStoredLicenses(level, identifier, &keySetIds); diff --git a/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp b/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp index 260f7c7f..0e55641a 100644 --- a/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp +++ b/libwvdrmengine/mediadrm/test/WVDrmPlugin_hal_test.cpp @@ -3115,6 +3115,62 @@ TEST_F(WVDrmPluginHalTest, RemoveOfflineLicense_NotFound) { ASSERT_FALSE(status.isOk()); } +TEST_F(WVDrmPluginHalTest, RemoveOfflineLicense_L3_OnlyMode) { + // Key set to remove. + const CdmKeySetId cdmKeySetId = "ksidDEADBEAF"; + const KeySetId keySetId{ + std::vector(cdmKeySetId.begin(), cdmKeySetId.end())}; + + // Desired key set ID is found in L3. + const std::vector cdmKeySetIdsL3 = { + "ksidDEADC0DE", "ksid1337", cdmKeySetId, "ksidBAD", "ksidCAFEB0BA"}; + + // In L3 mode, there should be no call to L1. + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL1, _, _)).Times(0); + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL3, _, NotNull())) + .WillOnce(DoAll(SetArgPointee<2>(cdmKeySetIdsL3), + testing::Return(CdmResponseType(wvcdm::NO_ERROR)))); + + // Only call L3. + EXPECT_CALL(*mCdm, RemoveOfflineLicense(_, kSecurityLevelL1, _)).Times(0); + EXPECT_CALL(*mCdm, RemoveOfflineLicense(cdmKeySetId, kSecurityLevelL3, _)) + .WillOnce(testing::Return(CdmResponseType(wvcdm::NO_ERROR))); + + // After setting L3 mode, only L3 should be checked for removal. + mPlugin->setPropertyString("securityLevel", + wvcdm::QUERY_VALUE_SECURITY_LEVEL_L3); + + const auto status = mPlugin->removeOfflineLicense(keySetId); + ASSERT_TRUE(status.isOk()); +} + +TEST_F(WVDrmPluginHalTest, RemoveOfflineLicense_L3_OnlyMode_NotFound) { + // Key set to remove. + const CdmKeySetId cdmKeySetId = "ksidDEADBEAF"; + const KeySetId keySetId{ + std::vector(cdmKeySetId.begin(), cdmKeySetId.end())}; + + // Desired key set ID is not found in L3. + const std::vector cdmKeySetIdsL3 = {"ksidDEADC0DE", "ksid1337", + "ksidBAD", "ksidCAFEB0BA"}; + + // In L3 mode, there should be no call to L1. + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL1, _, _)).Times(0); + EXPECT_CALL(*mCdm, ListStoredLicenses(kSecurityLevelL3, _, NotNull())) + .WillOnce(DoAll(SetArgPointee<2>(cdmKeySetIdsL3), + testing::Return(CdmResponseType(wvcdm::NO_ERROR)))); + + // No call to RemoveOfflineLicense should be made. + EXPECT_CALL(*mCdm, RemoveOfflineLicense(_, _, _)).Times(0); + + // After setting L3 mode, only L3 should be checked for removal. + mPlugin->setPropertyString("securityLevel", + wvcdm::QUERY_VALUE_SECURITY_LEVEL_L3); + + const auto status = mPlugin->removeOfflineLicense(keySetId); + ASSERT_FALSE(status.isOk()); +} + TEST_F(WVDrmPluginHalTest, CanStoreAtscLicense) { android::sp> cdm = new StrictMock(); StrictMock crypto; From d73997bc0b950fa488e8a8a4d100565f57f8bdc9 Mon Sep 17 00:00:00 2001 From: Kyle Zhang Date: Mon, 14 Oct 2024 18:15:37 +0000 Subject: [PATCH 3/3] Revert "Limit output buffer size during decrypt fallback" Revert submission 28914157 Reason for revert: b/372348308 Reverted changes: /q/submissionid:28914157 Change-Id: Ib77156ffe6abed0f8feee5d9f60f24a90e749ff8 --- .../cdm/core/src/crypto_session.cpp | 20 +------------------ .../test/oec_decrypt_fallback_chain.cpp | 16 +-------------- 2 files changed, 2 insertions(+), 34 deletions(-) diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index f7f2b9e5..939af6dd 100644 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -161,6 +161,7 @@ void AdvanceDestBuffer(OEMCrypto_DestBufferDesc* dest_buffer, size_t bytes) { switch (dest_buffer->type) { case OEMCrypto_BufferType_Clear: dest_buffer->buffer.clear.clear_buffer += bytes; + dest_buffer->buffer.clear.clear_buffer_length -= bytes; return; case OEMCrypto_BufferType_Secure: @@ -3253,11 +3254,6 @@ OEMCryptoResult CryptoSession::DecryptSample( } fake_sample.buffers.input_data_length = length; - if (fake_sample.buffers.output_descriptor.type == - OEMCrypto_BufferType_Clear) { - fake_sample.buffers.output_descriptor.buffer.clear - .clear_buffer_length = length; - } fake_sample.subsamples = &clear_subsample; fake_sample.subsamples_length = 1; @@ -3285,11 +3281,6 @@ OEMCryptoResult CryptoSession::DecryptSample( } fake_sample.buffers.input_data_length = length; - if (fake_sample.buffers.output_descriptor.type == - OEMCrypto_BufferType_Clear) { - fake_sample.buffers.output_descriptor.buffer.clear - .clear_buffer_length = length; - } fake_sample.subsamples = &encrypted_subsample; fake_sample.subsamples_length = 1; @@ -3382,10 +3373,6 @@ OEMCryptoResult CryptoSession::LegacyCopyBufferInChunks( // Calculate the size of the next chunk. const size_t chunk_size = std::min(remaining_input_data, max_chunk_size); - if (output_descriptor.type == OEMCrypto_BufferType_Clear) { - output_descriptor.buffer.clear.clear_buffer_length = chunk_size; - } - // Re-add "last subsample" flag if this is the last subsample. if (chunk_size == remaining_input_data) { subsample_flags |= OEMCrypto_LastSubsample; @@ -3433,11 +3420,6 @@ OEMCryptoResult CryptoSession::LegacyDecryptInChunks( // Calculate the size of the next chunk. const size_t chunk_size = std::min(remaining_input_data, max_chunk_size); fake_sample.buffers.input_data_length = chunk_size; - if (fake_sample.buffers.output_descriptor.type == - OEMCrypto_BufferType_Clear) { - fake_sample.buffers.output_descriptor.buffer.clear.clear_buffer_length = - chunk_size; - } if (is_protected) { fake_subsample.num_bytes_encrypted = chunk_size; } else { diff --git a/libwvdrmengine/oemcrypto/test/oec_decrypt_fallback_chain.cpp b/libwvdrmengine/oemcrypto/test/oec_decrypt_fallback_chain.cpp index fe303fc6..6cb7aac0 100644 --- a/libwvdrmengine/oemcrypto/test/oec_decrypt_fallback_chain.cpp +++ b/libwvdrmengine/oemcrypto/test/oec_decrypt_fallback_chain.cpp @@ -17,6 +17,7 @@ void advance_dest_buffer(OEMCrypto_DestBufferDesc* dest_buffer, size_t bytes) { switch (dest_buffer->type) { case OEMCrypto_BufferType_Clear: dest_buffer->buffer.clear.clear_buffer += bytes; + dest_buffer->buffer.clear.clear_buffer_length -= bytes; break; case OEMCrypto_BufferType_Secure: @@ -98,11 +99,6 @@ OEMCryptoResult DecryptFallbackChain::DecryptSample( const size_t length = subsample.num_bytes_clear + subsample.num_bytes_encrypted; fake_sample.buffers.input_data_length = length; - if (fake_sample.buffers.output_descriptor.type == - OEMCrypto_BufferType_Clear) { - fake_sample.buffers.output_descriptor.buffer.clear.clear_buffer_length = - length; - } fake_sample.subsamples = &subsample; fake_sample.subsamples_length = 1; @@ -148,11 +144,6 @@ OEMCryptoResult DecryptFallbackChain::DecryptSubsample( if (subsample.num_bytes_clear > 0) { fake_sample.buffers.input_data_length = subsample.num_bytes_clear; - if (fake_sample.buffers.output_descriptor.type == - OEMCrypto_BufferType_Clear) { - fake_sample.buffers.output_descriptor.buffer.clear.clear_buffer_length = - subsample.num_bytes_clear; - } fake_subsample.num_bytes_clear = subsample.num_bytes_clear; fake_subsample.num_bytes_encrypted = 0; fake_subsample.block_offset = 0; @@ -176,11 +167,6 @@ OEMCryptoResult DecryptFallbackChain::DecryptSubsample( if (subsample.num_bytes_encrypted > 0) { fake_sample.buffers.input_data_length = subsample.num_bytes_encrypted; - if (fake_sample.buffers.output_descriptor.type == - OEMCrypto_BufferType_Clear) { - fake_sample.buffers.output_descriptor.buffer.clear.clear_buffer_length = - subsample.num_bytes_encrypted; - } fake_subsample.num_bytes_clear = 0; fake_subsample.num_bytes_encrypted = subsample.num_bytes_encrypted; fake_subsample.block_offset = subsample.block_offset;