From 83a85430e32f23ff5af7941ecfc7bb7bca5c20ae Mon Sep 17 00:00:00 2001
From: Rahul Frias <rfrias@google.com>
Date: Tue, 23 Feb 2021 12:46:43 -0800
Subject: [PATCH] Release crypto resources when provisioning fails

[ Merge of http://go/wvgerrit/119564 ]

This closes a crypto session when the provisioning request fails. We
cannot be as eager when handling the response as the app may have
multiple simultaneous provisioning attempts in flight. In this case
all provisioning responses except the one associated with the last
request will fail. If we close the session on error, even the one
associated with the last request may fail.

Bug: 180986725
Test: WV unit/integration tests
Change-Id: Ic3d33a374e442b5bf040e345bed829d91c4ef1dc
---
 .../core/include/certificate_provisioning.h   | 15 ++++++++++++
 .../cdm/core/src/certificate_provisioning.cpp | 24 +++++++++++++++++--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/libwvdrmengine/cdm/core/include/certificate_provisioning.h b/libwvdrmengine/cdm/core/include/certificate_provisioning.h
index 6159fafe..8766de81 100644
--- a/libwvdrmengine/cdm/core/include/certificate_provisioning.h
+++ b/libwvdrmengine/cdm/core/include/certificate_provisioning.h
@@ -64,6 +64,12 @@ class CertificateProvisioning {
       const std::string& provisioning_response, std::string* result);
 
  private:
+  CdmResponseType GetProvisioningRequestInternal(
+      SecurityLevel requested_security_level, CdmCertificateType cert_type,
+      const std::string& cert_authority, const std::string& origin,
+      const std::string& spoid, CdmProvisioningRequest* request,
+      std::string* default_url);
+
   CdmResponseType SetSpoidParameter(
       const std::string& origin, const std::string& spoid,
       video_widevine::ProvisioningRequest* request);
@@ -71,6 +77,15 @@ class CertificateProvisioning {
   video_widevine::SignedProvisioningMessage::ProvisioningType
   GetProvisioningType();
 
+  // Closes crypto session if one is open. Avoid calling this method when
+  // processing a response. Multiple provisioning responses might be
+  // simultaneously in flight. Only the response associated with the last
+  // provisioning request can be processed. All the other responses will
+  // fail. If the session is closed when these responses fail, even the one
+  // associated with the last provisioning request may fail.
+  CdmResponseType CloseSessionOnError(CdmResponseType status);
+  void CloseSession();
+
   std::unique_ptr<CryptoSession> crypto_session_;
   CdmCertificateType cert_type_;
   std::unique_ptr<ServiceCertificate> service_certificate_;
diff --git a/libwvdrmengine/cdm/core/src/certificate_provisioning.cpp b/libwvdrmengine/cdm/core/src/certificate_provisioning.cpp
index a957ab95..87fe2180 100644
--- a/libwvdrmengine/cdm/core/src/certificate_provisioning.cpp
+++ b/libwvdrmengine/cdm/core/src/certificate_provisioning.cpp
@@ -194,6 +194,16 @@ CdmResponseType CertificateProvisioning::GetProvisioningRequest(
     const std::string& cert_authority, const std::string& origin,
     const std::string& spoid, CdmProvisioningRequest* request,
     std::string* default_url) {
+  return CloseSessionOnError(GetProvisioningRequestInternal(
+      requested_security_level, cert_type, cert_authority, origin, spoid,
+      request, default_url));
+}
+
+CdmResponseType CertificateProvisioning::GetProvisioningRequestInternal(
+    SecurityLevel requested_security_level, CdmCertificateType cert_type,
+    const std::string& cert_authority, const std::string& origin,
+    const std::string& spoid, CdmProvisioningRequest* request,
+    std::string* default_url) {
   if (!request || !default_url) {
     LOGE("Output parameter |%s| is not provided",
          request ? "default_url" : "request");
@@ -202,7 +212,7 @@ CdmResponseType CertificateProvisioning::GetProvisioningRequest(
 
   default_url->assign(kProvisioningServerUrl);
 
-  if (crypto_session_->IsOpen()) crypto_session_->Close();
+  CloseSession();
   CdmResponseType status = crypto_session_->Open(requested_security_level);
   if (NO_ERROR != status) {
     LOGE("Failed to create a crypto session: status = %d",
@@ -423,7 +433,7 @@ CdmResponseType CertificateProvisioning::HandleProvisioningResponse(
   }
 
   const CdmSecurityLevel security_level = crypto_session_->GetSecurityLevel();
-  crypto_session_->Close();
+  CloseSession();
 
   // This is the entire certificate (SignedDrmDeviceCertificate).
   const std::string& device_cert_data =
@@ -521,4 +531,14 @@ bool CertificateProvisioning::ExtractDeviceInfo(
   return true;
 }
 
+void CertificateProvisioning::CloseSession() {
+  if (crypto_session_->IsOpen()) crypto_session_->Close();
+}
+
+CdmResponseType CertificateProvisioning::CloseSessionOnError(
+    CdmResponseType status) {
+  if (status != NO_ERROR) CloseSession();
+  return status;
+}
+
 }  // namespace wvcdm