From 259927efc533da6acc0ecf23b00964c85babc4b8 Mon Sep 17 00:00:00 2001 From: "John \"Juce\" Bruce" Date: Thu, 29 Aug 2013 11:52:36 -0700 Subject: [PATCH] Do Not Return an Error When AddKey Says it Needs a Key Swallows the error NEED_KEY if it comes back from AddKey(), as this is expected behavior. (It means privacy mode is on and the key that was just added was the privacy certificate, ergo the real decryption key is still absent.) Note that this carefully does not squelch the notification that comes from NEED_KEY, which is still necessary in order for the app to make a second key request. Also streamlines a test case that I noticed was overcomplicated for what it did while poaching code from it for new test cases. Also removes a .gyp file that was erroneously being copied to the Android tree. Android does not use GYP. Bug: 10495563 Change-Id: Ife3ff0270a0d09dac1b0eb0d84bddffd811e1eef --- libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp | 9 +- .../mediadrm/test/WVDrmPlugin_test.cpp | 96 +++++++++++++------ libwvdrmengine/oemcrypto/oemcrypto.gyp | 91 ------------------ 3 files changed, 77 insertions(+), 119 deletions(-) delete mode 100644 libwvdrmengine/oemcrypto/oemcrypto.gyp diff --git a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp index 7e7d11c7..bf09a930 100644 --- a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp +++ b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp @@ -237,7 +237,14 @@ status_t WVDrmPlugin::provideKeyResponse( return mapCdmResponseType(res); } else { // For all other requests, we have a session ID. - return mapAndNotifyOfCdmResponseType(scope, res); + status_t status = mapAndNotifyOfCdmResponseType(scope, res); + // For "NEED_KEY," we still want to send the notifcation, but then we don't + // return the error. This is because "NEED_KEY" from AddKey() is an + // expected behavior when sending a privacy certificate. + if (res == wvcdm::NEED_KEY && mPropertySet.use_privacy_mode()) { + status = android::OK; + } + return status; } } diff --git a/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp b/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp index 9c0922de..e42a1ffb 100644 --- a/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp +++ b/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp @@ -322,6 +322,70 @@ TEST_F(WVDrmPluginTest, AddsKeys) { EXPECT_EQ(0u, emptyKeySetId.size()); } +TEST_F(WVDrmPluginTest, HandlesPrivacyCertCaseOfAddKey) { + StrictMock cdm; + StrictMock crypto; + WVDrmPlugin plugin(&cdm, &crypto); + + sp > listener = + new StrictMock(); + + const CdmClientPropertySet* propertySet = NULL; + + // Provide expected behavior in response to OpenSession and store the + // property set + EXPECT_CALL(cdm, OpenSession(_, _, _)) + .WillRepeatedly(DoAll(SetArgPointee<2>(cdmSessionId), + SaveArg<1>(&propertySet), + Return(wvcdm::NO_ERROR))); + + // Provide expected behavior when plugin requests session control info + EXPECT_CALL(cdm, QueryKeyControlInfo(cdmSessionId, _)) + .WillRepeatedly(Invoke(setSessionIdOnMap<4>)); + + // Let gMock know these calls will happen but we aren't interested in them. + EXPECT_CALL(cdm, AttachEventListener(_, _)) + .Times(AtLeast(0)); + + EXPECT_CALL(cdm, DetachEventListener(_, _)) + .Times(AtLeast(0)); + + EXPECT_CALL(cdm, CloseSession(_)) + .Times(AtLeast(0)); + + static const uint32_t kResponseSize = 256; + uint8_t responseRaw[kResponseSize]; + FILE* fp = fopen("/dev/urandom", "r"); + fread(responseRaw, sizeof(uint8_t), kResponseSize, fp); + fclose(fp); + + Vector response; + response.appendArray(responseRaw, kResponseSize); + Vector keySetId; + + EXPECT_CALL(*listener, sendEvent(DrmPlugin::kDrmPluginEventKeyNeeded, 0, + Pointee(ElementsAreArray(sessionIdRaw, + kSessionIdSize)), + NULL)) + .Times(1); + + EXPECT_CALL(cdm, AddKey(_, _, _)) + .WillRepeatedly(Return(wvcdm::NEED_KEY)); + + plugin.openSession(sessionId); + ASSERT_THAT(propertySet, NotNull()); + + status_t res = plugin.setListener(listener); + ASSERT_EQ(OK, res); + + res = plugin.setPropertyString(String8("privacyMode"), String8("enable")); + ASSERT_EQ(OK, res); + EXPECT_TRUE(propertySet->use_privacy_mode()); + + res = plugin.provideKeyResponse(sessionId, response, keySetId); + ASSERT_EQ(OK, res); +} + TEST_F(WVDrmPluginTest, CancelsKeyRequests) { StrictMock cdm; StrictMock crypto; @@ -1035,7 +1099,7 @@ TEST_F(WVDrmPluginTest, UnregistersForAllEventsOnDestruction) { EXPECT_CALL(cdm, DetachEventListener(cdmSessionId2, &plugin)) .Times(1); - // Let gMock know this call will happen but we aren't interested in it. + // Let gMock know these calls will happen but we aren't interested in them. EXPECT_CALL(cdm, AttachEventListener(_, _)) .Times(AtLeast(0)); @@ -1055,7 +1119,8 @@ TEST_F(WVDrmPluginTest, MarshalsEvents) { StrictMock crypto; WVDrmPlugin plugin(&cdm, &crypto); - sp listener = new MockDrmPluginListener(); + sp > listener = + new StrictMock(); { InSequence calls; @@ -1073,32 +1138,9 @@ TEST_F(WVDrmPluginTest, MarshalsEvents) { .Times(1); } - // Provide expected behavior to support session creation - EXPECT_CALL(cdm, OpenSession(StrEq("com.widevine"), _, _)) - .Times(AtLeast(1)) - .WillRepeatedly(DoAll(SetArgPointee<2>(cdmSessionId), - Return(wvcdm::NO_ERROR))); - - EXPECT_CALL(cdm, QueryKeyControlInfo(cdmSessionId, _)) - .Times(AtLeast(1)) - .WillRepeatedly(Invoke(setSessionIdOnMap<4>)); - - // Let gMock know these calls will happen but we aren't interested in them. - EXPECT_CALL(cdm, AttachEventListener(_, _)) - .Times(AtLeast(0)); - - EXPECT_CALL(cdm, DetachEventListener(_, _)) - .Times(AtLeast(0)); - - EXPECT_CALL(cdm, CloseSession(_)) - .Times(AtLeast(0)); - status_t res = plugin.setListener(listener); ASSERT_EQ(OK, res); - res = plugin.openSession(sessionId); - ASSERT_EQ(OK, res); - plugin.onEvent(cdmSessionId, LICENSE_EXPIRED_EVENT); plugin.onEvent(cdmSessionId, LICENSE_RENEWAL_NEEDED_EVENT); } @@ -1108,7 +1150,8 @@ TEST_F(WVDrmPluginTest, GeneratesProvisioningNeededEvent) { StrictMock crypto; WVDrmPlugin plugin(&cdm, &crypto); - sp listener = new MockDrmPluginListener(); + sp > listener = + new StrictMock(); EXPECT_CALL(*listener, sendEvent(DrmPlugin::kDrmPluginEventProvisionRequired, 0, Pointee(ElementsAreArray(sessionIdRaw, @@ -1116,7 +1159,6 @@ TEST_F(WVDrmPluginTest, GeneratesProvisioningNeededEvent) { NULL)) .Times(1); - // Provide expected behavior to support session creation EXPECT_CALL(cdm, OpenSession(StrEq("com.widevine"), _, _)) .Times(AtLeast(1)) .WillRepeatedly(DoAll(SetArgPointee<2>(cdmSessionId), diff --git a/libwvdrmengine/oemcrypto/oemcrypto.gyp b/libwvdrmengine/oemcrypto/oemcrypto.gyp deleted file mode 100644 index bf9b6525..00000000 --- a/libwvdrmengine/oemcrypto/oemcrypto.gyp +++ /dev/null @@ -1,91 +0,0 @@ -# Copyright 2012 Google Inc. All Rights Reserved. -# Author: rkuroiwa@google.com (Rintaro Kuroiwa) - -{ - 'variables': { - 'chromium_code': 1, - 'oec_target_type': "", - }, - 'targets': [ - { - 'target_name': 'oec_lib', - 'type': 'none', - 'conditions': [ - ['target_arch=="arm" and oec_target_type != "CAN_INSTALL_KEYBOX"', { - 'dependencies': [ - 'oec_mrvl', - ], - }, { - 'dependencies': [ - 'oec_mock', - ], - }], - ], - }, - { - 'target_name': 'oec_mock', - 'type': 'static_library', - 'sources': [ - 'mock/src/oemcrypto_mock.cpp', - 'mock/src/oemcrypto_engine_mock.cpp', - 'mock/src/oemcrypto_key_mock.cpp', - 'mock/src/oemcrypto_keybox_mock.cpp', - 'mock/src/wvcrc.cpp', - ], - 'include_dirs': [ - 'include', - ], - 'dependencies': [ - '../../../crypto/crypto.gyp:crypto', - # TODO(kqyang): make it platform independent. - '../chromium/util.gyp:lock', - '../chromium/util.gyp:string_conversions', - ], - }, - { - 'target_name': 'oec_mrvl', - 'type': 'static_library', - 'sources': [ - 'eureka/src/oemcrypto_mrvl.cpp', - ], - 'include_dirs': [ - '../include', - '../core/include', - ], - 'dependencies': [ - '../../../base/base.gyp:base', - '../../../crypto/crypto.gyp:crypto', - ], - 'cflags': [ - '-Wsign-conversion', - ], - 'link_settings': { - 'libraries': [ - '-lOSAL', - '-lPEAgent', - ], - }, - }, - { - 'target_name': 'oec_unittest', - 'type': '<(gtest_target_type)', - 'conditions': [ - ['target_arch!="arm" or oec_target_type == "CAN_INSTALL_KEYBOX"', { - 'defines': [ 'CAN_INSTALL_KEYBOX', ], - },], - ], - 'sources': [ - 'test/oemcrypto_test.cpp', - ], - 'include_dirs': [ - 'include', - 'mock/src', - ], - 'dependencies': [ - 'oec_lib', - '../../../testing/gtest.gyp:gtest', - '../../../testing/gtest.gyp:gtest_main', - ], - }, - ], -}