Add mutex to FabricTable

This supports locking during SignWithOpKeypair, and other operations
that may alter state in the fabric table, while CASESession is
performing work in the background during session establishment.

CASESession registers as a fabric table listener, and when a fabric
is removed (e.g. timeout) it attempts to cancel any outstanding work,
and also clears out the fabric index in the work helper data.

Therefore, if outstanding work has made it into SignWithOpKeypair,
it should be OK until complete.

It still relies on other tasks not altering FabricInfo, or the
configured OperationalKeystore, but that would have had to have been
true before anyways.

The mutex was not made recursive. It's omitted from a few functions,
which should be OK for now, and there should be cleanup on a subsequent
commit (and probably fix up const-ness of member functions, and
factoring of API vs. impl functions). This commit is to flush out
build/test errors on all CI platforms, and to discuss/review/comment on
the general approach.

src/credentials/FabricTable.cpp

@@ -910,6 +910,8 @@ FabricTable::AddOrUpdateInner(FabricIndex fabricIndex, bool isAddition, Crypto::
 
 CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(IsValidFabricIndex(fabricIndex), CHIP_ERROR_INVALID_ARGUMENT);
 
@@ -1019,6 +1021,8 @@ CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
 
 void FabricTable::DeleteAllFabrics()
 {
+    // Rely on mutex in impl methods
+
     static_assert(kMaxValidFabricIndex <= UINT8_MAX, "Cannot create more fabrics than UINT8_MAX");
 
     RevertPendingFabricData();
@@ -1031,6 +1035,8 @@ void FabricTable::DeleteAllFabrics()
 
 CHIP_ERROR FabricTable::Init(const FabricTable::InitParams & initParams)
 {
+    // Mutex not necessary while initializing
+
     VerifyOrReturnError(initParams.storage != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(initParams.opCertStore != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
@@ -1105,6 +1111,8 @@ CHIP_ERROR FabricTable::Init(const FabricTable::InitParams & initParams)
 
 void FabricTable::Forget(FabricIndex fabricIndex)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     ChipLogProgress(FabricProvisioning, "Forgetting fabric 0x%x", static_cast<unsigned>(fabricIndex));
 
     auto * fabricInfo = GetMutableFabricByIndex(fabricIndex);
@@ -1116,6 +1124,8 @@ void FabricTable::Forget(FabricIndex fabricIndex)
 
 void FabricTable::Shutdown()
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     VerifyOrReturn(mStorage != nullptr);
     ChipLogProgress(FabricProvisioning, "Shutting down FabricTable");
 
@@ -1141,6 +1151,8 @@ void FabricTable::Shutdown()
 
 FabricIndex FabricTable::GetDeletedFabricFromCommitMarker()
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     FabricIndex retVal = mDeletedFabricIndexFromInit;
 
     // Reset for next read
@@ -1151,6 +1163,8 @@ FabricIndex FabricTable::GetDeletedFabricFromCommitMarker()
 
 CHIP_ERROR FabricTable::AddFabricDelegate(FabricTable::Delegate * delegate)
 {
+    // Mutex not necessary while adding/removing delegates
+
     VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     for (FabricTable::Delegate * iter = mDelegateListRoot; iter != nullptr; iter = iter->next)
     {
@@ -1166,6 +1180,8 @@ CHIP_ERROR FabricTable::AddFabricDelegate(FabricTable::Delegate * delegate)
 
 void FabricTable::RemoveFabricDelegate(FabricTable::Delegate * delegateToRemove)
 {
+    // Mutex not necessary while adding/removing delegates
+
     VerifyOrReturn(delegateToRemove != nullptr);
 
     if (delegateToRemove == mDelegateListRoot)
@@ -1197,6 +1213,8 @@ void FabricTable::RemoveFabricDelegate(FabricTable::Delegate * delegateToRemove)
 
 CHIP_ERROR FabricTable::SetLastKnownGoodChipEpochTime(System::Clock::Seconds32 lastKnownGoodChipEpochTime)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     CHIP_ERROR err = CHIP_NO_ERROR;
     // Find our latest NotBefore time for any installed certificate.
     System::Clock::Seconds32 latestNotBefore = System::Clock::Seconds32(0);
@@ -1390,6 +1408,8 @@ CHIP_ERROR FabricTable::ReadFabricInfo(TLV::ContiguousBufferTLVReader & reader)
 
 Crypto::P256Keypair * FabricTable::AllocateEphemeralKeypairForCASE()
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     if (mOperationalKeystore != nullptr)
     {
         return mOperationalKeystore->AllocateEphemeralKeypairForCASE();
@@ -1400,6 +1420,8 @@ Crypto::P256Keypair * FabricTable::AllocateEphemeralKeypairForCASE()
 
 void FabricTable::ReleaseEphemeralKeypair(Crypto::P256Keypair * keypair)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     if (mOperationalKeystore != nullptr)
     {
         mOperationalKeystore->ReleaseEphemeralKeypair(keypair);
@@ -1481,6 +1503,8 @@ bool FabricTable::HasOperationalKeyForFabric(FabricIndex fabricIndex) const
 
 CHIP_ERROR FabricTable::SignWithOpKeypair(FabricIndex fabricIndex, ByteSpan message, P256ECDSASignature & outSignature) const
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     const FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
     VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_KEY_NOT_FOUND);
 
@@ -1525,6 +1549,8 @@ bool FabricTable::SetPendingDataFabricIndex(FabricIndex fabricIndex)
 
 CHIP_ERROR FabricTable::AllocatePendingOperationalKey(Optional<FabricIndex> fabricIndex, MutableByteSpan & outputCsr)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     // We can only manage commissionable pending fail-safe state if we have a keystore
     VerifyOrReturnError(mOperationalKeystore != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
@@ -1567,6 +1593,8 @@ CHIP_ERROR FabricTable::AllocatePendingOperationalKey(Optional<FabricIndex> fabr
 
 CHIP_ERROR FabricTable::AddNewPendingTrustedRootCert(const ByteSpan & rcac)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
     // We should not already have pending NOC chain elements when we get here
@@ -1644,6 +1672,8 @@ CHIP_ERROR FabricTable::AddNewPendingFabricCommon(const ByteSpan & noc, const By
                                                   Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned,
                                                   FabricIndex * outNewFabricIndex)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(outNewFabricIndex != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     static_assert(kMaxValidFabricIndex <= UINT8_MAX, "Cannot create more fabrics than UINT8_MAX");
@@ -1714,6 +1744,8 @@ CHIP_ERROR FabricTable::AddNewPendingFabricCommon(const ByteSpan & noc, const By
 CHIP_ERROR FabricTable::UpdatePendingFabricCommon(FabricIndex fabricIndex, const ByteSpan & noc, const ByteSpan & icac,
                                                   Crypto::P256Keypair * existingOpKey, bool isExistingOpKeyExternallyOwned)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(IsValidFabricIndex(fabricIndex), CHIP_ERROR_INVALID_ARGUMENT);
 
@@ -1772,6 +1804,8 @@ CHIP_ERROR FabricTable::UpdatePendingFabricCommon(FabricIndex fabricIndex, const
 
 CHIP_ERROR FabricTable::CommitPendingFabricData()
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     VerifyOrReturnError((mStorage != nullptr) && (mOpCertStore != nullptr), CHIP_ERROR_INCORRECT_STATE);
 
     bool haveNewTrustedRoot      = mStateFlags.Has(StateFlags::kIsTrustedRootPending);
@@ -2050,6 +2084,8 @@ void FabricTable::RevertPendingOpCertsExceptRoot()
 
 CHIP_ERROR FabricTable::SetFabricLabel(FabricIndex fabricIndex, const CharSpan & fabricLabel)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(IsValidFabricIndex(fabricIndex), CHIP_ERROR_INVALID_FABRIC_INDEX);
 
@@ -2073,6 +2109,8 @@ CHIP_ERROR FabricTable::SetFabricLabel(FabricIndex fabricIndex, const CharSpan &
 
 CHIP_ERROR FabricTable::GetFabricLabel(FabricIndex fabricIndex, CharSpan & outFabricLabel)
 {
+    std::unique_lock<std::mutex> lock(mMutex);
+
     const FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
     VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX);
 

src/credentials/FabricTable.h

@@ -22,6 +22,7 @@
 #pragma once
 
 #include <algorithm>
+#include <mutex>
 
 #include <app/util/basic-types.h>
 #include <credentials/CHIPCert.h>
@@ -1146,6 +1147,8 @@ class DLL_EXPORT FabricTable
     uint8_t mFabricCount = 0;
 
     BitFlags<StateFlags> mStateFlags;
+
+    mutable std::mutex mMutex;
 };
 
 } // namespace chip

src/protocols/secure_channel/CASESession.cpp

@@ -244,7 +244,7 @@ class CASESession::WorkHelper
 struct CASESession::SendSigma3Data
 {
     FabricTable * fabricTable;
-    FabricIndex fabricIndex;
+    std::atomic<FabricIndex> fabricIndex;
 
     chip::Platform::ScopedMemoryBuffer<uint8_t> msg_R3_Signed;
     size_t msg_r3_signed_len;
@@ -308,6 +308,7 @@ void CASESession::Clear()
     // Cancel any outstanding work.
     if (mSendSigma3Helper)
     {
+        mSendSigma3Helper->mData.fabricIndex = kUndefinedFabricIndex;
         mSendSigma3Helper->CancelWork();
         mSendSigma3Helper.reset();
     }