From a5015bf1c689ae80d73059c1328f88e7e53196a8 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Tue, 7 Jun 2022 01:43:14 +0000
Subject: [PATCH 01/13] Add invalidate pending CASESession establishment when
 Fabric changes

---
 src/app/clusters/bindings/BindingManager.cpp  |  8 ++-
 .../operational-credentials-server.cpp        | 13 ++++-
 src/app/server/Server.h                       |  9 +++-
 src/controller/CHIPDeviceControllerFactory.h  |  9 +++-
 src/credentials/FabricTable.cpp               | 27 +++++++---
 src/credentials/FabricTable.h                 | 13 ++++-
 src/protocols/secure_channel/CASESession.cpp  | 50 +++++++++++++------
 src/protocols/secure_channel/CASESession.h    | 34 +++++++++++++
 .../secure_channel/PairingSession.cpp         | 11 ++++
 src/protocols/secure_channel/PairingSession.h |  2 +
 10 files changed, 149 insertions(+), 27 deletions(-)

diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp
index ce52f52867920d..c2ca68728080b7 100644
--- a/src/app/clusters/bindings/BindingManager.cpp
+++ b/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(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;
+        }
         chip::BindingTable & bindingTable = chip::BindingTable::GetInstance();
         auto iter                         = bindingTable.begin();
         while (iter != bindingTable.end())
diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
index 22218fde8385ca..cc0e5ee76ca2bc 100644
--- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
+++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
@@ -360,8 +360,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();
 
@@ -824,6 +831,10 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *
     err = failSafeContext.SetUpdateNocCommandInvoked();
     VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));
 
+    // TODO calling SendFabricChangeNotification seems a little out of place, would it be better to have this called
+    // maybe in SetNOCCert(), or maybe somewhere else.
+    Server::GetInstance().GetFabricTable().SendFabricChangeNotification(fabric->GetFabricIndex());
+
     // We might have a new operational identity, so we should start advertising
     // it right away.  Also, we need to withdraw our old operational identity.
     // So we need to StartServer() here.
diff --git a/src/app/server/Server.h b/src/app/server/Server.h
index 66e7b223a161e0..7aeca848c394be 100644
--- a/src/app/server/Server.h
+++ b/src/app/server/Server.h
@@ -324,8 +324,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);
diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h
index 8c00195453f5aa..ee1df711f7f208 100644
--- a/src/controller/CHIPDeviceControllerFactory.h
+++ b/src/controller/CHIPDeviceControllerFactory.h
@@ -172,10 +172,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);
diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
index 3a7cca66f4fefb..76d0178fb33b44 100644
--- a/src/credentials/FabricTable.cpp
+++ b/src/credentials/FabricTable.cpp
@@ -670,6 +670,24 @@ 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;
+    }
+}
+
+void FabricTable::SendFabricChangeNotification(FabricIndex fabricIndex)
+{
+    NotifyDelegatesOfFabricChange(fabricIndex, false /* fabricDeleted */);
+}
+
 CHIP_ERROR FabricTable::AddNewFabricInner(FabricInfo & newFabric, FabricIndex * outputIndex)
 {
     if (!mNextAvailableFabricIndex.HasValue())
@@ -764,16 +782,11 @@ CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
         else
         {
             mFabricCount--;
-            ChipLogProgress(Discovery, "Fabric (0x%x) deleted. Calling OnFabricDeletedFromStorage",
+            ChipLogProgress(Discovery, "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;
 }
diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
index ff350d687480f1..da38b038616b81 100644
--- a/src/credentials/FabricTable.h
+++ b/src/credentials/FabricTable.h
@@ -336,9 +336,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
@@ -379,6 +381,11 @@ class DLL_EXPORT FabricTable
      */
     CHIP_ERROR AddNewFabric(FabricInfo & fabric, FabricIndex * assignedIndex);
 
+    /**
+     * Send notification that fabricIndex was updated to all FabricTable Delegates.
+     */
+    void SendFabricChangeNotification(FabricIndex fabricIndex);
+
     // This is same as AddNewFabric, but skip duplicate fabric check, because we have multiple nodes belongs to the same fabric in
     // test-cases
     CHIP_ERROR AddNewFabricForTest(FabricInfo & newFabric, FabricIndex * outputIndex);
@@ -431,6 +438,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;
 
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index ec6f106b0d231b..8fdc0e7e285575 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -144,13 +144,27 @@ void CASESession::Clear()
     mState = State::kInitialized;
     Crypto::ClearSecretData(mIPK);
 
+    // TODO we need to use the fabric table provided after Tennessee's PR.
+    //mFabricTable->RemoveFabricDelegate(&mFabricDelegate);
+
     mLocalNodeId = kUndefinedNodeId;
     mPeerNodeId  = kUndefinedNodeId;
     mFabricInfo  = nullptr;
 }
 
+void CASESession::InvalidateIfPendingEstablishment()
+{
+    if (!IsSessionEstablishmentInProgress()) {
+        return;
+    }
+    // TODO Double check if there is maybe a more suitable CHIP_ERROR, some other options are
+    // CHIP_ERROR_CERT_EXPIRED, CHIP_ERROR_TRANSACTION_CANCELED, or CHIP_ERROR_INCORRECT_STATE
+    AbortPendingEstablish(CHIP_ERROR_CANCELLED);
+}
+
 CHIP_ERROR CASESession::Init(SessionManager & sessionManager, SessionEstablishmentDelegate * delegate)
 {
+    CHIP_ERROR err = CHIP_NO_ERROR;
     VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(mGroupDataProvider != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
@@ -158,14 +172,22 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, SessionEstablishme
 
     ReturnErrorOnFailure(mCommissioningHash.Begin());
 
+    // TODO we need to use the fabric table provided after Tennessee's PR.
+    // SuccessOrExit(err = mFabricTable->AddFabricDelegate(&mFabricDelegate));
+
     mDelegate = delegate;
-    ReturnErrorOnFailure(AllocateSecureSession(sessionManager));
+    SuccessOrExit(err = AllocateSecureSession(sessionManager));
 
     mValidContext.Reset();
     mValidContext.mRequiredKeyUsages.Set(KeyUsageFlags::kDigitalSignature);
     mValidContext.mRequiredKeyPurposes.Set(KeyPurposeFlags::kServerAuth);
 
-    return CHIP_NO_ERROR;
+exit:
+    if (err != CHIP_NO_ERROR)
+    {
+        Clear();
+    }
+    return err;
 }
 
 CHIP_ERROR
@@ -232,18 +254,23 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
     return err;
 }
 
-void CASESession::OnResponseTimeout(ExchangeContext * ec)
+void CASESession::AbortPendingEstablish(CHIP_ERROR err)
 {
-    VerifyOrReturn(ec != nullptr, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout was called by null exchange"));
-    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));
     // 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);
+}
+
+void CASESession::OnResponseTimeout(ExchangeContext * ec)
+{
+    VerifyOrReturn(ec != nullptr, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout was called by null exchange"));
+    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);
 }
 
 CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session) const
@@ -1730,12 +1757,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;
 }
diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
index 1914f3a2cc3507..91077be2a30ae3 100644
--- a/src/protocols/secure_channel/CASESession.h
+++ b/src/protocols/secure_channel/CASESession.h
@@ -164,7 +164,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,
@@ -218,6 +249,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();
@@ -252,6 +285,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 = CASESessionFabricDelegate(this);
     State mState;
 };
 
diff --git a/src/protocols/secure_channel/PairingSession.cpp b/src/protocols/secure_channel/PairingSession.cpp
index 55d992c1655d83..029b00160dd0e1 100644
--- a/src/protocols/secure_channel/PairingSession.cpp
+++ b/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
diff --git a/src/protocols/secure_channel/PairingSession.h b/src/protocols/secure_channel/PairingSession.h
index 25e0c9f95bd06b..e87ba34687172d 100644
--- a/src/protocols/secure_channel/PairingSession.h
+++ b/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();
 

From 211a2a444761dcc4a43c577fdc1ebde133ea3335 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Wed, 8 Jun 2022 14:37:18 +0000
Subject: [PATCH 02/13] Address comment and clean up after merging master

---
 .../operational-credentials-server.cpp        |  4 --
 src/credentials/FabricTable.cpp               |  8 +--
 src/credentials/FabricTable.h                 |  5 --
 src/protocols/secure_channel/CASESession.cpp  | 52 +++++++++----------
 src/protocols/secure_channel/CASESession.h    |  4 +-
 5 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
