From 9c95aba4f504c434985882e994707a5fc07cf1cb Mon Sep 17 00:00:00 2001 From: Rahul Frias Date: Tue, 8 May 2018 00:30:46 -0700 Subject: [PATCH 1/3] Close CDM sessions only if opened [ Merge of http://go/wvgerrit/49822 ] This avoids logging an unnecessary error, when the session is not found. Bug: 79210873 Test: Wv unit/integration test, GtsMediaDrmTest, playback with Play Movies and Netflix. Change-Id: Ifef99d1380d763670ad0fa89c885fb5fd41567e2 --- libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp b/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp index f21b8564..2ab88b9d 100644 --- a/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp +++ b/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp @@ -142,14 +142,16 @@ WVDrmPlugin::~WVDrmPlugin() { } } mCryptoSessions.clear(); - CdmIdentifier identifier; - Status status = mCdmIdentifierBuilder.getCdmIdentifier(&identifier); - if (status != Status::OK) { - ALOGE("Failed to get cdm identifier %d", status); - } else { - status = mapCdmResponseType(mCDM->CloseCdm(identifier)); + if (mCdmIdentifierBuilder.is_sealed()) { + CdmIdentifier identifier; + Status status = mCdmIdentifierBuilder.getCdmIdentifier(&identifier); if (status != Status::OK) { - ALOGE("Failed to close cdm. status %d", status); + ALOGE("Failed to get cdm identifier %d", status); + } else { + status = mapCdmResponseType(mCDM->CloseCdm(identifier)); + if (status != Status::OK) { + ALOGE("Failed to close cdm. status %d", status); + } } } } From 98532d313b66562d44b1aa33f7777f9cbcd09a54 Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Tue, 8 May 2018 14:01:45 -0700 Subject: [PATCH 2/3] Use 128 bit AES for key control block Merge from Widevine repo of http://go/wvgerrit/49805 This aligns the oemcrypto reference code and unit tests to match the API design doc: http://go/oemcrypto bug: 79375509 test: unit tests pass Change-Id: I13761a7384a17e99d88e61aaf80b4a22941fd172 --- .../oemcrypto/mock/src/oemcrypto_session.cpp | 5 +---- libwvdrmengine/oemcrypto/test/oec_session_util.cpp | 11 ++++------- libwvdrmengine/oemcrypto/test/oec_session_util.h | 4 ---- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_session.cpp b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_session.cpp index 40983781..4894cd99 100644 --- a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_session.cpp +++ b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_session.cpp @@ -749,10 +749,7 @@ OEMCryptoResult SessionContext::InstallKey( return OEMCrypto_ERROR_INVALID_CONTEXT; } if (!DecryptMessage(content_key, key_control_iv, key_control, - &key_control_str, - (session_keys_->type() == OEMCrypto_EntitlementLicense - ? 256 - : 128) /* key size */)) { + &key_control_str, 128 /* key size */)) { LOGE("[Installkey(): ERROR: Could not decrypt content key]"); return OEMCrypto_ERROR_UNKNOWN_FAILURE; } diff --git a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp index a3fa9f0e..75c461dc 100644 --- a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp +++ b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp @@ -107,9 +107,9 @@ Session::Session() enc_key_(wvcdm::KEY_SIZE), public_rsa_(0), message_size_(sizeof(MessageData)), - num_keys_(4), // Most tests only use 4 keys. - // Other tests will explicitly call set_num_keys. - has_entitlement_license_(false) { + // Most tests only use 4 keys. Other tests will explicitly call + // set_num_keys. + num_keys_(4) { // Stripe the padded message. for (size_t i = 0; i < sizeof(padded_message_.padding); i++) { padded_message_.padding[i] = i % 0x100; @@ -313,7 +313,6 @@ void Session::LoadEnitlementTestKeys(const std::string& pst, } void Session::FillEntitledKeyArray() { - has_entitlement_license_ = true; for (size_t i = 0; i < num_keys_; ++i) { EntitledContentKeyData* key_data = &entitled_key_data_[i]; @@ -518,7 +517,6 @@ void Session::FillSimpleMessage(uint32_t duration, uint32_t control, void Session::FillSimpleEntitlementMessage( uint32_t duration, uint32_t control, uint32_t nonce, const std::string& pst) { - has_entitlement_license_ = true; EXPECT_EQ( 1, GetRandBytes(license_.mac_key_iv, sizeof(license_.mac_key_iv))); EXPECT_EQ(1, GetRandBytes(license_.mac_keys, sizeof(license_.mac_keys))); @@ -599,10 +597,9 @@ void Session::EncryptAndSign() { AES_cbc_encrypt(&license_.mac_keys[0], &encrypted_license().mac_keys[0], 2 * wvcdm::MAC_KEY_SIZE, &aes_key, iv_buffer, AES_ENCRYPT); - int key_size = has_entitlement_license() ? 256 : 128; for (unsigned int i = 0; i < num_keys_; i++) { memcpy(iv_buffer, &license_.keys[i].control_iv[0], wvcdm::KEY_IV_SIZE); - AES_set_encrypt_key(&license_.keys[i].key_data[0], key_size, &aes_key); + AES_set_encrypt_key(&license_.keys[i].key_data[0], 128, &aes_key); AES_cbc_encrypt( reinterpret_cast(&license_.keys[i].control), reinterpret_cast(&encrypted_license().keys[i].control), diff --git a/libwvdrmengine/oemcrypto/test/oec_session_util.h b/libwvdrmengine/oemcrypto/test/oec_session_util.h index 05c9eff3..cea7d8dc 100644 --- a/libwvdrmengine/oemcrypto/test/oec_session_util.h +++ b/libwvdrmengine/oemcrypto/test/oec_session_util.h @@ -378,9 +378,6 @@ class Session { // The size of the encrypted message. size_t message_size() { return message_size_; } - // If this session has an entitlement license. - bool has_entitlement_license() const { return has_entitlement_license_; } - private: // Generate mac and enc keys give the master key. void DeriveKeys(const uint8_t* master_key, @@ -410,7 +407,6 @@ class Session { vector encrypted_usage_entry_; uint32_t usage_entry_number_; string pst_; - bool has_entitlement_license_; // Clear Entitlement key data. This is the backing data for // |entitled_key_array_|. From a31398517424c848701cb6afa7a0affbf2ced632 Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Wed, 2 May 2018 13:54:18 -0700 Subject: [PATCH 3/3] Add unit test for nonce sharing Merge from Widevine repo of http://go/wvgerrit/49302 This CL adds some unit tests to verify that several OEMCrypto sessions do not share nonce tables. bug: 64850992 test: unit tests run on sailfish, taimen, and walleye. Change-Id: I06cf3fdafb84f8b09cf2f0e58c1866bac511a293 --- .../oemcrypto/test/oemcrypto_test.cpp | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp b/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp index 84b4d5c0..c94eafce 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_test.cpp @@ -1076,6 +1076,59 @@ TEST_F(OEMCryptoSessionTests, LoadKeyWithRepeatNonce) { ASSERT_NE(OEMCrypto_SUCCESS, sts); } +// This tests that a nonce cannot be used in new session. +TEST_F(OEMCryptoSessionTests, LoadKeyNonceReopenSession) { + Session s; + ASSERT_NO_FATAL_FAILURE(s.open()); + ASSERT_NO_FATAL_FAILURE(InstallTestSessionKeys(&s)); + uint32_t nonce = s.get_nonce(); + // Do not use the nonce now. Close session and use it after re-opening. + ASSERT_NO_FATAL_FAILURE(s.close()); + + // Actually, this isn't the same session. OEMCrypto opens a new session, but + // we are guarding against the possiblity that it re-uses the session data + // and might not clear out the nonce table correctly. + ASSERT_NO_FATAL_FAILURE(s.open()); + ASSERT_NO_FATAL_FAILURE(InstallTestSessionKeys(&s)); + ASSERT_NO_FATAL_FAILURE(s.FillSimpleMessage(0, + wvoec_mock::kControlNonceEnabled, + nonce)); // same old nonce + ASSERT_NO_FATAL_FAILURE(s.EncryptAndSign()); + OEMCryptoResult sts = OEMCrypto_LoadKeys( + s.session_id(), s.message_ptr(), s.message_size(), &s.signature()[0], + s.signature().size(), s.encrypted_license().mac_key_iv, + s.encrypted_license().mac_keys, s.num_keys(), s.key_array(), NULL, 0, + NULL, OEMCrypto_ContentLicense); + + ASSERT_NE(OEMCrypto_SUCCESS, sts); +} + +// This tests that a nonce cannot be used in wrong session. +TEST_F(OEMCryptoSessionTests, LoadKeyNonceWrongSession) { + Session s1; + ASSERT_NO_FATAL_FAILURE(s1.open()); + ASSERT_NO_FATAL_FAILURE(InstallTestSessionKeys(&s1)); + uint32_t nonce = s1.get_nonce(); + // Do not use the nonce. Also, leave the session open. We want to make sure + // that s and s1 do NOT share a nonce table. This is different from the + // LoadKeyNonceReopenSession in that we do not close s1. + + Session s2; + ASSERT_NO_FATAL_FAILURE(s2.open()); + ASSERT_NO_FATAL_FAILURE(InstallTestSessionKeys(&s2)); + ASSERT_NO_FATAL_FAILURE(s2.FillSimpleMessage(0, + wvoec_mock::kControlNonceEnabled, + nonce)); // nonce from session s1 + ASSERT_NO_FATAL_FAILURE(s2.EncryptAndSign()); + OEMCryptoResult sts = OEMCrypto_LoadKeys( + s2.session_id(), s2.message_ptr(), s2.message_size(), &s2.signature()[0], + s2.signature().size(), s2.encrypted_license().mac_key_iv, + s2.encrypted_license().mac_keys, s2.num_keys(), s2.key_array(), NULL, 0, + NULL, OEMCrypto_ContentLicense); + + ASSERT_NE(OEMCrypto_SUCCESS, sts); +} + TEST_F(OEMCryptoSessionTests, LoadKeyWithBadVerification) { Session s; ASSERT_NO_FATAL_FAILURE(s.open());