Skip to content

Commit

Permalink
Make SessionResumptionStorage injectable in controller. (#28422)
Browse files Browse the repository at this point in the history 
* Make SessionResumptionStorage injectable in controller.

Fixes #28351

* Address review comment.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Oct 25, 2023
1 parent 13b4a6a commit ca3c297
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 12 deletions.
27 changes: 21 additions & 6 deletions src/controller/CHIPDeviceControllerFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -94,6 +95,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState()
params.operationalKeystore = mOperationalKeystore;
params.opCertStore = mOpCertStore;
params.certificateValidityPolicy = mCertificateValidityPolicy;
params.sessionResumptionStorage = mSessionResumptionStorage;
}

return InitSystemState(params);
Expand Down Expand Up @@ -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();
Expand All @@ -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));

//
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -373,6 +387,7 @@ void DeviceControllerFactory::Shutdown()
mOperationalKeystore = nullptr;
mOpCertStore = nullptr;
mCertificateValidityPolicy = nullptr;
mSessionResumptionStorage = nullptr;
}

void DeviceControllerSystemState::Shutdown()
Expand Down
7 changes: 5 additions & 2 deletions src/controller/CHIPDeviceControllerFactory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -257,6 +259,7 @@ class DeviceControllerFactory
Crypto::OperationalKeystore * mOperationalKeystore = nullptr;
Credentials::OperationalCertificateStore * mOpCertStore = nullptr;
Credentials::CertificateValidityPolicy * mCertificateValidityPolicy = nullptr;
SessionResumptionStorage * mSessionResumptionStorage = nullptr;
bool mEnableServerInteractions = false;
};

Expand Down
27 changes: 23 additions & 4 deletions src/controller/CHIPDeviceControllerSystemState.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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; };
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ca3c297

Please sign in to comment.