index 69b7cefd1a74af..1af8f3bef0e172 100644
--- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
+++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
@@ -884,10 +884,6 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *
     err = failSafeContext.SetUpdateNocCommandInvoked();
     VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));
 
-    // TODO calling SendFabricChangeNotification seems a little out of place, would it be better to have this called
-    // maybe in SetNOCCert(), or maybe somewhere else.
-    Server::GetInstance().GetFabricTable().SendFabricChangeNotification(fabric->GetFabricIndex());
-
     // We might have a new operational identity, so we should start advertising
     // it right away.  Also, we need to withdraw our old operational identity.
     // So we need to StartServer() here.
diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
index 4d65cb2580e329..102e60a338ad8e 100644
--- a/src/credentials/FabricTable.cpp
+++ b/src/credentials/FabricTable.cpp
@@ -709,12 +709,6 @@ void FabricTable::NotifyDelegatesOfFabricChange(FabricIndex fabricIndex, bool fa
     }
 }
 
-void FabricTable::SendFabricChangeNotification(FabricIndex fabricIndex)
-{
-    NotifyDelegatesOfFabricChange(fabricIndex, false /* fabricDeleted */);
-}
-
-CHIP_ERROR FabricTable::AddNewFabricInner(FabricInfo & newFabric, FabricIndex * outputIndex)
 /*
  * A validation policy we can pass into VerifyCredentials to extract the
  * latest NotBefore time in the certificate chain without having to load the
@@ -757,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;
 }
 
diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
index dad166bbab40e3..0d5847f3765e4f 100644
--- a/src/credentials/FabricTable.h
+++ b/src/credentials/FabricTable.h
@@ -392,11 +392,6 @@ class DLL_EXPORT FabricTable
      */
     CHIP_ERROR AddNewFabric(FabricInfo & fabric, FabricIndex * assignedIndex);
 
-    /**
-     * Send notification that fabricIndex was updated to all FabricTable Delegates.
-     */
-    void SendFabricChangeNotification(FabricIndex fabricIndex);
-
     // This is same as AddNewFabric, but skip duplicate fabric check, because we have multiple nodes belongs to the same fabric in
     // test-cases
     CHIP_ERROR AddNewFabricForTest(FabricInfo & newFabric, FabricIndex * outputIndex);
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index 9e34ad62eb2d8e..644c5d67406030 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -144,8 +144,7 @@ void CASESession::Clear()
     mState = State::kInitialized;
     Crypto::ClearSecretData(mIPK);
 
-    // TODO we need to use the fabric table provided after Tennessee's PR.
-    //mFabricTable->RemoveFabricDelegate(&mFabricDelegate);
+    mFabricsTable->RemoveFabricDelegate(&mFabricDelegate);
 
     mLocalNodeId  = kUndefinedNodeId;
     mPeerNodeId   = kUndefinedNodeId;
@@ -166,7 +165,6 @@ void CASESession::InvalidateIfPendingEstablishment()
 CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::CertificateValidityPolicy * policy,
                              SessionEstablishmentDelegate * delegate)
 {
-    CHIP_ERROR err = CHIP_NO_ERROR;
     VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(mGroupDataProvider != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
@@ -174,43 +172,43 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::Certi
 
     ReturnErrorOnFailure(mCommissioningHash.Begin());
 
-    // TODO we need to use the fabric table provided after Tennessee's PR.
-    // SuccessOrExit(err = mFabricTable->AddFabricDelegate(&mFabricDelegate));
-
     mDelegate = delegate;
-    SuccessOrExit(err = AllocateSecureSession(sessionManager));
+    ReturnErrorOnFailure(AllocateSecureSession(sessionManager));
 
     mValidContext.Reset();
     mValidContext.mRequiredKeyUsages.Set(KeyUsageFlags::kDigitalSignature);
     mValidContext.mRequiredKeyPurposes.Set(KeyPurposeFlags::kServerAuth);
     mValidContext.mValidityPolicy = policy;
 
-exit:
-    if (err != CHIP_NO_ERROR)
-    {
-        Clear();
-    }
-    return err;
+    return CHIP_NO_ERROR;
 }
 
 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,
@@ -241,6 +239,8 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
     // been initialized
     SuccessOrExit(err);
 
+    SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));
+
     mFabricsTable             = fabricTable;
     mFabricIndex              = fabricIndex;
     mSessionResumptionStorage = sessionResumptionStorage;
@@ -264,6 +264,15 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
     return err;
 }
 
+void CASESession::OnResponseTimeout(ExchangeContext * ec)
+{
+    VerifyOrReturn(ec != nullptr, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout was called by null exchange"));
+    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
@@ -274,15 +283,6 @@ void CASESession::AbortPendingEstablish(CHIP_ERROR err)
     mDelegate->OnSessionEstablishmentError(err);
 }
 
-void CASESession::OnResponseTimeout(ExchangeContext * ec)
-{
-    VerifyOrReturn(ec != nullptr, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout was called by null exchange"));
-    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);
-}
-
 CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session) const
 {
     switch (mState)
diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
index 380862d402e07d..6fd81f1e5ac085 100644
--- a/src/protocols/secure_channel/CASESession.h
+++ b/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());
 
@@ -290,7 +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 = CASESessionFabricDelegate(this);
+    CASESessionFabricDelegate mFabricDelegate{this};
     State mState;
 };
 

From a07d4c9dec43e714e4879962157ffa4b34202869 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Wed, 8 Jun 2022 17:36:10 +0000
Subject: [PATCH 03/13] Fix runtime and compile time errors

---
 src/app/clusters/bindings/BindingManager.cpp           | 2 +-
 src/app/clusters/door-lock-server/door-lock-server.cpp | 9 ++++++++-
 src/protocols/secure_channel/CASESession.cpp           | 5 ++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp
