From c034e1f8d24fd22385cb14346f442e1616a90f94 Mon Sep 17 00:00:00 2001 From: Rahul Frias Date: Sun, 29 Nov 2020 03:20:11 -0800 Subject: [PATCH 1/2] Reprovision on error 10085 [ Merge of http://go/wvgerrit/110603 ] Qualcomm SoC may report 10085 (RSASSA-PSS signature error) when OEMCrypto_PrepareAndSignLicenseRequest is called. The app needs to reprovision (or the user needs to factory reset their device) in order to recover. If the 10085 error is returned, the app currently will get a MediaDrmStateException. The app has no way to be able to tell whether this is due to the 10085 error or some other error. This change returns a NEED_PROVISIONING error at the CDM level, which will result in the app receiving a NotProvisionedException when MediaDrm.getKeyRequest is called. Bug: 174375589 Test: WV unit/integration tests Change-Id: I4f2884c8a5fd88ab2e9bfbc0731a20e58cec0f36 --- .../cdm/core/src/crypto_session.cpp | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index 21ad78a7..43509f23 100644 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -65,6 +65,10 @@ constexpr int kMaxTerminateCountDown = 5; const std::string kStringNotAvailable = "NA"; +// TODO(b/174412779): Remove when b/170704368 is fixed. +// This is a Qualcomm specific error code +const int kRsaSsaPssSignatureLengthError = 10085; + // Constants relating to OEMCrypto resource rating tiers. These tables are // ordered by resource rating tier from lowest to highest. These should be // updated whenever the supported range of resource rating tiers changes. @@ -127,7 +131,7 @@ void AdvanceDestBuffer(OEMCrypto_DestBufferDesc* dest_buffer, size_t bytes) { return; } LOGE("Unrecognized OEMCryptoBufferType %u - doing nothing", - dest_buffer->type); + static_cast(dest_buffer->type)); } bool GetGenericSigningAlgorithm(CdmSigningAlgorithm algorithm, @@ -926,6 +930,14 @@ CdmResponseType CryptoSession::PrepareAndSignLicenseRequest( }); if (OEMCrypto_ERROR_SHORT_BUFFER != sts) { + // TODO(b/174412779): Remove when b/170704368 is fixed. + // Temporary workaround. If this error is returned the only way to + // recover is for the app to reprovision. + if (static_cast(sts) == kRsaSsaPssSignatureLengthError) { + LOGE("OEMCrypto PrepareAndSignLicenseRequest result = %d", + static_cast(sts)); + return NEED_PROVISIONING; + } return MapOEMCryptoResult(sts, GENERATE_SIGNATURE_ERROR, "PrepareAndSignLicenseRequest"); } @@ -953,6 +965,14 @@ CdmResponseType CryptoSession::PrepareAndSignLicenseRequest( core_message->resize(core_message_length); return NO_ERROR; } + // TODO(b/174412779): Remove when b/170704368 is fixed. + // Temporary workaround. If this error is returned the only way to + // recover is for the app to reprovision. + if (static_cast(sts) == kRsaSsaPssSignatureLengthError) { + LOGE("OEMCrypto PrepareAndSignLicenseRequest result = %d", + static_cast(sts)); + return NEED_PROVISIONING; + } return MapOEMCryptoResult(sts, GENERATE_SIGNATURE_ERROR, "PrepareAndSignLicenseRequest"); } From 78f4bca3a99773a5d609d3702d955657dcf03877 Mon Sep 17 00:00:00 2001 From: Alex Dale Date: Tue, 1 Dec 2020 19:05:34 -0800 Subject: [PATCH 2/2] Restrict reserved Client ID fields. [ Merge of http://go/wvgerrit/108904 ] Client ID name-value fields in the license request share the same namespace with app parameters and WV standard device information. As a result, it was possible for applications to provide parameters that could potentially fool the license server with spoof values. This CL restricts the use of the fields that are common across both the Android CDM and CE CDM. Currently, Android specific fields are restricted by the MediaDrmPlugin layer, and there are no CE CDM specific fields currently defined. The non-HIDL DRM plugin does not restrict these fields; however, it will be removed in S. Bug: 171723566 Test: Android integration test Change-Id: I5ad9ead73c5aff712dff8133953de5ddc3296452 --- libwvdrmengine/cdm/core/include/cdm_engine.h | 4 ++ .../cdm/core/src/client_identification.cpp | 38 +++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/cdm_engine.h b/libwvdrmengine/cdm/core/include/cdm_engine.h index d94cccee..9f840933 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine.h +++ b/libwvdrmengine/cdm/core/include/cdm_engine.h @@ -75,6 +75,10 @@ class CdmEngine { // app_parameters: Additional, application-specific parameters that factor // into the request generation. This is ignored for release // and renewal requests. + // Certain app parameter keys are reserved for CDM + // device identification on the license server. These + // parameters will be overwritten by the CDM request + // generator. // key_request: This must be non-null and point to a CdmKeyRequest. The // message field will be filled with the key request, the // type field will be filled with the key request type, diff --git a/libwvdrmengine/cdm/core/src/client_identification.cpp b/libwvdrmengine/cdm/core/src/client_identification.cpp index a1f012e6..a3404297 100644 --- a/libwvdrmengine/cdm/core/src/client_identification.cpp +++ b/libwvdrmengine/cdm/core/src/client_identification.cpp @@ -13,6 +13,7 @@ #include "string_conversions.h" #include "wv_cdm_constants.h" +namespace wvcdm { namespace { const std::string kKeyCompanyName = "company_name"; const std::string kKeyModelName = "model_name"; @@ -25,9 +26,35 @@ const std::string kKeyOemCryptoSecurityPatchLevel = "oem_crypto_security_patch_level"; const std::string kKeyOemCryptoBuildInformation = "oem_crypto_build_information"; -} // unnamed namespace -namespace wvcdm { +// These client identification keys are used by the CDM for relaying +// important device information that cannot be overwritten by the app. +const std::array kReservedProperties = { + kKeyCompanyName, + kKeyModelName, + kKeyArchitectureName, + kKeyDeviceName, + kKeyProductName, + kKeyBuildInfo, + kKeyWvCdmVersion, + kKeyOemCryptoSecurityPatchLevel, + kKeyOemCryptoBuildInformation, + // TODO(b/148813171,b/142280599): include "origin" and "application_name" + // to this list once collection of this information has been moved + // to the core CDM. +}; + +// Checks if the client-provided |prop_name| is reserved for CDM device +// identification with the license server. Property keys which are +// reserved should be dropped from the request. +bool IsPropertyKeyReserved(const std::string& prop_name) { + for (const std::string& reserved_prop_name : kReservedProperties) { + if (prop_name == reserved_prop_name) return true; + } + return false; +} +} // namespace + // Protobuf generated classes. using video_widevine::ClientIdentification_ClientCapabilities; using video_widevine::ClientIdentification_NameValue; @@ -99,7 +126,12 @@ CdmResponseType ClientIdentification::Prepare( ClientIdentification_NameValue* client_info; if (is_license_request_) { CdmAppParameterMap::const_iterator iter; - for (iter = app_parameters.begin(); iter != app_parameters.end(); iter++) { + for (iter = app_parameters.begin(); iter != app_parameters.end(); ++iter) { + if (IsPropertyKeyReserved(iter->first)) { + LOGD("Discarding client property: name = \"%s\", value = \"%s\"", + iter->first.c_str(), iter->second.c_str()); + continue; + } client_info = client_id->add_client_info(); client_info->set_name(iter->first); client_info->set_value(iter->second);