Make SessionResumptionStorage injectable in controller. (#28422)

* Make SessionResumptionStorage injectable in controller. Fixes #28351 * Address review comment.
project-chip · Aug 21, 2023 · 461c69e · 461c69e
1 parent 3b12679
commit 461c69e
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 12 deletions.
diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -65,6 +65,7 @@ CHIP_ERROR DeviceControllerFactory::Init(FactoryInitParams params)
     mOperationalKeystore       = params.operationalKeystore;
     mOpCertStore               = params.opCertStore;
     mCertificateValidityPolicy = params.certificateValidityPolicy;
+    mSessionResumptionStorage  = params.sessionResumptionStorage;
     mEnableServerInteractions  = params.enableServerInteractions;
 
     CHIP_ERROR err = InitSystemState(params);
@@ -94,6 +95,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState()
         params.operationalKeystore       = mOperationalKeystore;
         params.opCertStore               = mOpCertStore;
         params.certificateValidityPolicy = mCertificateValidityPolicy;
+        params.sessionResumptionStorage  = mSessionResumptionStorage;
     }
 
     return InitSystemState(params);
@@ -195,12 +197,24 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
         tempFabricTable         = stateParams.fabricTable;
     }
 
-    auto sessionResumptionStorage = chip::Platform::MakeUnique<SimpleSessionResumptionStorage>();
-    ReturnErrorOnFailure(sessionResumptionStorage->Init(params.fabricIndependentStorage));
-    stateParams.sessionResumptionStorage = std::move(sessionResumptionStorage);
+    SessionResumptionStorage * sessionResumptionStorage;
+    if (params.sessionResumptionStorage == nullptr)
+    {
+        auto ownedSessionResumptionStorage = chip::Platform::MakeUnique<SimpleSessionResumptionStorage>();
+        ReturnErrorOnFailure(ownedSessionResumptionStorage->Init(params.fabricIndependentStorage));
+        stateParams.ownedSessionResumptionStorage    = std::move(ownedSessionResumptionStorage);
+        stateParams.externalSessionResumptionStorage = nullptr;
+        sessionResumptionStorage                     = stateParams.ownedSessionResumptionStorage.get();
+    }
+    else
+    {
+        stateParams.ownedSessionResumptionStorage    = nullptr;
+        stateParams.externalSessionResumptionStorage = params.sessionResumptionStorage;
+        sessionResumptionStorage                     = stateParams.externalSessionResumptionStorage;
+    }
 
     auto delegate = chip::Platform::MakeUnique<ControllerFabricDelegate>();
-    ReturnErrorOnFailure(delegate->Init(stateParams.sessionResumptionStorage.get(), stateParams.groupDataProvider));
+    ReturnErrorOnFailure(delegate->Init(sessionResumptionStorage, stateParams.groupDataProvider));
     stateParams.fabricTableDelegate = delegate.get();
     ReturnErrorOnFailure(stateParams.fabricTable->AddFabricDelegate(stateParams.fabricTableDelegate));
     delegate.release();
@@ -222,7 +236,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
 
         // Enable listening for session establishment messages.
         ReturnErrorOnFailure(stateParams.caseServer->ListenForSessionEstablishment(
-            stateParams.exchangeMgr, stateParams.sessionMgr, stateParams.fabricTable, stateParams.sessionResumptionStorage.get(),
+            stateParams.exchangeMgr, stateParams.sessionMgr, stateParams.fabricTable, sessionResumptionStorage,
             stateParams.certificateValidityPolicy, stateParams.groupDataProvider));
 
         //
@@ -256,7 +270,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
 
     CASEClientInitParams sessionInitParams = {
         .sessionManager            = stateParams.sessionMgr,
-        .sessionResumptionStorage  = stateParams.sessionResumptionStorage.get(),
+        .sessionResumptionStorage  = sessionResumptionStorage,
         .certificateValidityPolicy = stateParams.certificateValidityPolicy,
         .exchangeMgr               = stateParams.exchangeMgr,
         .fabricTable               = stateParams.fabricTable,
@@ -373,6 +387,7 @@ void DeviceControllerFactory::Shutdown()
     mOperationalKeystore       = nullptr;
     mOpCertStore               = nullptr;
     mCertificateValidityPolicy = nullptr;
+    mSessionResumptionStorage  = nullptr;
 }
 
 void DeviceControllerSystemState::Shutdown()

diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h
@@ -34,6 +34,7 @@
 #include <credentials/GroupDataProvider.h>
 #include <credentials/OperationalCertificateStore.h>
 #include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
+#include <protocols/secure_channel/SessionResumptionStorage.h>
 
 namespace chip {
 
@@ -106,8 +107,8 @@ struct SetupParams
 };
 
 // TODO everything other than the fabric storage, group data provider, OperationalKeystore,