index c2ca68728080b7..81a7547a7659f2 100644
--- a/src/app/clusters/bindings/BindingManager.cpp
+++ b/src/app/clusters/bindings/BindingManager.cpp
@@ -25,7 +25,7 @@ namespace {
 
 class BindingFabricTableDelegate : public chip::FabricTable::Delegate
 {
-    void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) 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.
diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
index 2f69c839e6c1c8..db4dcfb5aa8973 100644
--- a/src/app/clusters/door-lock-server/door-lock-server.cpp
+++ b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -55,8 +55,15 @@ 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
     {
+        // TODO We likely want to do the same thing regardless of fabricDeleted. For
+        // now bailing out early when only update.
+        if (!fabricDeleted)
+        {
+            return;
+        }
+
         for (auto endpointId : EnabledEndpointsWithServerCluster(chip::app::Clusters::DoorLock::Id))
         {
             if (!DoorLockServer::Instance().OnFabricRemoved(endpointId, fabricIndex))
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index 644c5d67406030..40391fd0f64bf7 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -144,7 +144,10 @@ void CASESession::Clear()
     mState = State::kInitialized;
     Crypto::ClearSecretData(mIPK);
 
-    mFabricsTable->RemoveFabricDelegate(&mFabricDelegate);
+    if (mFabricsTable)
+    {
+        mFabricsTable->RemoveFabricDelegate(&mFabricDelegate);
+    }
 
     mLocalNodeId  = kUndefinedNodeId;
     mPeerNodeId   = kUndefinedNodeId;

From 0fbf786543eaa8f2583810541d068bf78633af7f Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Wed, 8 Jun 2022 19:15:24 +0000
Subject: [PATCH 04/13] Clean up a couple of TODOs

---
 src/app/clusters/door-lock-server/door-lock-server.cpp | 2 --
 src/protocols/secure_channel/CASESession.cpp           | 2 --
 2 files changed, 4 deletions(-)

diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
index db4dcfb5aa8973..f9318e44516dee 100644
--- a/src/app/clusters/door-lock-server/door-lock-server.cpp
+++ b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -57,8 +57,6 @@ class DoorLockClusterFabricDelegate : public chip::FabricTable::Delegate
 {
     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;
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index 40391fd0f64bf7..f34b0504939414 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -160,8 +160,6 @@ void CASESession::InvalidateIfPendingEstablishment()
     if (!IsSessionEstablishmentInProgress()) {
         return;
     }
-    // TODO Double check if there is maybe a more suitable CHIP_ERROR, some other options are
-    // CHIP_ERROR_CERT_EXPIRED, CHIP_ERROR_TRANSACTION_CANCELED, or CHIP_ERROR_INCORRECT_STATE
     AbortPendingEstablish(CHIP_ERROR_CANCELLED);
 }
 

From bed2c65a7a224a2e39579813f1f368fcfc5ee86e Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Thu, 9 Jun 2022 15:45:14 +0000
Subject: [PATCH 05/13] Breakout OnFabricNOCUpdated as a standalone callback

---
 src/app/clusters/bindings/BindingManager.cpp  | 11 +++---
 .../door-lock-server/door-lock-server.cpp     | 10 +++---
 .../operational-credentials-server.cpp        | 12 +++----
 src/app/server/Server.h                       | 15 ++++----
 src/controller/CHIPDeviceControllerFactory.h  | 15 ++++----
 src/credentials/FabricTable.cpp               | 35 ++++++++++---------
 src/credentials/FabricTable.h                 | 13 +++----
 src/protocols/secure_channel/CASESession.cpp  |  3 +-
 src/protocols/secure_channel/CASESession.h    | 11 ++++--
 9 files changed, 63 insertions(+), 62 deletions(-)

diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp
index 81a7547a7659f2..26304df7a5e88b 100644
--- a/src/app/clusters/bindings/BindingManager.cpp
+++ b/src/app/clusters/bindings/BindingManager.cpp
@@ -25,14 +25,8 @@ namespace {
 
 class BindingFabricTableDelegate : public chip::FabricTable::Delegate
 {
-    void OnFabricHasChanged(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex, bool fabricDeleted) override
+    void OnFabricDeletedFromStorage(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) 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())
@@ -54,6 +48,9 @@ class BindingFabricTableDelegate : public chip::FabricTable::Delegate
 
     // Intentionally left blank
     void OnFabricPersistedToStorage(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}
+
+    // Intentionally left blank
+    void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}
 };
 
 BindingFabricTableDelegate gFabricTableDelegate;
diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
index f9318e44516dee..d33138fe42dc57 100644
--- a/src/app/clusters/door-lock-server/door-lock-server.cpp
+++ b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -55,13 +55,8 @@ DoorLockServer DoorLockServer::instance;
 
 class DoorLockClusterFabricDelegate : public chip::FabricTable::Delegate
 {
-    void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
+    void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
     {
-        if (!fabricDeleted)
-        {
-            return;
-        }
-
         for (auto endpointId : EnabledEndpointsWithServerCluster(chip::app::Clusters::DoorLock::Id))
         {
             if (!DoorLockServer::Instance().OnFabricRemoved(endpointId, fabricIndex))
@@ -78,6 +73,9 @@ class DoorLockClusterFabricDelegate : public chip::FabricTable::Delegate
 
     // Intentionally left blank
     void OnFabricPersistedToStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override {}
+
+    // Intentionally left blank
+    void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}
 };
 static DoorLockClusterFabricDelegate gFabricDelegate;
 
diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
index 1af8f3bef0e172..663e19766e295a 100644
--- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
+++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
@@ -390,15 +390,8 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
 {
 
     // Gets called when a fabric is deleted from KVS store
-    void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
+    void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) 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();
 
@@ -465,6 +458,9 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
                         ChipLogValueX64(fabric->GetNodeId()), fabric->GetVendorId());
         fabricListChanged();
     }
+
+    // This is triggered by operation credential server so there is nothing additional that we need to do.
+    void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}
 };
 
 OpCredsFabricTableDelegate gFabricDelegate;
diff --git a/src/app/server/Server.h b/src/app/server/Server.h
index 371460a05455b4..099186f5acf54e 100644
--- a/src/app/server/Server.h
+++ b/src/app/server/Server.h
@@ -327,15 +327,8 @@ class Server
             return CHIP_NO_ERROR;
         };
 
-        void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
+        void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) 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);
@@ -378,6 +371,12 @@ class Server
             (void) fabricIndex;
         }
 
+        void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
+        {
+            (void) fabricTable;
+            (void) fabricIndex;
+        }
+
     private:
         Server * mServer = nullptr;
     };
diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h
index c7d49b890ebeb0..94dbc5fa69f612 100644
--- a/src/controller/CHIPDeviceControllerFactory.h
+++ b/src/controller/CHIPDeviceControllerFactory.h
@@ -173,17 +173,10 @@ class DeviceControllerFactory
             return CHIP_NO_ERROR;
         };
 
-        void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
+        void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) 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);
@@ -206,6 +199,12 @@ class DeviceControllerFactory
             (void) fabricIndex;
         }
 
+        void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
+        {
+            (void) fabricTable;
+            (void) fabricIndex;
+        }
+
     private:
         SessionManager * mSessionManager                    = nullptr;
         Credentials::GroupDataProvider * mGroupDataProvider = nullptr;
diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
index 102e60a338ad8e..af1e8c48f504bc 100644
--- a/src/credentials/FabricTable.cpp
+++ b/src/credentials/FabricTable.cpp
@@ -696,19 +696,6 @@ 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
@@ -752,7 +739,15 @@ CHIP_ERROR FabricTable::UpdateFabric(FabricIndex fabricIndex, FabricInfo & newFa
     // validity policy will see this condition and can act appropriately.
     mLastKnownGoodTime.UpdateLastKnownGoodChipEpochTime(notBeforeCollector.mLatestNotBefore);
 
-    NotifyDelegatesOfFabricChange(fabricIndex, false /* fabricDeleted */);
+    FabricTable::Delegate * delegate = mDelegateListRoot;
+    while (delegate)
+    {
+        // It is possible that delegate will remove itself from the list in OnFabricNOCUpdated,
+        // so we grab the next delegate in the list now.
+        FabricTable::Delegate * nextDelegate = delegate->next;
+        delegate->OnFabricNOCUpdated(*this, fabricIndex);
+        delegate = nextDelegate;
+    }
     return CHIP_NO_ERROR;
 }
 
@@ -856,11 +851,19 @@ CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
         else
         {
             mFabricCount--;
-            ChipLogProgress(FabricProvisioning, "Fabric (0x%x) deleted. Calling OnFabricHasChanged",
+            ChipLogProgress(FabricProvisioning, "Fabric (0x%x) deleted. Calling OnFabricDeletedFromStorage",
                             static_cast<unsigned>(fabricIndex));
         }
 
-        NotifyDelegatesOfFabricChange(fabricIndex, true /* fabricDeleted */);
+        FabricTable::Delegate * delegate = mDelegateListRoot;
+        while (delegate)
+        {
+            // It is possible that delegate will remove itself from the list in OnFabricDeletedFromStorage,
+            // so we grab the next delegate in the list now.
+            FabricTable::Delegate * nextDelegate = delegate->next;
+            delegate->OnFabricDeletedFromStorage(*this, fabricIndex);
+            delegate = nextDelegate;
+        }
     }
     return CHIP_NO_ERROR;
 }
diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
index 0d5847f3765e4f..86b1c7a41a0e31 100644
--- a/src/credentials/FabricTable.h
+++ b/src/credentials/FabricTable.h
@@ -347,11 +347,9 @@ class DLL_EXPORT FabricTable
         virtual ~Delegate() {}
 
         /**
-         * 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.
+         * Gets called when a fabric is deleted, such as on FabricTable::Delete().
          **/
-        virtual void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) = 0;
+        virtual void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) = 0;
 
         /**
          * Gets called when a fabric is loaded into Fabric Table from storage, such as
@@ -365,6 +363,11 @@ class DLL_EXPORT FabricTable
          **/
         virtual void OnFabricPersistedToStorage(FabricTable & fabricTable, FabricIndex fabricIndex) = 0;
 
+        /**
+         * Gets called when operational credentials are changed.
+         **/
+        virtual void OnFabricNOCUpdated(FabricTable & fabricTable, FabricIndex fabricIndex) = 0;
+
         // Intrusive list pointer for FabricTable to manage the entries.
         Delegate * next = nullptr;
     };
@@ -500,8 +503,6 @@ 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;
 
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index f34b0504939414..ba62c946f06944 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -157,7 +157,8 @@ void CASESession::Clear()
 
 void CASESession::InvalidateIfPendingEstablishment()
 {
-    if (!IsSessionEstablishmentInProgress()) {
+    if (!IsSessionEstablishmentInProgress())
+    {
         return;
     }
     AbortPendingEstablish(CHIP_ERROR_CANCELLED);
diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
index 6fd81f1e5ac085..40c0cdc01b2a7e 100644
--- a/src/protocols/secure_channel/CASESession.h
+++ b/src/protocols/secure_channel/CASESession.h
@@ -72,7 +72,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
      *   Initialize using configured fabrics and wait for session establishment requests.
      *
      * @param sessionManager                session manager from which to allocate a secure session object
-     * @param fabrics                       Table of fabrics that are currently configured on the device
+     * @param fabricTable                   Table of fabrics that are currently configured on the device
      * @param policy                        Optional application-provided certificate validity policy
      * @param delegate                      Callback object
      *
@@ -177,7 +177,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
         CASESessionFabricDelegate(CASESession * caseSession)
         :  mCASESession(caseSession) {}
 
-        void OnFabricHasChanged(FabricTable & fabricTable, FabricIndex fabricIndex, bool fabricDeleted) override
+        void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
         {
             (void) fabricTable;
             (void) fabricIndex;
@@ -196,6 +196,13 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
             (void) fabricIndex;
         }
 
+        void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
+        {
+            (void) fabricTable;
+            (void) fabricIndex;
+            mCASESession->InvalidateIfPendingEstablishment();
+        }
+
     private:
         CASESession * mCASESession;
     };

From c3cbc93724ac4d4a8cd430a0238038e015e082fa Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Thu, 9 Jun 2022 15:53:20 +0000
Subject: [PATCH 06/13] restyle

---
 src/protocols/secure_channel/CASESession.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
index 40c0cdc01b2a7e..414f1f4d598be6 100644
--- a/src/protocols/secure_channel/CASESession.h
+++ b/src/protocols/secure_channel/CASESession.h
@@ -174,8 +174,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
     class CASESessionFabricDelegate final : public chip::FabricTable::Delegate
     {
     public:
-        CASESessionFabricDelegate(CASESession * caseSession)
-        :  mCASESession(caseSession) {}
+        CASESessionFabricDelegate(CASESession * caseSession) :  mCASESession(caseSession) {}
 
         void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
         {
@@ -297,7 +296,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};
+    CASESessionFabricDelegate mFabricDelegate{ this };
     State mState;
 };
 

From 9ff13825949b1b3e8ca4feaae38e0b5777f9cc9b Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Thu, 9 Jun 2022 17:33:46 +0000
Subject: [PATCH 07/13] restyle and figuring out unit tests

---
 src/protocols/secure_channel/CASESession.h    |  2 +-
 .../secure_channel/tests/TestCASESession.cpp  | 26 +++++++++++--------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
index 414f1f4d598be6..0c1c334fcc3499 100644
--- a/src/protocols/secure_channel/CASESession.h
+++ b/src/protocols/secure_channel/CASESession.h
@@ -174,7 +174,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
     class CASESessionFabricDelegate final : public chip::FabricTable::Delegate
     {
     public:
-        CASESessionFabricDelegate(CASESession * caseSession) :  mCASESession(caseSession) {}
+        CASESessionFabricDelegate(CASESession * caseSession) : mCASESession(caseSession) {}
 
         void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
         {
diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp
index 816682767df078..bc5cf34a0bb48c 100644
--- a/src/protocols/secure_channel/tests/TestCASESession.cpp
+++ b/src/protocols/secure_channel/tests/TestCASESession.cpp
@@ -191,20 +191,24 @@ void CASE_SecurePairingWaitTest(nlTestSuite * inSuite, void * inContext)
 
     // Test all combinations of invalid parameters
     TestCASESecurePairingDelegate delegate;
-    CASESession pairing;
     FabricTable fabrics;
+    // In normal operation scope of FabricTable outlives CASESession. Without this scoping we hit
+    // ASAN test issue since FabricTable is not normally on the stack.
+    {
+        CASESession pairing;
 
-    NL_TEST_ASSERT(inSuite, pairing.GetSecureSessionType() == SecureSession::Type::kCASE);
+        NL_TEST_ASSERT(inSuite, pairing.GetSecureSessionType() == SecureSession::Type::kCASE);
 
-    pairing.SetGroupDataProvider(&gDeviceGroupDataProvider);
-    NL_TEST_ASSERT(inSuite,
-                   pairing.ListenForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, nullptr) ==
-                       CHIP_ERROR_INVALID_ARGUMENT);
-    NL_TEST_ASSERT(inSuite,
-                   pairing.ListenForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate) ==
-                       CHIP_ERROR_INVALID_ARGUMENT);
-    NL_TEST_ASSERT(inSuite,
-                   pairing.ListenForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate) == CHIP_NO_ERROR);
+        pairing.SetGroupDataProvider(&gDeviceGroupDataProvider);
+        NL_TEST_ASSERT(inSuite,
+                       pairing.ListenForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, nullptr) ==
+                           CHIP_ERROR_INVALID_ARGUMENT);
+        NL_TEST_ASSERT(inSuite,
+                       pairing.ListenForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate) ==
+                           CHIP_ERROR_INVALID_ARGUMENT);
+        NL_TEST_ASSERT(inSuite,
+                       pairing.ListenForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate) == CHIP_NO_ERROR);
+    }
 }
 
 void CASE_SecurePairingStartTest(nlTestSuite * inSuite, void * inContext)

From 03a88f03ffe8cdefbeb8ead54304c8759d1ca0d7 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Mon, 13 Jun 2022 19:54:07 +0000
Subject: [PATCH 08/13] Add unit test and fix issue discovered by unit test

---
 src/credentials/FabricTable.cpp               | 25 ++++---
 src/credentials/FabricTable.h                 |  6 ++
 src/protocols/secure_channel/CASESession.cpp  | 31 ++++++--
 src/protocols/secure_channel/CASESession.h    | 41 ++++++-----
 .../secure_channel/tests/TestCASESession.cpp  | 72 +++++++++++++++++++
 5 files changed, 145 insertions(+), 30 deletions(-)

diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
index 8d873d017a9d6c..4cfc99330d425a 100644
--- a/src/credentials/FabricTable.cpp
+++ b/src/credentials/FabricTable.cpp
@@ -710,6 +710,20 @@ class NotBeforeCollector : public Credentials::CertificateValidityPolicy
     System::Clock::Seconds32 mLatestNotBefore;
 };
 
