Invalidate pending CASESession establishment when Fabric changes

src/app/clusters/bindings/BindingManager.cpp

@@ -25,8 +25,14 @@ namespace {
 
 class BindingFabricTableDelegate : public chip::FabricTable::Delegate
 {
-    void OnFabricDeletedFromStorage(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
+    void OnFabricHasChanged(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex, bool fabricDeleted) override
     {
+        // TODO We likely want to do the same thing regardless of fabricDeleted. For
+        // now bailing out early when only update.
+        if (!fabricDeleted)
+        {
+            return;
+        }
         chip::BindingTable & bindingTable = chip::BindingTable::GetInstance();
         auto iter                         = bindingTable.begin();
         while (iter != bindingTable.end())

src/app/clusters/door-lock-server/door-lock-server.cpp

@@ -55,8 +55,13 @@ DoorLockServer DoorLockServer::instance;
 
 class DoorLockClusterFabricDelegate : public chip::FabricTable::Delegate
 {
-    void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
+    void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
     {
+        if (!fabricDeleted)
+        {
+            return;
+        }
+
         for (auto endpointId : EnabledEndpointsWithServerCluster(chip::app::Clusters::DoorLock::Id))
         {
             if (!DoorLockServer::Instance().OnFabricRemoved(endpointId, fabricIndex))

src/app/clusters/operational-credentials-server/operational-credentials-server.cpp

@@ -390,8 +390,15 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
 {
 
     // Gets called when a fabric is deleted from KVS store
-    void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
+    void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
     {
+        // TODO We likely want to do the same thing regardless of fabricDeleted. For
+        // now bailing out early when only update.
+        if (!fabricDeleted)
+        {
+            return;
+        }
+
         ChipLogProgress(Zcl, "OpCreds: Fabric index 0x%x was deleted from fabric storage.", static_cast<unsigned>(fabricIndex));
         fabricListChanged();
 

src/app/server/Server.h

@@ -327,8 +327,15 @@ class Server
             return CHIP_NO_ERROR;
         };
 
-        void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
+        void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
         {
+            // TODO We likely want to do the same thing regardless of fabricDeleted. For
+            // now bailing out early when only update.
+            if (!fabricDeleted)
+            {
+                return;
+            }
+
             (void) fabricTable;
             auto & sessionManager = mServer->GetSecureSessionManager();
             sessionManager.FabricRemoved(fabricIndex);

src/controller/CHIPDeviceControllerFactory.h

@@ -173,10 +173,17 @@ class DeviceControllerFactory
             return CHIP_NO_ERROR;
         };
 
-        void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
+        void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
         {
             (void) fabricTable;
 
+            // TODO We likely want to do the same thing regardless of fabricDeleted. For
+            // now bailing out early when only update.
+            if (!fabricDeleted)
+            {
+                return;
+            }
+
             if (mSessionManager != nullptr)
             {
                 mSessionManager->FabricRemoved(fabricIndex);

src/credentials/FabricTable.cpp

@@ -696,6 +696,19 @@ CHIP_ERROR FabricTable::AddNewFabric(FabricInfo & newFabric, FabricIndex * outpu
     return AddNewFabricInner(newFabric, outputIndex);
 }
 
+void FabricTable::NotifyDelegatesOfFabricChange(FabricIndex fabricIndex, bool fabricDeleted)
+{
+    FabricTable::Delegate * delegate = mDelegateListRoot;
+    while (delegate)
+    {
+        // It is possible that delegate will remove itself from the list in OnFabricHasChanged,
+        // so we grab the next delegate in the list now.
+        FabricTable::Delegate * nextDelegate = delegate->next;
+        delegate->OnFabricHasChanged(*this, fabricIndex, fabricDeleted);
+        delegate = nextDelegate;
+    }
+}
+
 /*
  * A validation policy we can pass into VerifyCredentials to extract the
  * latest NotBefore time in the certificate chain without having to load the
@@ -738,6 +751,8 @@ CHIP_ERROR FabricTable::UpdateFabric(FabricIndex fabricIndex, FabricInfo & newFa
     // for CASE and current time is also unknown, the certificate
     // validity policy will see this condition and can act appropriately.
     mLastKnownGoodTime.UpdateLastKnownGoodChipEpochTime(notBeforeCollector.mLatestNotBefore);
+
+    NotifyDelegatesOfFabricChange(fabricIndex, false /* fabricDeleted */);
     return CHIP_NO_ERROR;
 }
 
@@ -841,16 +856,11 @@ CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
         else
         {
             mFabricCount--;
-            ChipLogProgress(FabricProvisioning, "Fabric (0x%x) deleted. Calling OnFabricDeletedFromStorage",
+            ChipLogProgress(FabricProvisioning, "Fabric (0x%x) deleted. Calling OnFabricHasChanged",
                             static_cast<unsigned>(fabricIndex));
         }
 
-        FabricTable::Delegate * delegate = mDelegateListRoot;
-        while (delegate)
-        {
-            delegate->OnFabricDeletedFromStorage(*this, fabricIndex);
-            delegate = delegate->next;
-        }
+        NotifyDelegatesOfFabricChange(fabricIndex, true /* fabricDeleted */);
     }
     return CHIP_NO_ERROR;
 }

src/credentials/FabricTable.h

@@ -347,9 +347,11 @@ class DLL_EXPORT FabricTable
         virtual ~Delegate() {}
 
         /**
-         * Gets called when a fabric is deleted, such as on FabricTable::Delete().
+         * Gets called when a fabric is changed in a significant way, such as cert being updated or fabric being delete.
+         * `fabricDeleted` indicates if the fabric has been deleted in case delegate treats updates differently from
+         * updates.
          **/
-        virtual void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) = 0;
+        virtual void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) = 0;
 
         /**
          * Gets called when a fabric is loaded into Fabric Table from storage, such as
@@ -498,6 +500,8 @@ class DLL_EXPORT FabricTable
 
     CHIP_ERROR AddNewFabricInner(FabricInfo & fabric, FabricIndex * assignedIndex);
 
+    void NotifyDelegatesOfFabricChange(FabricIndex fabricIndex, bool fabricDeleted);
+
     FabricInfo mStates[CHIP_CONFIG_MAX_FABRICS];
     PersistentStorageDelegate * mStorage = nullptr;
 

src/protocols/secure_channel/CASESession.cpp

@@ -144,12 +144,25 @@ void CASESession::Clear()
     mState = State::kInitialized;
     Crypto::ClearSecretData(mIPK);
 
+    if (mFabricsTable)
+    {
+        mFabricsTable->RemoveFabricDelegate(&mFabricDelegate);
+    }
+
     mLocalNodeId  = kUndefinedNodeId;
     mPeerNodeId   = kUndefinedNodeId;
     mFabricsTable = nullptr;
     mFabricIndex  = kUndefinedFabricIndex;
 }
 
+void CASESession::InvalidateIfPendingEstablishment()
+{
+    if (!IsSessionEstablishmentInProgress()) {
+        return;
+    }
+    AbortPendingEstablish(CHIP_ERROR_CANCELLED);
+}
+
 CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::CertificateValidityPolicy * policy,
                              SessionEstablishmentDelegate * delegate)
 {
@@ -172,23 +185,31 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::Certi
 }
 
 CHIP_ERROR
-CASESession::ListenForSessionEstablishment(SessionManager & sessionManager, FabricTable * fabrics,
+CASESession::ListenForSessionEstablishment(SessionManager & sessionManager, FabricTable * fabricTable,
                                            SessionResumptionStorage * sessionResumptionStorage,
                                            Credentials::CertificateValidityPolicy * policy, SessionEstablishmentDelegate * delegate,
                                            Optional<ReliableMessageProtocolConfig> mrpConfig)
 {
-    VerifyOrReturnError(fabrics != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrReturnError(fabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     ReturnErrorOnFailure(Init(sessionManager, policy, delegate));
 
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));
+
     mRole                     = CryptoContext::SessionRole::kResponder;
-    mFabricsTable             = fabrics;
+    mFabricsTable             = fabricTable;
     mSessionResumptionStorage = sessionResumptionStorage;
     mLocalMRPConfig           = mrpConfig;
 
     ChipLogDetail(SecureChannel, "Allocated SecureSession (%p) - waiting for Sigma1 msg",
                   mSecureSessionHolder.Get().Value()->AsSecureSession());
 
-    return CHIP_NO_ERROR;
+exit:
+    if (err != CHIP_NO_ERROR)
+    {
+        Clear();
+    }
+    return err;
 }
 
 CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, FabricTable * fabricTable, FabricIndex fabricIndex,
@@ -219,6 +240,8 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
     // been initialized
     SuccessOrExit(err);
 
+    SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));
+
     mFabricsTable             = fabricTable;
     mFabricIndex              = fabricIndex;
     mSessionResumptionStorage = sessionResumptionStorage;
@@ -248,12 +271,17 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec)
     VerifyOrReturn(mExchangeCtxt == ec, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout exchange doesn't match"));
     ChipLogError(SecureChannel, "CASESession timed out while waiting for a response from the peer. Current state was %u",
                  to_underlying(mState));
+    AbortPendingEstablish(CHIP_ERROR_TIMEOUT);
+}
+
+void CASESession::AbortPendingEstablish(CHIP_ERROR err)
+{
     // Discard the exchange so that Clear() doesn't try closing it.  The
     // exchange will handle that.
     DiscardExchange();
     Clear();
     // Do this last in case the delegate frees us.
-    mDelegate->OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT);
+    mDelegate->OnSessionEstablishmentError(err);
 }
 
 CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session) const
@@ -1758,12 +1786,7 @@ CHIP_ERROR CASESession::OnMessageReceived(ExchangeContext * ec, const PayloadHea
     // Call delegate to indicate session establishment failure.
     if (err != CHIP_NO_ERROR)
     {
-        // Discard the exchange so that Clear() doesn't try closing it.  The
-        // exchange will handle that.
-        DiscardExchange();
-        Clear();
-        // Do this last in case the delegate frees us.
-        mDelegate->OnSessionEstablishmentError(err);
+        AbortPendingEstablish(err);
     }
     return err;
 }

src/protocols/secure_channel/CASESession.h

@@ -79,7 +79,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
      * @return CHIP_ERROR     The result of initialization
      */
     CHIP_ERROR ListenForSessionEstablishment(
-        SessionManager & sessionManager, FabricTable * fabrics, SessionResumptionStorage * sessionResumptionStorage,
+        SessionManager & sessionManager, FabricTable * fabricTable, SessionResumptionStorage * sessionResumptionStorage,
         Credentials::CertificateValidityPolicy * policy, SessionEstablishmentDelegate * delegate,
         Optional<ReliableMessageProtocolConfig> mrpConfig = Optional<ReliableMessageProtocolConfig>::Missing());
 
@@ -168,7 +168,38 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
      **/
     void Clear();
 
+    void InvalidateIfPendingEstablishment();
+
 private:
+    class CASESessionFabricDelegate final : public chip::FabricTable::Delegate
+    {
+    public:
+        CASESessionFabricDelegate(CASESession * caseSession)
+        :  mCASESession(caseSession) {}
+
+        void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
+        {
+            (void) fabricTable;
+            (void) fabricIndex;
+            mCASESession->InvalidateIfPendingEstablishment();
+        }
+
+        void OnFabricRetrievedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
+        {
+            (void) fabricTable;
+            (void) fabricIndex;
+        }
+
+        void OnFabricPersistedToStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
+        {
+            (void) fabricTable;
+            (void) fabricIndex;
+        }
+
+    private:
+        CASESession * mCASESession;
+    };
+
     enum class State : uint8_t
     {
         kInitialized       = 0,
@@ -223,6 +254,8 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
     void OnSuccessStatusReport() override;
     CHIP_ERROR OnFailureStatusReport(Protocols::SecureChannel::GeneralStatusCode generalCode, uint16_t protocolCode) override;
 
+    void AbortPendingEstablish(CHIP_ERROR err);
+
     CHIP_ERROR GetHardcodedTime();
 
     CHIP_ERROR SetEffectiveTime();
@@ -257,6 +290,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
     // Sigma1 initiator random, maintained to be reused post-Sigma1, such as when generating Sigma2 S2RK key
     uint8_t mInitiatorRandom[kSigmaParamRandomNumberSize];
 
+    CASESessionFabricDelegate mFabricDelegate{this};
     State mState;
 };
 

src/protocols/secure_channel/PairingSession.cpp

@@ -138,6 +138,17 @@ CHIP_ERROR PairingSession::DecodeMRPParametersIfPresent(TLV::Tag expectedTag, TL
     return tlvReader.ExitContainer(containerType);
 }
 
+bool PairingSession::IsSessionEstablishmentInProgress()
+{
+    if (!mSecureSessionHolder)
+    {
+        return false;
+    }
+
+    Transport::SecureSession * secureSession = mSecureSessionHolder->AsSecureSession();
+    return secureSession->IsPairing();
+}
+
 void PairingSession::Clear()
 {
     // Clear acts like the destructor if PairingSession, if it is call during

src/protocols/secure_channel/PairingSession.h

@@ -169,6 +169,8 @@ class DLL_EXPORT PairingSession : public SessionDelegate
      */
     CHIP_ERROR DecodeMRPParametersIfPresent(TLV::Tag expectedTag, TLV::ContiguousBufferTLVReader & tlvReader);
 
+    bool IsSessionEstablishmentInProgress();
+
     // TODO: remove Clear, we should create a new instance instead reset the old instance.
     void Clear();