diff --git a/src/app/ClusterStateCache.cpp b/src/app/ClusterStateCache.cpp index d6cd9645667b6d..8dc38301bad74d 100644 --- a/src/app/ClusterStateCache.cpp +++ b/src/app/ClusterStateCache.cpp @@ -220,7 +220,7 @@ CHIP_ERROR ClusterStateCache::Get(const ConcreteAttributePath & path, TLV::TLVRe } reader.Init(attributeState->Get>().Get(), - attributeState->Get>().BufferByteSize()); + attributeState->Get>().AllocatedSize()); return reader.Next(); } @@ -423,7 +423,7 @@ void ClusterStateCache::GetSortedFilters(std::vector>().Get(), - attributeIter.second.Get>().BufferByteSize()); + attributeIter.second.Get>().AllocatedSize()); ReturnOnFailure(bufReader.Next()); // Skip to the end of the element. ReturnOnFailure(bufReader.Skip()); diff --git a/src/credentials/OperationalCertificateStore.h b/src/credentials/OperationalCertificateStore.h index 4540da9d99621e..030e16e91f2753 100644 --- a/src/credentials/OperationalCertificateStore.h +++ b/src/credentials/OperationalCertificateStore.h @@ -64,19 +64,19 @@ class OperationalCertificateStore virtual bool HasCertificateForFabric(FabricIndex fabricIndex, CertChainElement element) const = 0; /** - * @brief Add and temporarily activate a new Trusted Root Certificate to storage for the given fabric + * @brief Add and temporarily activate a new Trusted Root Certificate for the given fabric * * The certificate is temporary until committed or reverted. - * The certificate is committed to storage on `CommitOpCertsForFabric`. + * The certificate is committed to storage only on `CommitOpCertsForFabric`. * The certificate is destroyed if `RevertPendingOpCerts` is called before `CommitOpCertsForFabric`. * * Only one pending trusted root certificate is supported at a time and it is illegal * to call this method if there is already a persisted root certificate for the given * fabric. * - * Uniqueness constraints for roots (see AddTrustedRootCertificate command in spec) is not + * Uniqueness constraints for roots (see AddTrustedRootCertificate command in spec) are not * enforced by this method and must be done as a more holistic check elsewhere. Cryptographic - * signature verification or path validation is not enforced by this method. + * signature verification or path validation are not enforced by this method. * * If `UpdateOpCertsForFabric` had been called before this method, this method will return * CHIP_ERROR_INCORRECT_STATE since it is illegal to update trusted roots when updating an @@ -106,11 +106,11 @@ class OperationalCertificateStore * to call this method if there is already a persisted certificate chain for the given * fabric. * - * Cryptographic signature verification or path validation is not enforced by this method. + * Cryptographic signature verification or path validation are not enforced by this method. * * If `UpdateOpCertsForFabric` had been called before this method, this method will return * CHIP_ERROR_INCORRECT_STATE since it is illegal to add a certificate chain after - * updating an existing NOC updating prior to a commit. + * updating an existing NOC and before committing or reverting the update. * * If `AddNewTrustedRootCertForFabric` had not been called before this method, this method will * return CHIP_ERROR_INCORRECT_STATE since it is illegal in this implementation to store an @@ -129,7 +129,7 @@ class OperationalCertificateStore * @retval CHIP_NO_ERROR on success * @retval CHIP_ERROR_NO_MEMORY if there is insufficient memory to maintain the temporary `noc` and `icac` cert copies * @retval CHIP_ERROR_INVALID_ARGUMENT if either the noc or icac are invalid sizes - * @retval CHIP_ERROR_INVALID_FABRIC_INDEX if `fabricIndex` mismatches the one from previous successful + * @retval CHIP_ERROR_INVALID_FABRIC_INDEX if `fabricIndex` mismatches the one from a previous successful * `AddNewTrustedRootCertForFabric`. * @retval CHIP_ERROR_INCORRECT_STATE if the certificate store is not properly initialized, if this method * is called after `UpdateOpCertsForFabric`, or if there was @@ -151,33 +151,34 @@ class OperationalCertificateStore * to call this method if there was not already a persisted certificate chain for the given * fabric. * - * Cryptographic signature verification or path validation is not enforced by this method. + * Cryptographic signature verification or path validation are not enforced by this method. * * If `AddNewOpCertsForFabric` had been called before this method, this method will return * CHIP_ERROR_INCORRECT_STATE since it is illegal to update a certificate chain after - * adding an existing NOC updating prior to a commit. + * adding an existing NOC and before committing or reverting the addition. * - * If there is no existing persisted trusted root certificate for the given fabricIndex, this - * method will method will eturn CHIP_ERROR_INCORRECT_STATE since it is illegal in this - * implementation to store an NOC chain without associated root. + * If there is no existing persisted trusted root certificate and NOC chain for the given + * fabricIndex, this method will method will return CHIP_ERROR_INCORRECT_STATE since it is + * illegal in this implementation to store an NOC chain without associated root, and it is illegal + * to update an opcert for a fabric not already configured. * - * @param fabricIndex - FabricIndex for which to add a new operational certificate chain - * @param noc - Buffer containing the NOC certificate to update - * @param icac - Buffer containing the ICAC certificate to update. If no ICAC is needed, `icac.empty()` must be true. + * @param fabricIndex - FabricIndex for which to update the operational certificate chain + * @param noc - Buffer containing the new NOC certificate to use + * @param icac - Buffer containing the ICAC certificate to use. If no ICAC is needed, `icac.empty()` must be true. * * @retval CHIP_NO_ERROR on success * @retval CHIP_ERROR_NO_MEMORY if there is insufficient memory to maintain the temporary `noc` and `icac` cert copies * @retval CHIP_ERROR_INVALID_ARGUMENT if either the noc or icac are invalid sizes * @retval CHIP_ERROR_INCORRECT_STATE if the certificate store is not properly initialized, if this method * is called after `AddNewOpCertsForFabric`, or if there was - * already a pending cert chain for the given `fabricIndex`, - * or if there is no associated persisted root for for the given `fabricIndex`. + * already a pending cert chain for the given `fabricIndex`, or if there are + * no associated persisted root and NOC chain for for the given `fabricIndex`. * @retval other CHIP_ERROR value on internal errors */ virtual CHIP_ERROR UpdateOpCertsForFabric(FabricIndex fabricIndex, const ByteSpan & noc, const ByteSpan & icac) = 0; /** - * @brief Permanently commit the certificate chain last setup via successful calls to + * @brief Permanently commit the certificate chain last configured via successful calls to * legal combinations of `AddNewTrustedRootCertForFabric`, `AddNewOpCertsForFabric` or * `UpdateOpCertsForFabric`, replacing previously committed data, if any. * @@ -187,8 +188,8 @@ class OperationalCertificateStore * * @retval CHIP_NO_ERROR on success * @retval CHIP_ERROR_INCORRECT_STATE if the certificate store is not properly initialized, - * or if not valid pending state is available. - * @retval CHIP_ERROR_INVALID_FABRIC_INDEX if there are no pending certificate chain for `fabricIndex` + * or if no valid pending state is available. + * @retval CHIP_ERROR_INVALID_FABRIC_INDEX if there is no pending certificate chain for `fabricIndex` * @retval other CHIP_ERROR value on internal storage errors */ virtual CHIP_ERROR CommitOpCertsForFabric(FabricIndex fabricIndex) = 0; @@ -196,15 +197,14 @@ class OperationalCertificateStore /** * @brief Permanently remove the certificate chain associated with a fabric. * - * This is to be used for fail-safe handling and RemoveFabric. Removes both the - * pending operational cert chain elements for the fabricIndex (if any) and the committed - * ones (if any). + * This is to be used for RemoveFabric. Removes both the pending operational cert chain + * elements for the fabricIndex (if any) and the committed ones (if any). * * @param fabricIndex - FabricIndex for which to remove the operational cert chain * * @retval CHIP_NO_ERROR on success * @retval CHIP_ERROR_INCORRECT_STATE if the certificate store is not properly initialized. - * @retval CHIP_ERROR_INVALID_FABRIC_INDEX if there was not operational certificate data at all for `fabricIndex` + * @retval CHIP_ERROR_INVALID_FABRIC_INDEX if there was no operational certificate data at all for `fabricIndex` * @retval other CHIP_ERROR value on internal storage errors */ virtual CHIP_ERROR RemoveOpCertsForFabric(FabricIndex fabricIndex) = 0; @@ -234,7 +234,7 @@ class OperationalCertificateStore * @param outCertificate - buffer to contain the certificate obtained from persistent or temporary storage * * @retval CHIP_NO_ERROR on success. - * @retval CHIP_ERROR_BUFFER_TOO_SMALL if `outCertificate` does not fit the certificate. + * @retval CHIP_ERROR_BUFFER_TOO_SMALL if `outCertificate` is too small to fit the certificate found. * @retval CHIP_ERROR_INCORRECT_STATE if the certificate store is not properly initialized. * @retval CHIP_ERROR_NOT_FOUND if the element cannot be found. * @retval CHIP_ERROR_INVALID_FABRIC_INDEX if the fabricIndex is invalid. diff --git a/src/credentials/PersistentStorageOpCertStore.cpp b/src/credentials/PersistentStorageOpCertStore.cpp index 5cebd3adc430a1..1a058620e561ac 100644 --- a/src/credentials/PersistentStorageOpCertStore.cpp +++ b/src/credentials/PersistentStorageOpCertStore.cpp @@ -220,12 +220,11 @@ CHIP_ERROR PersistentStorageOpCertStore::AddNewTrustedRootCertForFabric(FabricIn CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(StorageHasCertificate(mStorage, fabricIndex, CertChainElement::kRcac), CHIP_ERROR_INCORRECT_STATE); - Platform::ScopedMemoryBuffer rcacBuf; + Platform::ScopedMemoryBufferWithSize rcacBuf; ReturnErrorCodeIf(!rcacBuf.Alloc(rcac.size()), CHIP_ERROR_NO_MEMORY); memcpy(rcacBuf.Get(), rcac.data(), rcac.size()); mPendingRcac = std::move(rcacBuf); - mPendingRcacSize = static_cast(rcac.size()); mPendingFabricIndex = fabricIndex; mStateFlags.Set(StateFlags::kAddNewTrustedRootCalled); @@ -255,24 +254,19 @@ CHIP_ERROR PersistentStorageOpCertStore::AddNewOpCertsForFabric(FabricIndex fabr ReturnErrorCodeIf(StorageHasCertificate(mStorage, fabricIndex, CertChainElement::kNoc), CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(StorageHasCertificate(mStorage, fabricIndex, CertChainElement::kIcac), CHIP_ERROR_INCORRECT_STATE); - Platform::ScopedMemoryBuffer nocBuf; + Platform::ScopedMemoryBufferWithSize nocBuf; ReturnErrorCodeIf(!nocBuf.Alloc(noc.size()), CHIP_ERROR_NO_MEMORY); memcpy(nocBuf.Get(), noc.data(), noc.size()); - Platform::ScopedMemoryBuffer icacBuf; + Platform::ScopedMemoryBufferWithSize icacBuf; if (icac.size() > 0) { ReturnErrorCodeIf(!icacBuf.Alloc(icac.size()), CHIP_ERROR_NO_MEMORY); memcpy(icacBuf.Get(), icac.data(), icac.size()); } - mPendingNoc = std::move(nocBuf); - mPendingNocSize = static_cast(noc.size()); - - mPendingIcac = std::move(icacBuf); - mPendingIcacSize = static_cast(icac.size()); - - mPendingFabricIndex = fabricIndex; + mPendingNoc = std::move(nocBuf); + mPendingIcac = std::move(icacBuf); mStateFlags.Set(StateFlags::kAddNewOpCertsCalled); @@ -303,22 +297,19 @@ CHIP_ERROR PersistentStorageOpCertStore::UpdateOpCertsForFabric(FabricIndex fabr // Don't check for ICAC, we may not have had one before, but assume that if NOC is there, a // previous chain was at least partially there - Platform::ScopedMemoryBuffer nocBuf; + Platform::ScopedMemoryBufferWithSize nocBuf; ReturnErrorCodeIf(!nocBuf.Alloc(noc.size()), CHIP_ERROR_NO_MEMORY); memcpy(nocBuf.Get(), noc.data(), noc.size()); - Platform::ScopedMemoryBuffer icacBuf; + Platform::ScopedMemoryBufferWithSize icacBuf; if (icac.size() > 0) { ReturnErrorCodeIf(!icacBuf.Alloc(icac.size()), CHIP_ERROR_NO_MEMORY); memcpy(icacBuf.Get(), icac.data(), icac.size()); } - mPendingNoc = std::move(nocBuf); - mPendingNocSize = static_cast(noc.size()); - - mPendingIcac = std::move(icacBuf); - mPendingIcacSize = static_cast(icac.size()); + mPendingNoc = std::move(nocBuf); + mPendingIcac = std::move(icacBuf); // For NOC update, UpdateOpCertsForFabric is what determines the pending fabric index, // not a previous AddNewTrustedRootCertForFabric call. @@ -346,17 +337,17 @@ CHIP_ERROR PersistentStorageOpCertStore::CommitOpCertsForFabric(FabricIndex fabr // TODO: Handle transaction marking to revert partial certs at next boot if we get interrupted by reboot. // Start committing NOC first so we don't have dangling roots if one was added. - ByteSpan pendingNocSpan{ mPendingNoc.Get(), mPendingNocSize }; + ByteSpan pendingNocSpan{ mPendingNoc.Get(), mPendingNoc.AllocatedSize() }; CHIP_ERROR nocErr = SaveCertToStorage(mStorage, mPendingFabricIndex, CertChainElement::kNoc, pendingNocSpan); // ICAC storage handles deleting on empty/missing - ByteSpan pendingIcacSpan{ mPendingIcac.Get(), mPendingIcacSize }; + ByteSpan pendingIcacSpan{ mPendingIcac.Get(), mPendingIcac.AllocatedSize() }; CHIP_ERROR icacErr = SaveCertToStorage(mStorage, mPendingFabricIndex, CertChainElement::kIcac, pendingIcacSpan); CHIP_ERROR rcacErr = CHIP_NO_ERROR; if (HasPendingRootCert()) { - ByteSpan pendingRcacSpan{ mPendingRcac.Get(), mPendingRcacSize }; + ByteSpan pendingRcacSpan{ mPendingRcac.Get(), mPendingRcac.AllocatedSize() }; rcacErr = SaveCertToStorage(mStorage, mPendingFabricIndex, CertChainElement::kRcac, pendingRcacSpan); } @@ -401,7 +392,6 @@ bool PersistentStorageOpCertStore::HasAnyCertificateForFabric(FabricIndex fabric bool nocMissing = !StorageHasCertificate(mStorage, fabricIndex, CertChainElement::kNoc); bool anyPending = (mPendingRcac.Get() != nullptr) || (mPendingIcac.Get() != nullptr) || (mPendingNoc.Get() != nullptr); - // If there was *no* state, pending or persisted, we have an error if (rcacMissing && icacMissing && nocMissing && !anyPending) { return false; @@ -453,21 +443,21 @@ CHIP_ERROR PersistentStorageOpCertStore::GetPendingCertificate(FabricIndex fabri case CertChainElement::kRcac: if (mPendingRcac.Get() != nullptr) { - ByteSpan rcacSpan{ mPendingRcac.Get(), static_cast(mPendingRcacSize) }; + ByteSpan rcacSpan{ mPendingRcac.Get(), mPendingRcac.AllocatedSize() }; return CopySpanToMutableSpan(rcacSpan, outCertificate); } break; case CertChainElement::kIcac: if (mPendingIcac.Get() != nullptr) { - ByteSpan icacSpan{ mPendingIcac.Get(), static_cast(mPendingIcacSize) }; + ByteSpan icacSpan{ mPendingIcac.Get(), mPendingIcac.AllocatedSize() }; return CopySpanToMutableSpan(icacSpan, outCertificate); } break; case CertChainElement::kNoc: if (mPendingNoc.Get() != nullptr) { - ByteSpan nocSpan{ mPendingNoc.Get(), static_cast(mPendingNocSize) }; + ByteSpan nocSpan{ mPendingNoc.Get(), mPendingNoc.AllocatedSize() }; return CopySpanToMutableSpan(nocSpan, outCertificate); } break; diff --git a/src/credentials/PersistentStorageOpCertStore.h b/src/credentials/PersistentStorageOpCertStore.h index 3c2f36e7c53802..6772317db8d6c6 100644 --- a/src/credentials/PersistentStorageOpCertStore.h +++ b/src/credentials/PersistentStorageOpCertStore.h @@ -86,10 +86,6 @@ class PersistentStorageOpCertStore : public OperationalCertificateStore mPendingIcac.Free(); mPendingNoc.Free(); - mPendingRcacSize = 0; - mPendingIcacSize = 0; - mPendingNocSize = 0; - mPendingFabricIndex = kUndefinedFabricIndex; mStateFlags.ClearAll(); } @@ -116,13 +112,9 @@ class PersistentStorageOpCertStore : public OperationalCertificateStore // This pending fabric index is `kUndefinedFabricIndex` if there are no pending certs at all for the fabric FabricIndex mPendingFabricIndex = kUndefinedFabricIndex; - Platform::ScopedMemoryBuffer mPendingRcac; - Platform::ScopedMemoryBuffer mPendingIcac; - Platform::ScopedMemoryBuffer mPendingNoc; - - uint16_t mPendingRcacSize = 0; - uint16_t mPendingIcacSize = 0; - uint16_t mPendingNocSize = 0; + Platform::ScopedMemoryBufferWithSize mPendingRcac; + Platform::ScopedMemoryBufferWithSize mPendingIcac; + Platform::ScopedMemoryBufferWithSize mPendingNoc; BitFlags mStateFlags; }; diff --git a/src/lib/support/ScopedBuffer.h b/src/lib/support/ScopedBuffer.h index 5e1d10ec0d7037..f01816de1cde50 100644 --- a/src/lib/support/ScopedBuffer.h +++ b/src/lib/support/ScopedBuffer.h @@ -187,7 +187,13 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer ~ScopedMemoryBufferWithSize() { mSize = 0; } // return the size in bytes - inline size_t BufferByteSize() const { return mSize; } + inline size_t AllocatedSize() const { return mSize; } + + void Free() + { + mSize = 0; + ScopedMemoryBuffer::Free(); + } T * Release() {