+CHIP_ERROR FabricTable::SendFabricUpdateNOC(FabricIndex fabricIndex)
+{
+    FabricTable::Delegate * delegate = mDelegateListRoot;
+    while (delegate)
+    {
+        // It is possible that delegate will remove itself from the list in OnFabricNOCUpdated,
+        // so we grab the next delegate in the list now.
+        FabricTable::Delegate * nextDelegate = delegate->next;
+        delegate->OnFabricNOCUpdated(*this, fabricIndex);
+        delegate = nextDelegate;
+    }
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR FabricTable::UpdateFabric(FabricIndex fabricIndex, FabricInfo & newFabricInfo)
 {
     FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
@@ -723,15 +737,8 @@ CHIP_ERROR FabricTable::UpdateFabric(FabricIndex fabricIndex, FabricInfo & newFa
     // validity policy will see this condition and can act appropriately.
     mLastKnownGoodTime.UpdateLastKnownGoodChipEpochTime(notBeforeCollector.mLatestNotBefore);
 
-    FabricTable::Delegate * delegate = mDelegateListRoot;
-    while (delegate)
-    {
-        // It is possible that delegate will remove itself from the list in OnFabricNOCUpdated,
-        // so we grab the next delegate in the list now.
-        FabricTable::Delegate * nextDelegate = delegate->next;
-        delegate->OnFabricNOCUpdated(*this, fabricIndex);
-        delegate = nextDelegate;
-    }
+    SendFabricUpdateNOC(fabricIndex);
+
     return CHIP_NO_ERROR;
 }
 
diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
index 65c23945109462..777f589ae08657 100644
--- a/src/credentials/FabricTable.h
+++ b/src/credentials/FabricTable.h
@@ -429,6 +429,10 @@ class DLL_EXPORT FabricTable
      */
     CHIP_ERROR UpdateFabric(FabricIndex fabricIndex, FabricInfo & fabricInfo);
 
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+    void SendUpdateFabricNotificationForTest(FabricIndex fabricIndex) { SendFabricUpdateNOC(fabricIndex); }
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
+
     FabricInfo * FindFabric(Credentials::P256PublicKeySpan rootPubKey, FabricId fabricId);
     FabricInfo * FindFabricWithIndex(FabricIndex fabricIndex);
     const FabricInfo * FindFabricWithIndex(FabricIndex fabricIndex) const;
@@ -614,6 +618,8 @@ class DLL_EXPORT FabricTable
 
     CHIP_ERROR AddNewFabricInner(FabricInfo & fabric, FabricIndex * assignedIndex);
 
+    CHIP_ERROR SendFabricUpdateNOC(FabricIndex fabricIndex);
+
     FabricInfo mStates[CHIP_CONFIG_MAX_FABRICS];
     PersistentStorageDelegate * mStorage               = nullptr;
     Crypto::OperationalKeystore * mOperationalKeystore = nullptr;
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index 41c5abad199115..d40109f8d5d919 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -155,8 +155,12 @@ void CASESession::Clear()
     mFabricIndex  = kUndefinedFabricIndex;
 }
 
-void CASESession::InvalidateIfPendingEstablishment()
+void CASESession::InvalidateIfPendingEstablishmentOnFabric(FabricIndex fabricIndex)
 {
+    if (mFabricIndex != fabricIndex)
+    {
+        return;
+    }
     if (!IsSessionEstablishmentInProgress())
     {
         return;
@@ -276,14 +280,14 @@ 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));
+    // Discard the exchange so that Clear() doesn't try closing it.  The
+    // exchange will handle that.
+    DiscardExchange();
     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(err);
@@ -1692,6 +1696,22 @@ CHIP_ERROR CASESession::OnMessageReceived(ExchangeContext * ec, const PayloadHea
     Protocols::SecureChannel::MsgType msgType = static_cast<Protocols::SecureChannel::MsgType>(payloadHeader.GetMessageType());
     SuccessOrExit(err);
 
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+    if (mStopHandshakeAtState.HasValue() && mState == mStopHandshakeAtState.Value())
+    {
+        mStopHandshakeAtState = Optional<State>::Missing();
+        // For testing purposes we are trying to stop a successful CASESession from happening by dropping part of the
+        // handshake in the middle. We are trying to keep both sides of the CASESession establishment in an active
+        // pending state. In order to keep this side open we have to tell the exchange context that we will send an
+        // async message.
+        //
+        // Should you need to resume the CASESession you could theortically pass along msg to a callback that gets
+        // registered when setting mStopHandshakeAtState.
+        mExchangeCtxt->WillSendMessage();
+        return CHIP_NO_ERROR;
+    }
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
+
 #if CHIP_CONFIG_SLOW_CRYPTO
     if (msgType == Protocols::SecureChannel::MsgType::CASE_Sigma1 || msgType == Protocols::SecureChannel::MsgType::CASE_Sigma2 ||
         msgType == Protocols::SecureChannel::MsgType::CASE_Sigma2Resume ||
@@ -1788,6 +1808,9 @@ 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();
         AbortPendingEstablish(err);
     }
     return err;
diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
index 345cfae45a9d11..2214948e1bdaa6 100644
--- a/src/protocols/secure_channel/CASESession.h
+++ b/src/protocols/secure_channel/CASESession.h
@@ -61,6 +61,19 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
                                public PairingSession
 {
 public:
+    // Public so it is accessible to unit test
+    enum class State : uint8_t
+    {
+        kInitialized       = 0,
+        kSentSigma1        = 1,
+        kSentSigma2        = 2,
+        kSentSigma3        = 3,
+        kSentSigma1Resume  = 4,
+        kSentSigma2Resume  = 5,
+        kFinished          = 6,
+        kFinishedViaResume = 7,
+    };
+
     ~CASESession() override;
 
     Transport::SecureSession::Type GetSecureSessionType() const override { return Transport::SecureSession::Type::kCASE; }
@@ -173,7 +186,11 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
      **/
     void Clear();
 
-    void InvalidateIfPendingEstablishment();
+    void InvalidateIfPendingEstablishmentOnFabric(FabricIndex fabricIndex);
+
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+    void SetStopSigmaHandshakeAt(Optional<State> state) { mStopHandshakeAtState = state; }
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 
 private:
     class CASESessionFabricDelegate final : public chip::FabricTable::Delegate
@@ -184,8 +201,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
         void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
         {
             (void) fabricTable;
-            (void) fabricIndex;
-            mCASESession->InvalidateIfPendingEstablishment();
+            mCASESession->InvalidateIfPendingEstablishmentOnFabric(fabricIndex);
         }
 
         void OnFabricRetrievedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
@@ -203,26 +219,13 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
         void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
         {
             (void) fabricTable;
-            (void) fabricIndex;
-            mCASESession->InvalidateIfPendingEstablishment();
+            mCASESession->InvalidateIfPendingEstablishmentOnFabric(fabricIndex);
         }
 
     private:
         CASESession * mCASESession;
     };
 
-    enum class State : uint8_t
-    {
-        kInitialized       = 0,
-        kSentSigma1        = 1,
-        kSentSigma2        = 2,
-        kSentSigma3        = 3,
-        kSentSigma1Resume  = 4,
-        kSentSigma2Resume  = 5,
-        kFinished          = 6,
-        kFinishedViaResume = 7,
-    };
-
     /*
      * Initialize the object given a reference to the SessionManager, certificate validity policy and a delegate which will be
      * notified of any further progress on this session.
@@ -312,6 +315,10 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
 
     CASESessionFabricDelegate mFabricDelegate{ this };
     State mState;
+
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+    Optional<State> mStopHandshakeAtState = Optional<State>::Missing();
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 };
 
 } // namespace chip
diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp
index c66c8e2defcd27..a2bf24972c1eb9 100644
--- a/src/protocols/secure_channel/tests/TestCASESession.cpp
+++ b/src/protocols/secure_channel/tests/TestCASESession.cpp
@@ -254,6 +254,7 @@ void CASE_SecurePairingWaitTest(nlTestSuite * inSuite, void * inContext)
         NL_TEST_ASSERT(inSuite,
                        pairing.PrepareForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate, ScopedNodeId()) ==
                            CHIP_NO_ERROR);
+    }
 }
 
 void CASE_SecurePairingStartTest(nlTestSuite * inSuite, void * inContext)
@@ -827,6 +828,76 @@ static void CASE_SessionResumptionStorage(nlTestSuite * inSuite, void * inContex
     }
 }
 
+static void CASE_InvalidatePendingSessionEstablishment(nlTestSuite * inSuite, void * inContext)
+{
+    SessionManager sessionManager;
+    TestCASESecurePairingDelegate delegateCommissioner;
+    CASESession pairingCommissioner;
+    pairingCommissioner.SetGroupDataProvider(&gCommissionerGroupDataProvider);
+
+    TestContext & ctx = *reinterpret_cast<TestContext *>(inContext);
+
+    TestCASESecurePairingDelegate delegateAccessory;
+    CASESession pairingAccessory;
+    ReliableMessageProtocolConfig verySleepyAccessoryRmpConfig(System::Clock::Milliseconds32(360000),
+                                                               System::Clock::Milliseconds32(100000));
+    ReliableMessageProtocolConfig nonSleepyCommissionerRmpConfig(System::Clock::Milliseconds32(5000),
+                                                                 System::Clock::Milliseconds32(300));
+
+    auto & loopback            = ctx.GetLoopback();
+    loopback.mSentMessageCount = 0;
+
+    NL_TEST_ASSERT(inSuite,
+                   ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::MsgType::CASE_Sigma1,
+                                                                                     &pairingAccessory) == CHIP_NO_ERROR);
+
+    pairingCommissioner.SetStopSigmaHandshakeAt(MakeOptional(CASESession::State::kSentSigma1));
+
+    ExchangeContext * contextCommissioner = ctx.NewUnauthenticatedExchangeToBob(&pairingCommissioner);
+
+    pairingAccessory.SetGroupDataProvider(&gDeviceGroupDataProvider);
+    NL_TEST_ASSERT(inSuite,
+                   pairingAccessory.PrepareForSessionEstablishment(sessionManager, &gDeviceFabrics, nullptr, nullptr,
+                                                                   &delegateAccessory, ScopedNodeId(),
+                                                                   MakeOptional(verySleepyAccessoryRmpConfig)) == CHIP_NO_ERROR);
+
+    gDeviceFabrics.SendUpdateFabricNotificationForTest(gDeviceFabricIndex);
+    ctx.DrainAndServiceIO();
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0);
+
+    NL_TEST_ASSERT(inSuite,
+                   pairingCommissioner.EstablishSession(sessionManager, &gCommissionerFabrics,
+                                                        ScopedNodeId{ Node01_01, gCommissionerFabricIndex }, contextCommissioner,
+                                                        nullptr, nullptr, &delegateCommissioner,
+                                                        MakeOptional(nonSleepyCommissionerRmpConfig)) == CHIP_NO_ERROR);
+    ctx.DrainAndServiceIO();
+
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 0);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 0);
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 0);
+
+    gCommissionerFabrics.SendUpdateFabricNotificationForTest(gCommissionerFabricIndex);
+    ctx.DrainAndServiceIO();
+
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 1);
+
+    gDeviceFabrics.SendUpdateFabricNotificationForTest(gDeviceFabricIndex);
+    ctx.DrainAndServiceIO();
+
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 1);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 1);
+
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 0);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 0);
+    NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mIdleRetransTimeout == System::Clock::Milliseconds32(5000));
+    NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(300));
+    //    NL_TEST_ASSERT(inSuite, pairingCommissioner.GetRemoteMRPConfig().mIdleRetransTimeout ==
+    //    System::Clock::Milliseconds32(360000)); NL_TEST_ASSERT(inSuite,
+    //                   pairingCommissioner.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(100000));
+}
+
 // Test Suite
 
 /**
@@ -842,6 +913,7 @@ static const nlTest sTests[] =
     NL_TEST_DEF("Sigma1Parsing", CASE_Sigma1ParsingTest),
     NL_TEST_DEF("DestinationId", CASE_DestinationIdTest),
     NL_TEST_DEF("SessionResumptionStorage", CASE_SessionResumptionStorage),
+    NL_TEST_DEF("InvalidatePendingSessionEstablishment", CASE_InvalidatePendingSessionEstablishment),
 
     NL_TEST_SENTINEL()
 };

From 4ceff6b6e23cb2f81eeaf5014e474398fc9dcc51 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Mon, 13 Jun 2022 20:15:55 +0000
Subject: [PATCH 09/13] Fix formating issue

---
 src/protocols/secure_channel/CASESession.cpp           | 1 -
 src/protocols/secure_channel/tests/TestCASESession.cpp | 8 ++++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index d40109f8d5d919..301b24f230e6e0 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -189,7 +189,6 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::Certi
     return CHIP_NO_ERROR;
 }
 
-
 CHIP_ERROR
 CASESession::PrepareForSessionEstablishment(SessionManager & sessionManager, FabricTable * fabricTable,
                                             SessionResumptionStorage * sessionResumptionStorage,
diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp
index a2bf24972c1eb9..994cf0acd2191d 100644
--- a/src/protocols/secure_channel/tests/TestCASESession.cpp
+++ b/src/protocols/secure_channel/tests/TestCASESession.cpp
@@ -249,11 +249,11 @@ void CASE_SecurePairingWaitTest(nlTestSuite * inSuite, void * inContext)
                        pairing.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, nullptr, ScopedNodeId()) ==
                            CHIP_ERROR_INVALID_ARGUMENT);
         NL_TEST_ASSERT(inSuite,
-                       pairing.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate, ScopedNodeId()) ==
-                           CHIP_ERROR_INVALID_ARGUMENT);
+                       pairing.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate,
+                                                              ScopedNodeId()) == CHIP_ERROR_INVALID_ARGUMENT);
         NL_TEST_ASSERT(inSuite,
-                       pairing.PrepareForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate, ScopedNodeId()) ==
-                           CHIP_NO_ERROR);
+                       pairing.PrepareForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate,
+                                                              ScopedNodeId()) == CHIP_NO_ERROR);
     }
 }
 

From cdb8b104e2beed80843bf0f16d7b90bed60ea971 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Mon, 13 Jun 2022 21:03:02 +0000
Subject: [PATCH 10/13] clean up TestCASESession

---
 .../secure_channel/tests/TestCASESession.cpp      | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp
index 994cf0acd2191d..dfe8ddff8f175e 100644
--- a/src/protocols/secure_channel/tests/TestCASESession.cpp
+++ b/src/protocols/secure_channel/tests/TestCASESession.cpp
@@ -839,10 +839,6 @@ static void CASE_InvalidatePendingSessionEstablishment(nlTestSuite * inSuite, vo
 
     TestCASESecurePairingDelegate delegateAccessory;
     CASESession pairingAccessory;
-    ReliableMessageProtocolConfig verySleepyAccessoryRmpConfig(System::Clock::Milliseconds32(360000),
-                                                               System::Clock::Milliseconds32(100000));
-    ReliableMessageProtocolConfig nonSleepyCommissionerRmpConfig(System::Clock::Milliseconds32(5000),
-                                                                 System::Clock::Milliseconds32(300));
 
     auto & loopback            = ctx.GetLoopback();
     loopback.mSentMessageCount = 0;
@@ -858,8 +854,7 @@ static void CASE_InvalidatePendingSessionEstablishment(nlTestSuite * inSuite, vo
     pairingAccessory.SetGroupDataProvider(&gDeviceGroupDataProvider);
     NL_TEST_ASSERT(inSuite,
                    pairingAccessory.PrepareForSessionEstablishment(sessionManager, &gDeviceFabrics, nullptr, nullptr,
-                                                                   &delegateAccessory, ScopedNodeId(),
-                                                                   MakeOptional(verySleepyAccessoryRmpConfig)) == CHIP_NO_ERROR);
+                                                                   &delegateAccessory, ScopedNodeId()) == CHIP_NO_ERROR);
 
     gDeviceFabrics.SendUpdateFabricNotificationForTest(gDeviceFabricIndex);
     ctx.DrainAndServiceIO();
@@ -868,8 +863,7 @@ static void CASE_InvalidatePendingSessionEstablishment(nlTestSuite * inSuite, vo
     NL_TEST_ASSERT(inSuite,
                    pairingCommissioner.EstablishSession(sessionManager, &gCommissionerFabrics,
                                                         ScopedNodeId{ Node01_01, gCommissionerFabricIndex }, contextCommissioner,
-                                                        nullptr, nullptr, &delegateCommissioner,
-                                                        MakeOptional(nonSleepyCommissionerRmpConfig)) == CHIP_NO_ERROR);
+                                                        nullptr, nullptr, &delegateCommissioner) == CHIP_NO_ERROR);
     ctx.DrainAndServiceIO();
 
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 0);
@@ -891,11 +885,6 @@ static void CASE_InvalidatePendingSessionEstablishment(nlTestSuite * inSuite, vo
 
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 0);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 0);
-    NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mIdleRetransTimeout == System::Clock::Milliseconds32(5000));
-    NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(300));
-    //    NL_TEST_ASSERT(inSuite, pairingCommissioner.GetRemoteMRPConfig().mIdleRetransTimeout ==
-    //    System::Clock::Milliseconds32(360000)); NL_TEST_ASSERT(inSuite,
-    //                   pairingCommissioner.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(100000));
 }
 
 // Test Suite

From 91c5614e6a16960792f1e01cfc58805797c7c130 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Tue, 14 Jun 2022 13:28:13 +0000
Subject: [PATCH 11/13] Change simulating updating fabric NOC test to run in
 host tests only

---
 .../secure_channel/tests/TestCASESession.cpp  | 25 +++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp
index dfe8ddff8f175e..f55dfcb78771ca 100644
--- a/src/protocols/secure_channel/tests/TestCASESession.cpp
+++ b/src/protocols/secure_channel/tests/TestCASESession.cpp
@@ -348,11 +348,26 @@ void CASE_SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inConte
     NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == sTestCaseMessageCount);
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 1);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 1);
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 0);
     NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mIdleRetransTimeout == System::Clock::Milliseconds32(5000));
     NL_TEST_ASSERT(inSuite, pairingAccessory.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(300));
     NL_TEST_ASSERT(inSuite, pairingCommissioner.GetRemoteMRPConfig().mIdleRetransTimeout == System::Clock::Milliseconds32(360000));
     NL_TEST_ASSERT(inSuite,
                    pairingCommissioner.GetRemoteMRPConfig().mActiveRetransTimeout == System::Clock::Milliseconds32(100000));
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+    // Confirming that FabricTable sending a notification that fabric was updated doesn't affect
+    // already established connections.
+    //
+    // This is compiled for host tests which is enough test coverage
+    gCommissionerFabrics.SendUpdateFabricNotificationForTest(gCommissionerFabricIndex);
+    gDeviceFabrics.SendUpdateFabricNotificationForTest(gDeviceFabricIndex);
+    NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == sTestCaseMessageCount);
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 1);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 1);
+    NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0);
+    NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 0);
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 }
 
 void CASE_SecurePairingHandshakeTest(nlTestSuite * inSuite, void * inContext)
@@ -828,7 +843,8 @@ static void CASE_SessionResumptionStorage(nlTestSuite * inSuite, void * inContex
     }
 }
 
-static void CASE_InvalidatePendingSessionEstablishment(nlTestSuite * inSuite, void * inContext)
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+static void CASE_SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuite * inSuite, void * inContext)
 {
     SessionManager sessionManager;
     TestCASESecurePairingDelegate delegateCommissioner;
@@ -886,6 +902,7 @@ static void CASE_InvalidatePendingSessionEstablishment(nlTestSuite * inSuite, vo
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 0);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 0);
 }
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 
 // Test Suite
 
@@ -902,7 +919,11 @@ static const nlTest sTests[] =
     NL_TEST_DEF("Sigma1Parsing", CASE_Sigma1ParsingTest),
     NL_TEST_DEF("DestinationId", CASE_DestinationIdTest),
     NL_TEST_DEF("SessionResumptionStorage", CASE_SessionResumptionStorage),
-    NL_TEST_DEF("InvalidatePendingSessionEstablishment", CASE_InvalidatePendingSessionEstablishment),
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+    // This is compiled for host tests which is enough test coverage to ensure updating NOC invalidates
+    // CASESession that are in the process of establishing.
+    NL_TEST_DEF("InvalidatePendingSessionEstablishment", CASE_SimulateUpdateNOCInvalidatePendingEstablishment),
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 
     NL_TEST_SENTINEL()
 };

From ad4d1361145a34caee8bc1f8fe3aa881f7b0655e Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Wed, 15 Jun 2022 15:08:34 +0000
Subject: [PATCH 12/13] Address PR comments

---
 src/credentials/FabricTable.cpp                |  4 ++--
 src/credentials/FabricTable.h                  |  6 ++++--
 .../secure_channel/tests/TestCASESession.cpp   | 18 +++++++++---------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
index fd797169b59863..418e97566965e9 100644
--- a/src/credentials/FabricTable.cpp
+++ b/src/credentials/FabricTable.cpp
@@ -754,7 +754,7 @@ class NotBeforeCollector : public Credentials::CertificateValidityPolicy
     System::Clock::Seconds32 mLatestNotBefore;
 };
 
-CHIP_ERROR FabricTable::SendFabricUpdateNOC(FabricIndex fabricIndex)
+CHIP_ERROR FabricTable::NotifyNOCUpdatedOnFabric(FabricIndex fabricIndex)
 {
     FabricTable::Delegate * delegate = mDelegateListRoot;
     while (delegate)
@@ -781,7 +781,7 @@ CHIP_ERROR FabricTable::UpdateFabric(FabricIndex fabricIndex, FabricInfo & newFa
     // validity policy will see this condition and can act appropriately.
     mLastKnownGoodTime.UpdateLastKnownGoodChipEpochTime(notBeforeCollector.mLatestNotBefore);
 
-    SendFabricUpdateNOC(fabricIndex);
+    NotifyNOCUpdatedOnFabric(fabricIndex);
 
     return CHIP_NO_ERROR;
 }
diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
index 798c9f1b5ab279..a17882cd08a8a3 100644
--- a/src/credentials/FabricTable.h
+++ b/src/credentials/FabricTable.h
@@ -409,8 +409,10 @@ class DLL_EXPORT FabricTable
      */
     CHIP_ERROR UpdateFabric(FabricIndex fabricIndex, FabricInfo & fabricInfo);
 
+    // TODO this #if CONFIG_IM_BUILD_FOR_UNIT_TEST is temporary. There is a change incoming soon
+    // that will allow triggering NOC update directly.
 #if CONFIG_IM_BUILD_FOR_UNIT_TEST
-    void SendUpdateFabricNotificationForTest(FabricIndex fabricIndex) { SendFabricUpdateNOC(fabricIndex); }
+    void SendUpdateFabricNotificationForTest(FabricIndex fabricIndex) { NotifyNOCUpdatedOnFabric(fabricIndex); }
 #endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 
     FabricInfo * FindFabric(const Crypto::P256PublicKey & rootPubKey, FabricId fabricId);
@@ -603,7 +605,7 @@ class DLL_EXPORT FabricTable
 
     CHIP_ERROR AddNewFabricInner(FabricInfo & fabric, FabricIndex * assignedIndex);
 
-    CHIP_ERROR SendFabricUpdateNOC(FabricIndex fabricIndex);
+    CHIP_ERROR NotifyNOCUpdatedOnFabric(FabricIndex fabricIndex);
 
     FabricInfo mStates[CHIP_CONFIG_MAX_FABRICS];
     PersistentStorageDelegate * mStorage               = nullptr;
diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp
index f55dfcb78771ca..731e782e8f3249 100644
--- a/src/protocols/secure_channel/tests/TestCASESession.cpp
+++ b/src/protocols/secure_channel/tests/TestCASESession.cpp
@@ -240,20 +240,20 @@ void CASE_SecurePairingWaitTest(nlTestSuite * inSuite, void * inContext)
     // In normal operation scope of FabricTable outlives CASESession. Without this scoping we hit
     // ASAN test issue since FabricTable is not normally on the stack.
     {
-        CASESession pairing;
+        CASESession caseSession;
 
-        NL_TEST_ASSERT(inSuite, pairing.GetSecureSessionType() == SecureSession::Type::kCASE);
+        NL_TEST_ASSERT(inSuite, caseSession.GetSecureSessionType() == SecureSession::Type::kCASE);
 
-        pairing.SetGroupDataProvider(&gDeviceGroupDataProvider);
+        caseSession.SetGroupDataProvider(&gDeviceGroupDataProvider);
         NL_TEST_ASSERT(inSuite,
-                       pairing.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, nullptr, ScopedNodeId()) ==
-                           CHIP_ERROR_INVALID_ARGUMENT);
+                       caseSession.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, nullptr,
+                                                                  ScopedNodeId()) == CHIP_ERROR_INVALID_ARGUMENT);
         NL_TEST_ASSERT(inSuite,
-                       pairing.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate,
-                                                              ScopedNodeId()) == CHIP_ERROR_INVALID_ARGUMENT);
+                       caseSession.PrepareForSessionEstablishment(sessionManager, nullptr, nullptr, nullptr, &delegate,
+                                                                  ScopedNodeId()) == CHIP_ERROR_INVALID_ARGUMENT);
         NL_TEST_ASSERT(inSuite,
-                       pairing.PrepareForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate,
-                                                              ScopedNodeId()) == CHIP_NO_ERROR);
+                       caseSession.PrepareForSessionEstablishment(sessionManager, &fabrics, nullptr, nullptr, &delegate,
+                                                                  ScopedNodeId()) == CHIP_NO_ERROR);
     }
 }
 

From 5fe85ab9eaccc31c14cac598eeb3840dd5a83cb6 Mon Sep 17 00:00:00 2001
From: Terence Hampson <thampson@google.com>
Date: Wed, 15 Jun 2022 19:07:16 +0000
Subject: [PATCH 13/13] Address PR comments

---
 src/protocols/secure_channel/CASESession.cpp  | 14 +--
 src/protocols/secure_channel/CASESession.h    | 89 ++++++++-----------
 .../secure_channel/tests/TestCASESession.cpp  | 26 +++++-
 3 files changed, 69 insertions(+), 60 deletions(-)

diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index af21b52d4e194d..2a84ec920838e6 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -146,7 +146,7 @@ void CASESession::Clear()
 
     if (mFabricsTable)
     {
-        mFabricsTable->RemoveFabricDelegate(&mFabricDelegate);
+        mFabricsTable->RemoveFabricDelegate(this);
     }
 
     mLocalNodeId  = kUndefinedNodeId;
@@ -196,11 +196,13 @@ CASESession::PrepareForSessionEstablishment(SessionManager & sessionManager, Fab
                                             SessionEstablishmentDelegate * delegate, ScopedNodeId previouslyEstablishedPeer,
                                             Optional<ReliableMessageProtocolConfig> mrpConfig)
 {
+    // Below VerifyOrReturnError is not SuccessOrExit since we only want to goto `exit:` after
+    // Init has been successfully called.
     VerifyOrReturnError(fabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     ReturnErrorOnFailure(Init(sessionManager, policy, delegate, previouslyEstablishedPeer));
 
     CHIP_ERROR err = CHIP_NO_ERROR;
-    SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));
+    SuccessOrExit(err = fabricTable->AddFabricDelegate(this));
 
     mFabricsTable             = fabricTable;
     mRole                     = CryptoContext::SessionRole::kResponder;
