Reset crypto session pointers on RemoveKeys.

[ Merge of http://go/wvgerrit/189590 ]
[ Cherry-pick of http://ag/26541307 ]

The CDM session shares its CryptoSession instance with a few additional
member objects (CdmLicense and PolicyEngine).  When the CDM session's
crypto session is reset, it must also reset the CdmLicense and
PolicyEngine otherwise, a potential stale pointer reference may occur.

Test: request_license_test on Oriole
Test: run_x86_64_tests
Bug: 311239278
Change-Id: Ie175513ae652dcd96e12e5e1def574a8a56d5863

libwvdrmengine/cdm/core/include/cdm_session.h

@@ -265,6 +265,8 @@ class CdmSession {
 
   bool HasRootOfTrustBeenRenewed();
 
+  CdmResponseType ResetCryptoSession();
+
   // 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);
@@ -340,8 +342,9 @@ class CdmSession {
   bool has_license_been_loaded_ = false;
   bool has_license_been_restored_ = false;
 
-  bool mock_license_parser_in_use_;
+  bool mock_crypto_session_in_use_ = false;
-  bool mock_policy_engine_in_use_;
+  bool mock_license_parser_in_use_ = false;
+  bool mock_policy_engine_in_use_ = false;
 
   CORE_DISALLOW_COPY_AND_ASSIGN(CdmSession);
 };

libwvdrmengine/cdm/core/include/license.h

@@ -90,6 +90,15 @@ class CdmLicense {
       const CdmKeyResponse& license_response,
       std::string* provider_session_token);
 
+  // Testing only.  Caller retains ownership of pointers.
+  void set_crypto_session(CryptoSession* crypto_session) {
+    crypto_session_ = crypto_session;
+  }
+
+  void set_policy_engine(PolicyEngine* policy_engine) {
+    policy_engine_ = policy_engine;
+  }
+
  private:
   CdmResponseType HandleKeyErrorResponse(
       const video_widevine::SignedMessage& signed_message);
@@ -131,8 +140,8 @@ class CdmLicense {
   bool SetTypeAndId(CdmLicenseType license_type, const std::string& request_id,
                     T* content_id);
 
-  CryptoSession* crypto_session_;
+  CryptoSession* crypto_session_ = nullptr;
-  PolicyEngine* policy_engine_;
+  PolicyEngine* policy_engine_ = nullptr;
   std::string server_url_;
   std::string client_token_;
   const CdmSessionId session_id_;

libwvdrmengine/cdm/core/include/policy_engine.h

@@ -106,6 +106,8 @@ class PolicyEngine {
 
   virtual const LicenseIdentification& license_id() { return license_id_; }
 
+  WvCdmEventListener* event_listener() { return event_listener_; }
+
   bool GetSecondsSinceStarted(int64_t* seconds_since_started);
   bool GetSecondsSinceLastPlayed(int64_t* seconds_since_started);
 
@@ -131,6 +133,11 @@ class PolicyEngine {
     return license_keys_->MeetsConstraints(key_id);
   }
 
+  // Testing only.  Caller retains ownership.
+  void set_crypto_session(CryptoSession* crypto_session) {
+    crypto_session_ = crypto_session;
+  }
+
  private:
   friend class PolicyEngineTest;
   friend class PolicyEngineConstraintsTest;

libwvdrmengine/cdm/core/include/wv_cdm_types.h

@@ -945,6 +945,8 @@ const char* IdToString(const std::string& id);
 // provided as string pointers.
 const char* IdPtrToString(const std::string* id);
 
+const char* BoolToString(bool value);
+
 // Logging utilities for OEMCrypto types.
 const char* OemCryptoResultToString(OEMCryptoResult result);
 }  // namespace wvcdm

libwvdrmengine/cdm/core/src/cdm_session.cpp

@@ -81,9 +81,7 @@ CdmSession::CdmSession(wvutil::FileSystem* file_system,
       security_level_(kSecurityLevelUninitialized),
       requested_security_level_(kLevelDefault),
       is_initial_usage_update_(true),
-      is_usage_update_needed_(false),
+      is_usage_update_needed_(false) {
-      mock_license_parser_in_use_(false),
-      mock_policy_engine_in_use_(false) {
   assert(metrics_);  // metrics_ must not be null.
   crypto_metrics_ = metrics_->GetCryptoMetrics();
   crypto_session_.reset(CryptoSession::MakeCryptoSession(crypto_metrics_));
@@ -866,18 +864,10 @@ CdmResponseType CdmSession::DeleteUsageEntry(uint32_t usage_entry_index) {
   // The usage entry cannot be deleted if it has a crypto session handling
   // it, so close and reopen session.
   UpdateUsageEntryInformation();
-  CdmResponseType sts;
-  crypto_session_->Close();
-  crypto_session_.reset(CryptoSession::MakeCryptoSession(crypto_metrics_));
-  M_TIME(sts = crypto_session_->Open(requested_security_level_),
-         crypto_metrics_, crypto_session_open_, sts, requested_security_level_);
-  if (sts != NO_ERROR) return sts;
 
-  usage_table_ = nullptr;
+  crypto_session_->Close();
-  bool has_support = false;
+  CdmResponseType sts = ResetCryptoSession();
-  if (crypto_session_->HasUsageTableSupport(&has_support) && has_support) {
+  if (sts != NO_ERROR) return sts;
-    usage_table_ = crypto_session_->GetUsageTable();
-  }
 
   if (usage_table_ == nullptr) {
     LOGE("Usage table header unavailable");
@@ -1012,14 +1002,8 @@ bool CdmSession::StoreLicense(CdmOfflineLicenseState state, int* error_detail) {
 }
 
 CdmResponseType CdmSession::RemoveKeys() {
-  CdmResponseType sts;
+  crypto_session_->Close();
-  crypto_session_.reset(CryptoSession::MakeCryptoSession(crypto_metrics_));
+  return ResetCryptoSession();
-  // Ignore errors
-  M_TIME(sts = crypto_session_->Open(requested_security_level_),
-         crypto_metrics_, crypto_session_open_, sts, requested_security_level_);
-  policy_engine_.reset(
-      new PolicyEngine(session_id_, nullptr, crypto_session_.get()));
-  return CdmResponseType(NO_ERROR);
 }
 
 CdmResponseType CdmSession::RemoveLicense() {
@@ -1313,6 +1297,77 @@ CdmResponseType CdmSession::GenerateRsaSignature(const std::string& message,
   return crypto_session_->GenerateRsaSignature(message, signature, scheme);
 }
 
+CdmResponseType CdmSession::ResetCryptoSession() {
+  LOGD("Resetting crypto session: session_id = %s, ksid = %s",
+       IdToString(session_id_), IdToString(key_set_id_));
+  if (mock_crypto_session_in_use_) {
+    // If the crypto session is not reset, then there is nothing to do.
+    return CdmResponseType(NO_ERROR);
+  }
+  CdmResponseType sts;
+  crypto_session_.reset(CryptoSession::MakeCryptoSession(crypto_metrics_));
+  usage_table_ = nullptr;
+  M_TIME(sts = crypto_session_->Open(requested_security_level_),
+         crypto_metrics_, crypto_session_open_, sts, requested_security_level_);
+
+  CdmResponseType final_sts(NO_ERROR);
+  if (sts != NO_ERROR) {
+    // Challenging case, still need to reset other components.
+    LOGE("Failed to open crypto session: sts = %s", sts.ToString().c_str());
+    final_sts = sts;
+  } else {
+    // Reset all component dependent on the crypto session.
+    security_level_ = crypto_session_->GetSecurityLevel();
+    crypto_metrics_->crypto_session_security_level_.Record(security_level_);
+
+    if (!file_handle_->Init(security_level_)) {
+      LOGE("Unable to initialize file handle");
+      final_sts = CdmResponseType(SESSION_FILE_HANDLE_INIT_ERROR);
+    }
+
+    if (!file_handle_->HasCertificate(atsc_mode_enabled_)) {
+      LOGE("Missing certificate: atsc_mode_enabled = %s",
+           BoolToString(atsc_mode_enabled_));
+      final_sts = CdmResponseType(NEED_PROVISIONING);
+    }
+
+    bool has_support = false;
+    if (crypto_session_->HasUsageTableSupport(&has_support) && has_support) {
+      usage_table_ = crypto_session_->GetUsageTable();
+    }
+  }
+
+  // Even if the session is not open, other members need new session pointer.
+  if (mock_policy_engine_in_use_) {
+    // Simply pass the new pointer.
+    policy_engine_->set_crypto_session(crypto_session_.get());
+  } else {
+    // Attempt to maintain event listener.
+    WvCdmEventListener* event_listener =
+        policy_engine_ ? policy_engine_->event_listener() : nullptr;
+    policy_engine_.reset(
+        new PolicyEngine(session_id_, event_listener, crypto_session_.get()));
+  }
+
+  if (mock_license_parser_in_use_) {
+    license_parser_->set_crypto_session(crypto_session_.get());
+    license_parser_->set_policy_engine(policy_engine_.get());
+  } else {
+    license_parser_.reset(new CdmLicense(session_id_));
+    std::string service_certificate;
+    if (!Properties::GetServiceCertificate(session_id_, &service_certificate))
+      service_certificate.clear();
+    if (!license_parser_->Init(Properties::UsePrivacyMode(session_id_),
+                               service_certificate, crypto_session_.get(),
+                               policy_engine_.get())) {
+      LOGE("Failed to initialize license parser");
+      final_sts = CdmResponseType(LICENSE_PARSER_INIT_ERROR);
+    }
+  }
+
+  return final_sts;
+}
+
 // For testing only - takes ownership of pointers
 
 void CdmSession::set_license_parser(CdmLicense* license_parser) {
@@ -1322,6 +1377,7 @@ void CdmSession::set_license_parser(CdmLicense* license_parser) {
 
 void CdmSession::set_crypto_session(CryptoSession* crypto_session) {
   crypto_session_.reset(crypto_session);
+  mock_crypto_session_in_use_ = true;
 }
 
 void CdmSession::set_policy_engine(PolicyEngine* policy_engine) {

libwvdrmengine/cdm/core/src/license.cpp

@@ -195,9 +195,7 @@ std::vector<CryptoKey> ExtractContentKeys(
 }  // namespace
 
 CdmLicense::CdmLicense(const CdmSessionId& session_id)
-    : crypto_session_(nullptr),
+    : session_id_(session_id),
-      policy_engine_(nullptr),
-      session_id_(session_id),
       initialized_(false),
       protocol_version_(video_widevine::VERSION_2_2),
       renew_with_client_id_(false),
@@ -207,16 +205,18 @@ CdmLicense::CdmLicense(const CdmSessionId& session_id)
       license_key_type_(kLicenseKeyTypeContent) {}
 
 CdmLicense::CdmLicense(const CdmSessionId& session_id, wvutil::Clock* clock)
-    : crypto_session_(nullptr),
+    : session_id_(session_id),
-      policy_engine_(nullptr),
-      session_id_(session_id),
       initialized_(false),
       protocol_version_(video_widevine::VERSION_2_2),
       renew_with_client_id_(false),
       is_offline_(false),
       use_privacy_mode_(false),
+      clock_(clock),
       license_key_type_(kLicenseKeyTypeContent) {
-  clock_.reset(clock);
+  if (!clock_) {
+    LOGW("Input |clock| is null, using default");
+    clock_.reset(new wvutil::Clock());
+  }
 }
 
 CdmLicense::~CdmLicense() {}

libwvdrmengine/cdm/core/src/policy_engine.cpp

@@ -39,9 +39,12 @@ PolicyEngine::PolicyEngine(CdmSessionId session_id,
     if (version >= kMinOemCryptoApiVersionSupportsRenewalDelayBase) {
       policy_timers_.reset(new PolicyTimersV18());
     }
+  } else {
+    LOGW("Failed to get API version: session_id = %s", IdToString(session_id));
   }
 
-  if (policy_timers_ == nullptr) {
+  if (!policy_timers_) {
+    // Use V16 policy timers if getting version failed.
     policy_timers_.reset(new PolicyTimersV16());
   }
   InitDevice(crypto_session);

libwvdrmengine/cdm/core/src/wv_cdm_types.cpp

@@ -12,6 +12,8 @@ namespace wvcdm {
 namespace {
 const char kEmptyIdRep[] = "<empty>";
 const char kNullIdRep[] = "<null>";
+const char kFalseRep[] = "false";
+const char kTrueRep[] = "true";
 
 // Thread local buffer used by UnknownEnumValueToString() to represent
 // unknown enum values.
@@ -894,6 +896,8 @@ const char* IdPtrToString(const std::string* id) {
   return id->empty() ? kEmptyIdRep : id->c_str();
 }
 
+const char* BoolToString(bool value) { return value ? kTrueRep : kFalseRep; }
+
 const char* OemCryptoResultToString(OEMCryptoResult result) {
   switch (result) {
     /* OEMCrypto return values */

libwvdrmengine/cdm/core/test/license_unittest.cpp

@@ -136,6 +136,7 @@ const std::string kFakeKeyTooLong =
 const std::string kFakeKeyTooShort = a2bs_hex("06e247e7f924208011");
 const std::string kFakeIv = a2bs_hex("3d515a3ee0be1687080ac59da9e0d69a");
 const std::string kFakeBuildInfo = "Mock Crypto Session - License Test";
+const uint32_t kDefaultOemCryptoVersion = 18;
 
 class MockCryptoSession : public TestCryptoSession {
  public:
@@ -215,84 +216,85 @@ class CdmLicenseTest : public WvCdmTestBase {
  protected:
   CdmLicenseTest(const std::string& pssh = (kCencInitDataHdr + kCencPssh))
       : pssh_(pssh) {}
+
   void SetUp() override {
     WvCdmTestBase::SetUp();
-    clock_ = new MockClock();
+    crypto_session_.reset(new MockCryptoSession(&crypto_metrics_));
-    crypto_session_ = new MockCryptoSession(&crypto_metrics_);
-    init_data_ = new InitializationData(CENC_INIT_DATA_FORMAT, pssh_);
-    policy_engine_ = new MockPolicyEngine(crypto_session_);
-
     ON_CALL(*crypto_session_, GetSupportedCertificateTypes(NotNull()))
         .WillByDefault(
             DoAll(SetArgPointee<0>(kDefaultSupportedCertTypes), Return(true)));
+    // PolicyEngine will call GetApiVersion() on creation.
+    EXPECT_CALL(*crypto_session_, GetApiVersion(NotNull()))
+        .WillRepeatedly(
+            DoAll(SetArgPointee<0>(kDefaultOemCryptoVersion), Return(true)));
+
+    policy_engine_.reset(new MockPolicyEngine(crypto_session_.get()));
+
+    init_data_ = InitializationData(CENC_INIT_DATA_FORMAT, pssh_);
+
+    clock_ = new MockClock();
+    cdm_license_.reset(new CdmLicenseTestPeer(kCdmSessionId, clock_));
   }
 
   void TearDown() override {
-    delete cdm_license_;
+    // Nullify pointers for objects owned by CdmLicense.
-    delete policy_engine_;
-    delete init_data_;
-    delete crypto_session_;
-    delete clock_;
-  }
-
-  virtual void CreateCdmLicense() {
-    cdm_license_ = new CdmLicenseTestPeer(kCdmSessionId, clock_);
     clock_ = nullptr;
+
+    cdm_license_.reset();
+
+    // Release mock objects used by the CdmLicense.
+    // Order is important.
+    policy_engine_.reset();
+    crypto_session_.reset();
   }
 
-  CdmLicenseTestPeer* cdm_license_ = nullptr;
-  MockClock* clock_ = nullptr;
   metrics::CryptoMetrics crypto_metrics_;
-  MockCryptoSession* crypto_session_ = nullptr;
+  MockClock* clock_ = nullptr;  // Owned by |cdm_license_|.
-  InitializationData* init_data_ = nullptr;
+  std::unique_ptr<CdmLicenseTestPeer> cdm_license_;
-  MockPolicyEngine* policy_engine_ = nullptr;
+  std::unique_ptr<MockPolicyEngine> policy_engine_;
+  std::unique_ptr<MockCryptoSession> crypto_session_;
+
+  InitializationData init_data_;
   std::string pssh_;
 };
 
 TEST_F(CdmLicenseTest, InitSuccess) {
   EXPECT_CALL(*crypto_session_, IsOpen()).WillOnce(Return(true));
-
-  CreateCdmLicense();
   EXPECT_TRUE(cdm_license_->Init(false, kEmptyServiceCertificate,
-                                 crypto_session_, policy_engine_));
+                                 crypto_session_.get(), policy_engine_.get()));
 }
 
 TEST_F(CdmLicenseTest, InitFail_CryptoSessionNull) {
-  CreateCdmLicense();
   EXPECT_FALSE(cdm_license_->Init(false, kEmptyServiceCertificate, nullptr,
-                                  policy_engine_));
+                                  policy_engine_.get()));
 }
 
 TEST_F(CdmLicenseTest, InitFail_PolicyEngineNull) {
   EXPECT_CALL(*crypto_session_, IsOpen()).WillOnce(Return(true));
 
-  CreateCdmLicense();
   EXPECT_FALSE(cdm_license_->Init(false, kEmptyServiceCertificate,
-                                  crypto_session_, nullptr));
+                                  crypto_session_.get(), nullptr));
 }
 
 TEST_F(CdmLicenseTest, InitWithEmptyServiceCert) {
   EXPECT_CALL(*crypto_session_, IsOpen()).WillOnce(Return(true));
 
-  CreateCdmLicense();
   EXPECT_TRUE(cdm_license_->Init(true, kEmptyServiceCertificate,
-                                 crypto_session_, policy_engine_));
+                                 crypto_session_.get(), policy_engine_.get()));
 }
 
 TEST_F(CdmLicenseTest, InitWithInvalidServiceCert) {
   EXPECT_CALL(*crypto_session_, IsOpen()).WillOnce(Return(true));
 
-  CreateCdmLicense();
   EXPECT_FALSE(cdm_license_->Init(true, kInvalidServiceCertificate,
-                                  crypto_session_, policy_engine_));
+                                  crypto_session_.get(), policy_engine_.get()));
 }
 
 TEST_F(CdmLicenseTest, InitWithServiceCert) {
   EXPECT_CALL(*crypto_session_, IsOpen()).WillOnce(Return(true));
 
-  CreateCdmLicense();
   EXPECT_TRUE(cdm_license_->Init(true, kDefaultServiceCertificate,
-                                 crypto_session_, policy_engine_));
+                                 crypto_session_.get(), policy_engine_.get()));
 }
 
 TEST_F(CdmLicenseTest, PrepareKeyRequestValidation) {
@@ -335,15 +337,14 @@ TEST_F(CdmLicenseTest, PrepareKeyRequestValidation) {
       .WillOnce(
           DoAll(SetArgPointee<0>(kWatermarkingConfigurable), Return(true)));
 
-  CreateCdmLicense();
   EXPECT_TRUE(cdm_license_->Init(true, kDefaultServiceCertificate,
-                                 crypto_session_, policy_engine_));
+                                 crypto_session_.get(), policy_engine_.get()));
 
   CdmAppParameterMap app_parameters;
   CdmKeyMessage signed_request;
   std::string server_url;
   EXPECT_EQ(cdm_license_->PrepareKeyRequest(
-                *init_data_, kToken, kLicenseTypeStreaming, app_parameters,
+                init_data_, kToken, kLicenseTypeStreaming, app_parameters,
                 &signed_request, &server_url),
             KEY_MESSAGE);
 
@@ -470,15 +471,14 @@ TEST_F(CdmLicenseTest, PrepareKeyRequestValidationV15) {
       .WillOnce(
           DoAll(SetArgPointee<0>(kWatermarkingNotSupported), Return(true)));
 
-  CreateCdmLicense();
   EXPECT_TRUE(cdm_license_->Init(true, kDefaultServiceCertificate,
-                                 crypto_session_, policy_engine_));
+                                 crypto_session_.get(), policy_engine_.get()));
 
   CdmAppParameterMap app_parameters;
   CdmKeyMessage signed_request;
   std::string server_url;
   EXPECT_EQ(cdm_license_->PrepareKeyRequest(
-                *init_data_, kToken, kLicenseTypeStreaming, app_parameters,
+                init_data_, kToken, kLicenseTypeStreaming, app_parameters,
                 &signed_request, &server_url),
             KEY_MESSAGE);
 
@@ -616,9 +616,8 @@ TEST_P(CdmLicenseEntitledKeyTest, LoadsEntitledKeys) {
   }
 
   // Set up the CdmLicense with the mocks and fake entitlement key
-  CreateCdmLicense();
   ASSERT_TRUE(cdm_license_->Init(true, kDefaultServiceCertificate,
-                                 crypto_session_, policy_engine_));
+                                 crypto_session_.get(), policy_engine_.get()));
   cdm_license_->set_entitlement_keys(entitlement_license);
 
   // Call the function under test and check its return value

libwvdrmengine/cdm/core/test/policy_engine_unittest.cpp

@@ -267,7 +267,7 @@ class PolicyEngineTestV18 : public PolicyEngineTest {
   }
 };
 
-TEST_F(PolicyEngineTest, NoLicense) {
+TEST_F(PolicyEngineTestV16, NoLicense) {
   EXPECT_FALSE(policy_engine_->CanDecryptContent(kKeyId));
 }
 
@@ -2826,6 +2826,10 @@ TEST_F(PolicyEngineTestV16, PlaybackOk_RestoreWithoutPlaybackTimes) {
   EXPECT_TRUE(policy_engine_->CanDecryptContent(kKeyId));
 }
 
+TEST_F(PolicyEngineTestV18, NoLicense) {
+  EXPECT_FALSE(policy_engine_->CanDecryptContent(kKeyId));
+}
+
 // These tests exercise license policy when OEMCrypto supports v18.
 // The following scenarios are from the duration-and-renewal doc.
 // Verifies correct reporting of events, OnSessionRenewalNeeded,

libwvdrmengine/cdm/test/request_license_test.cpp

@@ -3467,6 +3467,26 @@ TEST_F(WvCdmRequestLicenseTest, StreamingLicenseRenewalProhibited) {
   decryptor_->CloseSession(session_id_);
 }
 
+TEST_F(WvCdmRequestLicenseTest, StreamingLicense_WithCallToRemoveKeys) {
+  Unprovision();
+  Provision();
+
+  decryptor_->OpenSession(config_.key_system(), nullptr, kDefaultCdmIdentifier,
+                          nullptr, &session_id_);
+  GenerateKeyRequest(binary_key_id(), kLicenseTypeStreaming);
+  const std::string license_response =
+      GetKeyRequestResponse(config_.license_server(), config_.client_auth());
+  ASSERT_FALSE(license_response.empty());
+
+  EXPECT_EQ(wvcdm::NO_ERROR, decryptor_->RemoveKeys(session_id_));
+
+  // Should fail gracefully.
+  EXPECT_NE(wvcdm::KEY_ADDED,
+            decryptor_->AddKey(session_id_, license_response, &key_set_id_));
+
+  decryptor_->CloseSession(session_id_);
+}
+
 TEST_F(WvCdmRequestLicenseTest, OfflineLicenseRenewal) {
   Unprovision();
   Provision();