-// OperationalCertificateStore and SessionKeystore here should be removed. We're blocked
-// because of the need to support !CHIP_DEVICE_LAYER
+// OperationalCertificateStore, SessionKeystore, and SessionResumptionStorage here should
+// be removed. We're blocked because of the need to support !CHIP_DEVICE_LAYER
 struct FactoryInitParams
 {
     System::Layer * systemLayer                                        = nullptr;
@@ -121,6 +122,7 @@ struct FactoryInitParams
     FabricTable * fabricTable                                          = nullptr;
     OperationalKeystore * operationalKeystore                          = nullptr;
     Credentials::OperationalCertificateStore * opCertStore             = nullptr;
+    SessionResumptionStorage * sessionResumptionStorage                = nullptr;
 #if CONFIG_NETWORK_LAYER_BLE
     Ble::BleLayer * bleLayer = nullptr;
 #endif
@@ -257,6 +259,7 @@ class DeviceControllerFactory
     Crypto::OperationalKeystore * mOperationalKeystore                  = nullptr;
     Credentials::OperationalCertificateStore * mOpCertStore             = nullptr;
     Credentials::CertificateValidityPolicy * mCertificateValidityPolicy = nullptr;
+    SessionResumptionStorage * mSessionResumptionStorage                = nullptr;
     bool mEnableServerInteractions                                      = false;
 };
 

diff --git a/src/controller/CHIPDeviceControllerSystemState.h b/src/controller/CHIPDeviceControllerSystemState.h
@@ -85,10 +85,18 @@ struct DeviceControllerSystemStateParams
     Credentials::GroupDataProvider * groupDataProvider = nullptr;
     Crypto::SessionKeystore * sessionKeystore          = nullptr;
 
+    // NOTE: Exactly one of externalSessionResumptionStorage (externally provided,
+    // externally owned) or ownedSessionResumptionStorage (managed by the system
+    // state) must be non-null.
+    SessionResumptionStorage * externalSessionResumptionStorage = nullptr;
+
     // Params that will be deallocated via Platform::Delete in
     // DeviceControllerSystemState::Shutdown.
     DeviceTransportMgr * transportMgr = nullptr;
-    Platform::UniquePtr<SimpleSessionResumptionStorage> sessionResumptionStorage;
+    // NOTE: Exactly one of externalSessionResumptionStorage (externally provided,
+    // externally owned) or ownedSessionResumptionStorage (managed by the system
+    // state) must be non-null.
+    Platform::UniquePtr<SimpleSessionResumptionStorage> ownedSessionResumptionStorage;
     Credentials::CertificateValidityPolicy * certificateValidityPolicy            = nullptr;
     SessionManager * sessionMgr                                                   = nullptr;
     Protocols::SecureChannel::UnsolicitedStatusHandler * unsolicitedStatusHandler = nullptr;
@@ -132,8 +140,18 @@ class DeviceControllerSystemState
         mCASESessionManager(params.caseSessionManager), mSessionSetupPool(params.sessionSetupPool),
         mCASEClientPool(params.caseClientPool), mGroupDataProvider(params.groupDataProvider), mTimerDelegate(params.timerDelegate),
         mReportScheduler(params.reportScheduler), mSessionKeystore(params.sessionKeystore),
-        mFabricTableDelegate(params.fabricTableDelegate), mSessionResumptionStorage(std::move(params.sessionResumptionStorage))
+        mFabricTableDelegate(params.fabricTableDelegate),
+        mOwnedSessionResumptionStorage(std::move(params.ownedSessionResumptionStorage))
     {
+        if (mOwnedSessionResumptionStorage)
+        {
+            mSessionResumptionStorage = mOwnedSessionResumptionStorage.get();
+        }
+        else
+        {
+            mSessionResumptionStorage = params.externalSessionResumptionStorage;
+        }
+
 #if CONFIG_NETWORK_LAYER_BLE
         mBleLayer = params.bleLayer;
 #endif
@@ -172,7 +190,7 @@ class DeviceControllerSystemState
             mUnsolicitedStatusHandler != nullptr && mExchangeMgr != nullptr && mMessageCounterManager != nullptr &&
             mFabrics != nullptr && mCASESessionManager != nullptr && mSessionSetupPool != nullptr && mCASEClientPool != nullptr &&
             mGroupDataProvider != nullptr && mReportScheduler != nullptr && mTimerDelegate != nullptr &&
-            mSessionKeystore != nullptr;
+            mSessionKeystore != nullptr && mSessionResumptionStorage != nullptr;
     };
 
     System::Layer * SystemLayer() const { return mSystemLayer; };
@@ -221,7 +239,8 @@ class DeviceControllerSystemState
     app::reporting::ReportScheduler * mReportScheduler                             = nullptr;
     Crypto::SessionKeystore * mSessionKeystore                                     = nullptr;
     FabricTable::Delegate * mFabricTableDelegate                                   = nullptr;
-    Platform::UniquePtr<SimpleSessionResumptionStorage> mSessionResumptionStorage;
+    SessionResumptionStorage * mSessionResumptionStorage                           = nullptr;
+    Platform::UniquePtr<SimpleSessionResumptionStorage> mOwnedSessionResumptionStorage;
 
     // If mTempFabricTable is not null, it was created during
     // DeviceControllerFactory::InitSystemState and needs to be