@@ -248,7 +250,7 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
     // been initialized
     SuccessOrExit(err);
 
-    SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));
+    SuccessOrExit(err = fabricTable->AddFabricDelegate(this));
 
     mFabricsTable             = fabricTable;
     mFabricIndex              = fabricInfo->GetFabricIndex();
@@ -279,7 +281,7 @@ 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));
-    // Discard the exchange so that Clear() doesn't try closing it.  The
+    // Discard the exchange so that Clear() doesn't try aborting it.  The
     // exchange will handle that.
     DiscardExchange();
     AbortPendingEstablish(CHIP_ERROR_TIMEOUT);
@@ -1734,7 +1736,7 @@ CHIP_ERROR CASESession::OnMessageReceived(ExchangeContext * ec, const PayloadHea
         // pending state. In order to keep this side open we have to tell the exchange context that we will send an
         // async message.
         //
-        // Should you need to resume the CASESession you could theortically pass along msg to a callback that gets
+        // Should you need to resume the CASESession, you could theoretically pass along the msg to a callback that gets
         // registered when setting mStopHandshakeAtState.
         mExchangeCtxt->WillSendMessage();
         return CHIP_NO_ERROR;
@@ -1837,7 +1839,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
+        // Discard the exchange so that Clear() doesn't try aborting it.  The
         // exchange will handle that.
         DiscardExchange();
         AbortPendingEstablish(err);
diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
index 2214948e1bdaa6..01601ba8d2676c 100644
--- a/src/protocols/secure_channel/CASESession.h
+++ b/src/protocols/secure_channel/CASESession.h
@@ -58,22 +58,10 @@ namespace chip {
 // when implementing concurrent CASE session.
 class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
                                public Messaging::ExchangeDelegate,
+                               public FabricTable::Delegate,
                                public PairingSession
 {
 public:
-    // Public so it is accessible to unit test
-    enum class State : uint8_t
-    {
-        kInitialized       = 0,
-        kSentSigma1        = 1,
-        kSentSigma2        = 2,
-        kSentSigma3        = 3,
-        kSentSigma1Resume  = 4,
-        kSentSigma2Resume  = 5,
-        kFinished          = 6,
-        kFinishedViaResume = 7,
-    };
-
     ~CASESession() override;
 
     Transport::SecureSession::Type GetSecureSessionType() const override { return Transport::SecureSession::Type::kCASE; }
@@ -179,6 +167,28 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
     //// SessionDelegate ////
     void OnSessionReleased() override;
 
+    //// FabricTable::Delegate Implementation ////
+    void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
+    {
+        (void) fabricTable;
+        InvalidateIfPendingEstablishmentOnFabric(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;
+        InvalidateIfPendingEstablishmentOnFabric(fabricIndex);
+    }
+
     FabricIndex GetFabricIndex() const { return mFabricIndex; }
 
     // TODO: remove Clear, we should create a new instance instead reset the old instance.
@@ -186,44 +196,18 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
      **/
     void Clear();
 
-    void InvalidateIfPendingEstablishmentOnFabric(FabricIndex fabricIndex);
-
-#if CONFIG_IM_BUILD_FOR_UNIT_TEST
-    void SetStopSigmaHandshakeAt(Optional<State> state) { mStopHandshakeAtState = state; }
-#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
-
 private:
-    class CASESessionFabricDelegate final : public chip::FabricTable::Delegate
+    friend class CASESessionForTest;
+    enum class State : uint8_t
     {
-    public:
-        CASESessionFabricDelegate(CASESession * caseSession) : mCASESession(caseSession) {}
-
-        void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
-        {
-            (void) fabricTable;
-            mCASESession->InvalidateIfPendingEstablishmentOnFabric(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;
-            mCASESession->InvalidateIfPendingEstablishmentOnFabric(fabricIndex);
-        }
-
-    private:
-        CASESession * mCASESession;
+        kInitialized       = 0,
+        kSentSigma1        = 1,
+        kSentSigma2        = 2,
+        kSentSigma3        = 3,
+        kSentSigma1Resume  = 4,
+        kSentSigma2Resume  = 5,
+        kFinished          = 6,
+        kFinishedViaResume = 7,
     };
 
     /*
@@ -286,6 +270,12 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
     CHIP_ERROR ValidateReceivedMessage(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                        const System::PacketBufferHandle & msg);
 
+    void InvalidateIfPendingEstablishmentOnFabric(FabricIndex fabricIndex);
+
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+    void SetStopSigmaHandshakeAt(Optional<State> state) { mStopHandshakeAtState = state; }
+#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
+
     Crypto::Hash_SHA256_stream mCommissioningHash;
     Crypto::P256PublicKey mRemotePubKey;
 #ifdef ENABLE_HSM_CASE_EPHEMERAL_KEY
@@ -313,7 +303,6 @@ 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;
 
 #if CONFIG_IM_BUILD_FOR_UNIT_TEST
diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp
index 731e782e8f3249..339fc7123fc811 100644
--- a/src/protocols/secure_channel/tests/TestCASESession.cpp
+++ b/src/protocols/secure_channel/tests/TestCASESession.cpp
@@ -843,8 +843,18 @@ static void CASE_SessionResumptionStorage(nlTestSuite * inSuite, void * inContex
     }
 }
 
+// TODO, move all tests above into this class
 #if CONFIG_IM_BUILD_FOR_UNIT_TEST
-static void CASE_SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuite * inSuite, void * inContext)
+namespace chip {
+// TODO rename CASESessionForTest to TestCASESession. Not doing that immediately since that requires
+// removing a lot of the `using namesapce` above which is a larger cleanup.
+class CASESessionForTest
+{
+public:
+    static void CASE_SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuite * inSuite, void * inContext);
+};
+
+void CASESessionForTest::CASE_SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuite * inSuite, void * inContext)
 {
     SessionManager sessionManager;
     TestCASESecurePairingDelegate delegateCommissioner;
@@ -863,6 +873,8 @@ static void CASE_SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuite * i
                    ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::MsgType::CASE_Sigma1,
                                                                                      &pairingAccessory) == CHIP_NO_ERROR);
 
+    // In order for all the test iterations below, we need to stop the CASE sigma handshake in the middle such
+    // that the CASE session is in the process of being established.
     pairingCommissioner.SetStopSigmaHandshakeAt(MakeOptional(CASESession::State::kSentSigma1));
 
     ExchangeContext * contextCommissioner = ctx.NewUnauthenticatedExchangeToBob(&pairingCommissioner);
@@ -882,26 +894,32 @@ static void CASE_SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuite * i
                                                         nullptr, nullptr, &delegateCommissioner) == CHIP_NO_ERROR);
     ctx.DrainAndServiceIO();
 
+    // At this point the CASESession is in the process of establishing. Confirm that there are no errors and there are session
+    // has not been established.
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 0);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 0);
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 0);
 
+    // Simulating an update to the Fabric NOC for gCommissionerFabrics fabric table.
+    // Confirm that CASESession on commisioner side has reported an error.
     gCommissionerFabrics.SendUpdateFabricNotificationForTest(gCommissionerFabricIndex);
     ctx.DrainAndServiceIO();
-
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 1);
 
+    // Simulating an update to the Fabric NOC for gDeviceFabrics fabric table.
+    // Confirm that CASESession on accessory side has reported an error.
     gDeviceFabrics.SendUpdateFabricNotificationForTest(gDeviceFabricIndex);
     ctx.DrainAndServiceIO();
-
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 1);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 1);
 
+    // Sanity check that pairing did not complete.
     NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 0);
     NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 0);
 }
+} // namespace chip
 #endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 
 // Test Suite
@@ -922,7 +940,7 @@ static const nlTest sTests[] =
 #if CONFIG_IM_BUILD_FOR_UNIT_TEST
     // This is compiled for host tests which is enough test coverage to ensure updating NOC invalidates
     // CASESession that are in the process of establishing.
-    NL_TEST_DEF("InvalidatePendingSessionEstablishment", CASE_SimulateUpdateNOCInvalidatePendingEstablishment),
+    NL_TEST_DEF("InvalidatePendingSessionEstablishment", chip::CASESessionForTest::CASE_SimulateUpdateNOCInvalidatePendingEstablishment),
 #endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
 
     NL_TEST_SENTINEL()