diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp index 63f7afebd7d0ba..5e2f117df7517b 100644 --- a/src/credentials/FabricTable.cpp +++ b/src/credentials/FabricTable.cpp @@ -910,6 +910,8 @@ FabricTable::AddOrUpdateInner(FabricIndex fabricIndex, bool isAddition, Crypto:: CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex) { + std::unique_lock 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 lock(mMutex); + ChipLogProgress(FabricProvisioning, "Forgetting fabric 0x%x", static_cast(fabricIndex)); auto * fabricInfo = GetMutableFabricByIndex(fabricIndex); @@ -1116,6 +1124,8 @@ void FabricTable::Forget(FabricIndex fabricIndex) void FabricTable::Shutdown() { + std::unique_lock lock(mMutex); + VerifyOrReturn(mStorage != nullptr); ChipLogProgress(FabricProvisioning, "Shutting down FabricTable"); @@ -1141,6 +1151,8 @@ void FabricTable::Shutdown() FabricIndex FabricTable::GetDeletedFabricFromCommitMarker() { + std::unique_lock 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 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 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 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 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, MutableByteSpan & outputCsr) { + std::unique_lock 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 fabr CHIP_ERROR FabricTable::AddNewPendingTrustedRootCert(const ByteSpan & rcac) { + std::unique_lock 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 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 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 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 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 lock(mMutex); + const FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex); VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX); diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h index 42f88024b877c6..2a19f5bd97d60f 100644 --- a/src/credentials/FabricTable.h +++ b/src/credentials/FabricTable.h @@ -22,6 +22,7 @@ #pragma once #include +#include #include #include @@ -1146,6 +1147,8 @@ class DLL_EXPORT FabricTable uint8_t mFabricCount = 0; BitFlags mStateFlags; + + mutable std::mutex mMutex; }; } // namespace chip diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 008b7c372d264b..36a372069405bb 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -244,7 +244,7 @@ class CASESession::WorkHelper struct CASESession::SendSigma3Data { FabricTable * fabricTable; - FabricIndex fabricIndex; + std::atomic fabricIndex; chip::Platform::ScopedMemoryBuffer 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(); }