From 3709a4f419b756c1f3e709e2eadfd5cb2a5623c7 Mon Sep 17 00:00:00 2001 From: Rahul Frias Date: Wed, 22 Jul 2020 03:03:34 -0700 Subject: [PATCH] Allow offline licenses to be loaded and restored in the same session [ Merge of http://go/wvgerrit/103243 ] In v16, OEMCrypto specifications required that an error be returned if multiple attempts are made to load an offline license into a session. This caused the GTS test testConcurrentDrmCertificates to fail. It was introduced to verify that a license could retrieved and loaded into a session and then restored. This was based on an app use case. Ideally we would like to disallow a this behavior but need to make sure it is not being used by apps. For now this will be allowed. If detected, the CDM will reintialize the OEMCrypto session and allow the license to be restored. Bug: 161551490 Test: WV unit integration tests, GtsMediaTestCases and WidevineConcurrentDrmCertificatesTest#testConcurrentDrmCertificates, MediaDrmTest#testMultipleLoadKeys on a redfin Change-Id: I0834e4419c3a6dccfd77aaea3afa3d65c2c0c742 --- libwvdrmengine/cdm/core/include/cdm_session.h | 21 +++++++ .../cdm/core/include/wv_cdm_types.h | 1 + libwvdrmengine/cdm/core/src/cdm_session.cpp | 56 +++++++++++++++++++ .../cdm/core/test/test_printers.cpp | 3 + .../cdm/test/request_license_test.cpp | 18 ++++++ libwvdrmengine/include/WVErrors.h | 3 +- libwvdrmengine/include/mapErrors-inl.h | 2 + libwvdrmengine/include_hidl/mapErrors-inl.h | 1 + 8 files changed, 104 insertions(+), 1 deletion(-) diff --git a/libwvdrmengine/cdm/core/include/cdm_session.h b/libwvdrmengine/cdm/core/include/cdm_session.h index 6325e49e..e173e8a4 100644 --- a/libwvdrmengine/cdm/core/include/cdm_session.h +++ b/libwvdrmengine/cdm/core/include/cdm_session.h @@ -239,6 +239,12 @@ class CdmSession { // true otherwise. bool VerifyOfflineUsageEntry(); + // On android, we previously permitted a license to be loaded and restored + // in the same session. This method releases resources so that + // CdmSession::Init can safely be invoked before a new license is restored. + // TODO(b/161865160): Investigate whether we can dispense with this scenario. + virtual CdmResponseType ReleaseOfflineResources(); + // These setters are for testing only. Takes ownership of the pointers. void set_license_parser(CdmLicense* license_parser); void set_crypto_session(CryptoSession* crypto_session); @@ -294,6 +300,21 @@ class CdmSession { // license type release and offline related information CdmKeySetId key_set_id_; + // TODO(b/161865160): Use these variables to cache Init parameters. Remove + // when b/ has been addressed. + // |cdm_client_property_set_| and |event_listener_| point to a data + // member of WVDrmPlugin or WVDrmPlugin itself. It is safe for CdmSession + // to make use of these objects without taking ownership since WVDrmPlugin + // lifetime exceeds CdmSession (WVDrmPlugin indirectly owns CdmSession + // objects). These pointers if set, should be valid till CdmSession + // destruction. + CdmClientPropertySet* cdm_client_property_set_ = nullptr; + CdmSessionId* forced_session_id_ = nullptr; + CdmSessionId forced_session_id_value_; + WvCdmEventListener* event_listener_ = nullptr; + bool has_license_been_loaded_ = false; + bool has_license_been_restored_ = false; + bool mock_license_parser_in_use_; bool mock_policy_engine_in_use_; diff --git a/libwvdrmengine/cdm/core/include/wv_cdm_types.h b/libwvdrmengine/cdm/core/include/wv_cdm_types.h index c06ed7e5..4b833910 100644 --- a/libwvdrmengine/cdm/core/include/wv_cdm_types.h +++ b/libwvdrmengine/cdm/core/include/wv_cdm_types.h @@ -414,6 +414,7 @@ enum CdmResponseType { SHRINK_USAGE_TABLE_HEADER_ENTRY_IN_USE = 359, LICENSE_USAGE_ENTRY_MISSING = 360, LOAD_USAGE_ENTRY_INVALID_SESSION = 361, + RESTORE_OFFLINE_LICENSE_ERROR_3 = 362, // Don't forget to add new values to // * core/test/test_printers.cpp. // * android/include/mapErrors-inl.h diff --git a/libwvdrmengine/cdm/core/src/cdm_session.cpp b/libwvdrmengine/cdm/core/src/cdm_session.cpp index 8e1ccf34..599fc2c8 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_session.cpp @@ -111,6 +111,18 @@ CdmResponseType CdmSession::Init(CdmClientPropertySet* cdm_client_property_set, return REINIT_ERROR; } + // Save parameters in case Init needs to be called again (load and restore + // offline license) + if (cdm_client_property_set) + cdm_client_property_set_ = cdm_client_property_set; + + if (forced_session_id) { + forced_session_id_value_ = *forced_session_id; + forced_session_id_ = &forced_session_id_value_; + } + + if (event_listener) event_listener_ = event_listener; + if (cdm_client_property_set && cdm_client_property_set->security_level() == QUERY_VALUE_SECURITY_LEVEL_L3) { requested_security_level_ = kLevel3; @@ -229,6 +241,24 @@ CdmResponseType CdmSession::Init(CdmClientPropertySet* cdm_client_property_set, return NO_ERROR; } +CdmResponseType CdmSession::ReleaseOfflineResources() { + // |license_parser_| and |policy_engine_| are reset in Init. No need to + // deallocate here. + if (usage_support_type_ == kUsageEntrySupport && + has_provider_session_token() && usage_table_header_ != nullptr && + !is_release_) { + UpdateUsageEntryInformation(); + } + + if (!key_set_id_.empty()) { + // Unreserve the license ID. + file_handle_->UnreserveLicenseId(key_set_id_); + } + crypto_session_.reset(CryptoSession::MakeCryptoSession(crypto_metrics_)); + initialized_ = false; + return NO_ERROR; +} + CdmResponseType CdmSession::RestoreOfflineSession(const CdmKeySetId& key_set_id, CdmLicenseType license_type, int* error_detail) { @@ -239,6 +269,31 @@ CdmResponseType CdmSession::RestoreOfflineSession(const CdmKeySetId& key_set_id, if (!key_set_id_.empty()) { file_handle_->UnreserveLicenseId(key_set_id_); } + + // On android, we previously permitted an offline license to be loaded and + // restored in the same session. OEMCrypto v16+ disallows it so we need to + // release and initialize an OEMCrypto session. We will still prohibit + // multiple restore attempts on the same session. + // TODO(b/161865160): reevalute this scenario. Should we also + // (a) only allow a restore for the same key set ID that was loaded + // (b) if (a) is true, indicate success and do nothing else rather than + // release resources and reinitialize. + // We need to investigate the conditions that caused an app failure and + // led us to add a test to support this use case as there were multiple + // related issues. + if (!has_license_been_loaded_ && has_license_been_restored_) { + LOGE("Disallow multiple offline license restores"); + return RESTORE_OFFLINE_LICENSE_ERROR_3; + } + + if (has_license_been_loaded_) { + CdmResponseType status = ReleaseOfflineResources(); + if (status != NO_ERROR) return status; + status = + Init(cdm_client_property_set_, forced_session_id_, event_listener_); + if (status != NO_ERROR) return status; + } + has_license_been_restored_ = true; key_set_id_ = key_set_id; DeviceFiles::CdmLicenseData license_data; @@ -585,6 +640,7 @@ CdmResponseType CdmSession::AddKeyInternal(const CdmKeyResponse& key_response) { sts = StoreLicense(); if (sts != NO_ERROR) return sts; } + has_license_been_loaded_ = true; return KEY_ADDED; } diff --git a/libwvdrmengine/cdm/core/test/test_printers.cpp b/libwvdrmengine/cdm/core/test/test_printers.cpp index 6caa8931..51848a80 100644 --- a/libwvdrmengine/cdm/core/test/test_printers.cpp +++ b/libwvdrmengine/cdm/core/test/test_printers.cpp @@ -737,6 +737,9 @@ void PrintTo(const enum CdmResponseType& value, ::std::ostream* os) { case RESTORE_OFFLINE_LICENSE_ERROR_2: *os << "RESTORE_OFFLINE_LICENSE_ERROR_2"; break; + case RESTORE_OFFLINE_LICENSE_ERROR_3: + *os << "RESTORE_OFFLINE_LICENSE_ERROR_3"; + break; case REWRAP_DEVICE_RSA_KEY_ERROR: *os << "REWRAP_DEVICE_RSA_KEY_ERROR"; break; diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index 98089883..f035fe9c 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -2696,6 +2696,24 @@ TEST_F(WvCdmRequestLicenseTest, DisallowMultipleRestoreOfflineKeyTest) { decryptor_->CloseSession(session_id_); } +TEST_F(WvCdmRequestLicenseTest, AllowLoadAndRestoreOfflineKeyTest) { + Unprovision(); + Provision(); + + std::string key_id; + std::string client_auth; + GetOfflineConfiguration(&key_id, &client_auth); + + decryptor_->OpenSession(config_.key_system(), nullptr, kDefaultCdmIdentifier, + nullptr, &session_id_); + GenerateKeyRequest(key_id, kLicenseTypeOffline); + VerifyKeyRequestResponse(config_.license_server(), client_auth); + + CdmKeySetId key_set_id = key_set_id_; + EXPECT_EQ(wvcdm::KEY_ADDED, decryptor_->RestoreKey(session_id_, key_set_id)); + decryptor_->CloseSession(session_id_); +} + TEST_F(WvCdmRequestLicenseTest, ReleaseOfflineKeyTest) { Unprovision(); Provision(); diff --git a/libwvdrmengine/include/WVErrors.h b/libwvdrmengine/include/WVErrors.h index 9cfb2228..c495e083 100644 --- a/libwvdrmengine/include/WVErrors.h +++ b/libwvdrmengine/include/WVErrors.h @@ -296,10 +296,11 @@ enum { kShrinkUsageTableHeaderEntryInUse = ERROR_DRM_VENDOR_MIN + 311, kLicenseUsageEntryMissing = ERROR_DRM_VENDOR_MIN + 312, kLoadUsageEntryInvalidSession = ERROR_DRM_VENDOR_MIN + 313, + kRestoreOfflineLicenseError3 = ERROR_DRM_VENDOR_MIN + 314, // This should always follow the last error code. // The offset value should be updated each time a new error code is added. - kErrorWVDrmMaxErrorUsed = ERROR_DRM_VENDOR_MIN + 313, + kErrorWVDrmMaxErrorUsed = ERROR_DRM_VENDOR_MIN + 314, // Used by crypto test mode kErrorTestMode = ERROR_DRM_VENDOR_MAX, diff --git a/libwvdrmengine/include/mapErrors-inl.h b/libwvdrmengine/include/mapErrors-inl.h index e7315d7f..b1eccf55 100644 --- a/libwvdrmengine/include/mapErrors-inl.h +++ b/libwvdrmengine/include/mapErrors-inl.h @@ -478,6 +478,8 @@ static android::status_t mapCdmResponseType(wvcdm::CdmResponseType res) { return kRenewKeyError2; case wvcdm::RESTORE_OFFLINE_LICENSE_ERROR_2: return kRestoreOfflineLicenseError2; + case wvcdm::RESTORE_OFFLINE_LICENSE_ERROR_3: + return kRestoreOfflineLicenseError3; case wvcdm::SAMPLE_AND_SUBSAMPLE_SIZE_MISMATCH: return kSampleAndSubsampleSizeMismatch; case wvcdm::SESSION_FILE_HANDLE_INIT_ERROR: diff --git a/libwvdrmengine/include_hidl/mapErrors-inl.h b/libwvdrmengine/include_hidl/mapErrors-inl.h index c6f61778..74ed8b90 100644 --- a/libwvdrmengine/include_hidl/mapErrors-inl.h +++ b/libwvdrmengine/include_hidl/mapErrors-inl.h @@ -166,6 +166,7 @@ static Status mapCdmResponseType(wvcdm::CdmResponseType res) { case wvcdm::RENEW_KEY_ERROR_1: case wvcdm::RENEW_KEY_ERROR_2: case wvcdm::RESTORE_OFFLINE_LICENSE_ERROR_2: + case wvcdm::RESTORE_OFFLINE_LICENSE_ERROR_3: case wvcdm::NOT_INITIALIZED_ERROR: case wvcdm::REINIT_ERROR: case wvcdm::SESSION_KEYS_NOT_FOUND: