From b4ecb1aa1449aae9fd02cee75bc9a41775807aba Mon Sep 17 00:00:00 2001 From: tehampson Date: Fri, 24 Jun 2022 11:04:46 -0400 Subject: [PATCH] Invalidate CASE resumption storage when UpdateNOC is called (#19860) Using FabricTable::Delegate callback we clean up CASE resumption storage for associated fabric index on UpdateNOC --- src/app/server/Server.h | 31 +++++++++--------- .../CHIPDeviceControllerFactory.cpp | 2 +- src/controller/CHIPDeviceControllerFactory.h | 32 ++++++++++++------- src/transport/SessionManager.cpp | 10 +++++- src/transport/SessionManager.h | 24 +++++++++++++- 5 files changed, 68 insertions(+), 31 deletions(-) diff --git a/src/app/server/Server.h b/src/app/server/Server.h index bc07651b664ed7..20143f941e62a1 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -396,21 +396,7 @@ class Server void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override { (void) fabricTable; - auto & sessionManager = mServer->GetSecureSessionManager(); - sessionManager.FabricRemoved(fabricIndex); - - // Remove all CASE session resumption state - auto * sessionResumptionStorage = mServer->GetSessionResumptionStorage(); - if (sessionResumptionStorage != nullptr) - { - CHIP_ERROR err = sessionResumptionStorage->DeleteAll(fabricIndex); - if (err != CHIP_NO_ERROR) - { - ChipLogError(AppServer, - "Warning, failed to delete session resumption state for fabric index 0x%x: %" CHIP_ERROR_FORMAT, - static_cast(fabricIndex), err.Format()); - } - } + ClearCASEResumptionStateOnFabricChange(fabricIndex); Credentials::GroupDataProvider * groupDataProvider = mServer->GetGroupDataProvider(); if (groupDataProvider != nullptr) @@ -440,10 +426,23 @@ class Server void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override { (void) fabricTable; - (void) fabricIndex; + ClearCASEResumptionStateOnFabricChange(fabricIndex); } private: + void ClearCASEResumptionStateOnFabricChange(chip::FabricIndex fabricIndex) + { + auto * sessionResumptionStorage = mServer->GetSessionResumptionStorage(); + VerifyOrReturn(sessionResumptionStorage != nullptr); + CHIP_ERROR err = sessionResumptionStorage->DeleteAll(fabricIndex); + if (err != CHIP_NO_ERROR) + { + ChipLogError(AppServer, + "Warning, failed to delete session resumption state for fabric index 0x%x: %" CHIP_ERROR_FORMAT, + static_cast(fabricIndex), err.Format()); + } + } + Server * mServer = nullptr; }; diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index 4629cee504fc50..16a9990053ebbb 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/src/controller/CHIPDeviceControllerFactory.cpp @@ -174,7 +174,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params) stateParams.sessionResumptionStorage = std::move(sessionResumptionStorage); auto delegate = chip::Platform::MakeUnique(); - ReturnErrorOnFailure(delegate->Init(stateParams.sessionMgr, stateParams.groupDataProvider)); + ReturnErrorOnFailure(delegate->Init(stateParams.sessionResumptionStorage.get(), stateParams.groupDataProvider)); stateParams.fabricTableDelegate = delegate.get(); ReturnErrorOnFailure(stateParams.fabricTable->AddFabricDelegate(stateParams.fabricTableDelegate)); delegate.release(); diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h index b760d574272787..a2eaf5744cf337 100644 --- a/src/controller/CHIPDeviceControllerFactory.h +++ b/src/controller/CHIPDeviceControllerFactory.h @@ -170,28 +170,24 @@ class DeviceControllerFactory class ControllerFabricDelegate final : public chip::FabricTable::Delegate { public: - CHIP_ERROR Init(SessionManager * sessionManager, Credentials::GroupDataProvider * groupDataProvider) + CHIP_ERROR Init(SessionResumptionStorage * sessionResumptionStorage, Credentials::GroupDataProvider * groupDataProvider) { - VerifyOrReturnError(sessionManager != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(sessionResumptionStorage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(groupDataProvider != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - mSessionManager = sessionManager; - mGroupDataProvider = groupDataProvider; + mSessionResumptionStorage = sessionResumptionStorage; + mGroupDataProvider = groupDataProvider; return CHIP_NO_ERROR; }; void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override { (void) fabricTable; - - if (mSessionManager != nullptr) - { - mSessionManager->FabricRemoved(fabricIndex); - } if (mGroupDataProvider != nullptr) { mGroupDataProvider->RemoveFabric(fabricIndex); } + ClearCASEResumptionStateOnFabricChange(fabricIndex); }; void OnFabricRetrievedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override @@ -209,12 +205,24 @@ class DeviceControllerFactory void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override { (void) fabricTable; - (void) fabricIndex; + ClearCASEResumptionStateOnFabricChange(fabricIndex); } private: - SessionManager * mSessionManager = nullptr; - Credentials::GroupDataProvider * mGroupDataProvider = nullptr; + void ClearCASEResumptionStateOnFabricChange(chip::FabricIndex fabricIndex) + { + VerifyOrReturn(mSessionResumptionStorage != nullptr); + CHIP_ERROR err = mSessionResumptionStorage->DeleteAll(fabricIndex); + if (err != CHIP_NO_ERROR) + { + ChipLogError(AppServer, + "Warning, failed to delete session resumption state for fabric index 0x%x: %" CHIP_ERROR_FORMAT, + static_cast(fabricIndex), err.Format()); + } + } + + Credentials::GroupDataProvider * mGroupDataProvider = nullptr; + SessionResumptionStorage * mSessionResumptionStorage = nullptr; }; private: diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index e06025739745e7..cd686d03de5de3 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -72,7 +72,10 @@ uint32_t EncryptedPacketBufferHandle::GetMessageCounter() const SessionManager::SessionManager() : mState(State::kNotReady) {} -SessionManager::~SessionManager() {} +SessionManager::~SessionManager() +{ + this->Shutdown(); +} CHIP_ERROR SessionManager::Init(System::Layer * systemLayer, TransportMgrBase * transportMgr, Transport::MessageCounterManagerInterface * messageCounterManager, @@ -82,6 +85,7 @@ CHIP_ERROR SessionManager::Init(System::Layer * systemLayer, TransportMgrBase * VerifyOrReturnError(transportMgr != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(storageDelegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(fabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + ReturnErrorOnFailure(fabricTable->AddFabricDelegate(this)); mState = State::kInitialized; mSystemLayer = systemLayer; @@ -102,6 +106,10 @@ CHIP_ERROR SessionManager::Init(System::Layer * systemLayer, TransportMgrBase * void SessionManager::Shutdown() { + if (mFabricTable != nullptr) + { + mFabricTable->RemoveFabricDelegate(this); + } mMessageCounterManager = nullptr; mState = State::kNotReady; diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 219fd4eb20c069..2ab169a29abab3 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -120,7 +120,7 @@ class EncryptedPacketBufferHandle final : private System::PacketBufferHandle EncryptedPacketBufferHandle(PacketBufferHandle && aBuffer) : PacketBufferHandle(std::move(aBuffer)) {} }; -class DLL_EXPORT SessionManager : public TransportMgrDelegate +class DLL_EXPORT SessionManager : public TransportMgrDelegate, public FabricTable::Delegate { public: SessionManager(); @@ -246,6 +246,28 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate using SessionHandleCallback = bool (*)(void * context, SessionHandle & sessionHandle); CHIP_ERROR ForEachSessionHandle(void * context, SessionHandleCallback callback); + //// FabricTable::Delegate Implementation //// + void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override + { + (void) fabricTable; + this->FabricRemoved(fabricIndex); + } + void OnFabricRetrievedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override + { + (void) fabricTable; + (void) fabricIndex; + } + void OnFabricPersistedToStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override + { + (void) fabricTable; + (void) fabricIndex; + } + void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override + { + (void) fabricTable; + (void) fabricIndex; + } + private: /** * The State of a secure transport object.