From d802baa4d4792ef9029327629d03e4ebabb9be5e Mon Sep 17 00:00:00 2001 From: Rahul Frias Date: Thu, 8 Apr 2021 01:30:47 -0700 Subject: [PATCH] Address CE CDM test failures and code review comments The android CL ag/13947818 was submitted before some CE CDM test failures were noticed and code review comments were received. Bug: 184813991 Test: WV unit/integration test Change-Id: Ic31ca5bc5e46994e01eca56248e6bdffedd779f3 --- libwvdrmengine/cdm/core/include/cdm_session.h | 3 +- libwvdrmengine/cdm/core/src/cdm_session.cpp | 41 +++++-------------- libwvdrmengine/cdm/core/src/device_files.cpp | 4 +- libwvdrmengine/cdm/core/test/test_base.cpp | 22 +++++++++- .../cdm/test/request_license_test.cpp | 34 +++++++-------- 5 files changed, 53 insertions(+), 51 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/cdm_session.h b/libwvdrmengine/cdm/core/include/cdm_session.h index d88063e6..ce589236 100644 --- a/libwvdrmengine/cdm/core/include/cdm_session.h +++ b/libwvdrmengine/cdm/core/include/cdm_session.h @@ -230,7 +230,8 @@ class CdmSession { const std::string& certificate, const CryptoWrappedKey& wrapped_private_key); - CdmResponseType LoadPrivateKey(const CryptoWrappedKey& wrapped_private_key); + CdmResponseType LoadPrivateKey(const std::string& certificate, + const CryptoWrappedKey& wrapped_private_key); bool GenerateKeySetId(bool atsc_mode_enabled, CdmKeySetId* key_set_id); diff --git a/libwvdrmengine/cdm/core/src/cdm_session.cpp b/libwvdrmengine/cdm/core/src/cdm_session.cpp index 07634c2a..8865b221 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_session.cpp @@ -172,6 +172,9 @@ CdmResponseType CdmSession::Init(CdmClientPropertySet* cdm_client_property_set, if (cdm_client_property_set != nullptr) atsc_mode_enabled_ = cdm_client_property_set->use_atsc_mode(); + // If a DRM certificate does not exist, indicate that provisioning is needed. + // The actual validation and loading of a certificate will happen when + // a key request is generated or an offline license is loaded. if (!file_handle_->HasCertificate(atsc_mode_enabled_)) return NEED_PROVISIONING; @@ -1251,31 +1254,14 @@ CdmResponseType CdmSession::LoadPrivateKey() { return NEED_PROVISIONING; } - CdmResponseType status = LoadPrivateKey(private_key); - - if (status == NO_ERROR) { - drm_certificate_ = drm_certificate; - wrapped_private_key_.set_type(private_key.type()); - wrapped_private_key_.set_key(private_key.key()); - } - - return status; + return LoadPrivateKey(drm_certificate, private_key); } CdmResponseType CdmSession::LoadPrivateOrLegacyKey( const std::string& certificate, const CryptoWrappedKey& private_key) { // Use provided key if valid - if (!certificate.empty() && private_key.IsValid()) { - CdmResponseType status = LoadPrivateKey(private_key); - - if (status == NO_ERROR) { - drm_certificate_ = certificate; - wrapped_private_key_.set_type(private_key.type()); - wrapped_private_key_.set_key(private_key.key()); - } - - return status; - } + if (!certificate.empty() && private_key.IsValid()) + return LoadPrivateKey(certificate, private_key); // Otherwise use key from legacy certificate std::string drm_certificate; @@ -1285,19 +1271,11 @@ CdmResponseType CdmSession::LoadPrivateOrLegacyKey( DeviceFiles::kCertificateValid) return NEED_PROVISIONING; - CdmResponseType status = LoadPrivateKey(wrapped_private_key); - - if (status == NO_ERROR) { - drm_certificate_ = drm_certificate; - wrapped_private_key_.set_type(wrapped_private_key.type()); - wrapped_private_key_.set_key(wrapped_private_key.key()); - } - - return status; + return LoadPrivateKey(drm_certificate, wrapped_private_key); } CdmResponseType CdmSession::LoadPrivateKey( - const CryptoWrappedKey& private_key) { + const std::string& drm_certificate, const CryptoWrappedKey& private_key) { CdmResponseType load_cert_sts; M_TIME( load_cert_sts = crypto_session_->LoadCertificatePrivateKey(private_key), @@ -1308,6 +1286,9 @@ CdmResponseType CdmSession::LoadPrivateKey( case NO_ERROR: metrics_->drm_certificate_key_type_.Record( DrmKeyTypeToMetricValue(private_key.type())); + + drm_certificate_ = drm_certificate; + wrapped_private_key_ = std::move(private_key); return NO_ERROR; case SESSION_LOST_STATE_ERROR: case SYSTEM_INVALIDATED_ERROR: diff --git a/libwvdrmengine/cdm/core/src/device_files.cpp b/libwvdrmengine/cdm/core/src/device_files.cpp index d21943c3..06dab5e9 100644 --- a/libwvdrmengine/cdm/core/src/device_files.cpp +++ b/libwvdrmengine/cdm/core/src/device_files.cpp @@ -127,8 +127,8 @@ bool ExtractFromDeviceCertificate(const DeviceCertificate& device_certificate, RETURN_FALSE_IF_NULL(certificate); RETURN_FALSE_IF_NULL(private_key); - bool has_certificate = device_certificate.has_certificate(); - bool has_key = device_certificate.has_wrapped_private_key(); + const bool has_certificate = device_certificate.has_certificate(); + const bool has_key = device_certificate.has_wrapped_private_key(); // If no certificate information, nothing to be done. DeviceCertificate // is a legacy DRM certificate diff --git a/libwvdrmengine/cdm/core/test/test_base.cpp b/libwvdrmengine/cdm/core/test/test_base.cpp index 83e2d551..a256c42e 100644 --- a/libwvdrmengine/cdm/core/test/test_base.cpp +++ b/libwvdrmengine/cdm/core/test/test_base.cpp @@ -397,16 +397,36 @@ void WvCdmTestBase::Provision() { void WvCdmTestBase::EnsureProvisioned() { CdmSessionId session_id; FileSystem file_system; + // OpenSession will check if a DRM certificate exists, while + // GenerateKeyRequest will actually load the wrapped private key. + // Either may return a NEED_PROVISIONING error, so both have to be checked. TestCdmEngine cdm_engine(&file_system, std::shared_ptr(new EngineMetrics)); CdmResponseType status = cdm_engine.OpenSession(config_.key_system(), nullptr, nullptr, &session_id); + + CdmAppParameterMap app_parameters; + CdmKeySetId key_set_id; + InitializationData init_data(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id()); + CdmKeyRequest key_request; + if (status == NO_ERROR) { + status = cdm_engine.GenerateKeyRequest(session_id, key_set_id, init_data, + kLicenseTypeStreaming, + app_parameters, &key_request); + } + if (status == NEED_PROVISIONING) { Provision(); status = cdm_engine.OpenSession(config_.key_system(), nullptr, nullptr, &session_id); + + ASSERT_EQ(NO_ERROR, status); + status = cdm_engine.GenerateKeyRequest(session_id, key_set_id, init_data, + kLicenseTypeStreaming, + app_parameters, &key_request); + ASSERT_EQ(KEY_MESSAGE, status); } - ASSERT_EQ(NO_ERROR, status); + ASSERT_EQ(KEY_MESSAGE, status); ASSERT_NE("", session_id) << "Could not open CDM session."; ASSERT_TRUE(cdm_engine.IsOpenSession(session_id)); ASSERT_EQ(NO_ERROR, cdm_engine.CloseSession(session_id)); diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index e8dff3e5..d0a658bd 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -53,7 +53,6 @@ namespace { // HTTP response codes. const int kHttpOk = 200; const int kDrmCertificateExpiryPeriod = 150; -const std::string kCencMimeType = "video/mp4"; const wvcdm::CdmIdentifier kExampleIdentifier = { wvcdm::EMPTY_SPOID, "com.example", "com.example", 7, 9}; @@ -1752,9 +1751,9 @@ class WvCdmRequestLicenseTest : public WvCdmTestBase { CdmResponseType GenerateKeyRequest() { CdmAppParameterMap app_parameters; - return GenerateKeyRequest(kCencMimeType, binary_key_id(), app_parameters, - kLicenseTypeStreaming, kDefaultCdmIdentifier, - nullptr); + return GenerateKeyRequest(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(), + app_parameters, kLicenseTypeStreaming, + kDefaultCdmIdentifier, nullptr); } void GenerateKeyRequest(const std::string& init_data, @@ -2063,9 +2062,9 @@ class WvCdmRequestLicenseTest : public WvCdmTestBase { if (status == NO_ERROR) { wvcdm::CdmAppParameterMap app_parameters; - status = - GenerateKeyRequest(kCencMimeType, binary_key_id(), app_parameters, - kLicenseTypeStreaming, identifier, nullptr); + status = GenerateKeyRequest(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(), + app_parameters, kLicenseTypeStreaming, + identifier, nullptr); } decryptor_->CloseSession(session_id_); @@ -2098,12 +2097,12 @@ class WvCdmRequestLicenseTest : public WvCdmTestBase { if (IsProvisioned(identifier, requested_security_level)) return; std::string provisioning_server; - CdmCertificateType cert_type = kCertificateWidevine; std::string cert_authority, cert, wrapped_key; - CdmResponseType status = decryptor_->GetProvisioningRequest( - cert_type, cert_authority, identifier, kEmptyServiceCertificate, - requested_security_level, &key_msg_, &provisioning_server); + const CdmResponseType status = decryptor_->GetProvisioningRequest( + kCertificateWidevine, cert_authority, identifier, + kEmptyServiceCertificate, requested_security_level, &key_msg_, + &provisioning_server); EXPECT_EQ(wvcdm::NO_ERROR, status); if (NO_ERROR != status) return; EXPECT_EQ(provisioning_server, kDefaultProvisioningServerUrl); @@ -2299,9 +2298,9 @@ TEST_F(WvCdmRequestLicenseTest, PerOriginProvisioningTest) { CdmAppParameterMap app_parameters; EXPECT_EQ(KEY_MESSAGE, - GenerateKeyRequest(kCencMimeType, binary_key_id(), app_parameters, - kLicenseTypeStreaming, kDefaultCdmIdentifier, - nullptr)); + GenerateKeyRequest(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(), + app_parameters, kLicenseTypeStreaming, + kDefaultCdmIdentifier, nullptr)); decryptor_->CloseSession(session_id_); // The other identifier should not be provisioned. @@ -2881,7 +2880,7 @@ TEST_F(WvCdmRequestLicenseTest, StreamingWithExpiringCertTest) { kExampleIdentifier, nullptr, &session_id_)); CdmAppParameterMap app_parameters; - GenerateKeyRequest(KEY_MESSAGE, kCencMimeType, binary_key_id(), + GenerateKeyRequest(KEY_MESSAGE, ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(), app_parameters, kLicenseTypeStreaming, kExampleIdentifier, nullptr); VerifyKeyRequestResponse(config_.license_server(), config_.client_auth()); @@ -2968,8 +2967,9 @@ TEST_F(WvCdmRequestLicenseTest, RestoreOfflineKeysWithExpiringCertTest) { kExampleIdentifier, nullptr, &session_id_)); CdmAppParameterMap app_parameters; - GenerateKeyRequest(KEY_MESSAGE, kCencMimeType, key_id, app_parameters, - kLicenseTypeOffline, kExampleIdentifier, nullptr); + GenerateKeyRequest(KEY_MESSAGE, ISO_BMFF_VIDEO_MIME_TYPE, key_id, + app_parameters, kLicenseTypeOffline, kExampleIdentifier, + nullptr); VerifyKeyRequestResponse(config_.license_server(), client_auth); CdmKeySetId key_set_id = key_set_id_;