Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restyle Follow-up minor changes to OpCertStore #19836

Closed
wants to merge 2 commits into from
Closed

Restyle Follow-up minor changes to OpCertStore #19836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6015ce5
Follow-up minor changes to OpCertStore
tcarmelveilleux Jun 21, 2022
afec9a0
Restyled by clang-format
restyled-commits Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/app/ClusterStateCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ CHIP_ERROR ClusterStateCache::Get(const ConcreteAttributePath & path, TLV::TLVRe
}

reader.Init(attributeState->Get<Platform::ScopedMemoryBufferWithSize<uint8_t>>().Get(),
attributeState->Get<Platform::ScopedMemoryBufferWithSize<uint8_t>>().BufferByteSize());
attributeState->Get<Platform::ScopedMemoryBufferWithSize<uint8_t>>().AllocatedSize());
return reader.Next();
}

Expand Down Expand Up @@ -423,7 +423,7 @@ void ClusterStateCache::GetSortedFilters(std::vector<std::pair<DataVersionFilter
{
TLV::TLVReader bufReader;
bufReader.Init(attributeIter.second.Get<Platform::ScopedMemoryBufferWithSize<uint8_t>>().Get(),
attributeIter.second.Get<Platform::ScopedMemoryBufferWithSize<uint8_t>>().BufferByteSize());
attributeIter.second.Get<Platform::ScopedMemoryBufferWithSize<uint8_t>>().AllocatedSize());
ReturnOnFailure(bufReader.Next());
// Skip to the end of the element.
ReturnOnFailure(bufReader.Skip());
Expand Down
50 changes: 25 additions & 25 deletions src/credentials/OperationalCertificateStore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
*
Expand All @@ -187,24 +188,23 @@ 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;

/**
* @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;
Expand Down Expand Up @@ -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.
Expand Down
42 changes: 16 additions & 26 deletions src/credentials/PersistentStorageOpCertStore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t> rcacBuf;
Platform::ScopedMemoryBufferWithSize<uint8_t> rcacBuf;
ReturnErrorCodeIf(!rcacBuf.Alloc(rcac.size()), CHIP_ERROR_NO_MEMORY);
memcpy(rcacBuf.Get(), rcac.data(), rcac.size());

mPendingRcac = std::move(rcacBuf);
mPendingRcacSize = static_cast<uint16_t>(rcac.size());
mPendingRcac = std::move(rcacBuf);

mPendingFabricIndex = fabricIndex;
mStateFlags.Set(StateFlags::kAddNewTrustedRootCalled);
Expand Down Expand Up @@ -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<uint8_t> nocBuf;
Platform::ScopedMemoryBufferWithSize<uint8_t> nocBuf;
ReturnErrorCodeIf(!nocBuf.Alloc(noc.size()), CHIP_ERROR_NO_MEMORY);
memcpy(nocBuf.Get(), noc.data(), noc.size());

Platform::ScopedMemoryBuffer<uint8_t> icacBuf;
Platform::ScopedMemoryBufferWithSize<uint8_t> 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<uint16_t>(noc.size());

mPendingIcac = std::move(icacBuf);
mPendingIcacSize = static_cast<uint16_t>(icac.size());

mPendingFabricIndex = fabricIndex;
mPendingNoc = std::move(nocBuf);
mPendingIcac = std::move(icacBuf);

mStateFlags.Set(StateFlags::kAddNewOpCertsCalled);

Expand Down Expand Up @@ -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<uint8_t> nocBuf;
Platform::ScopedMemoryBufferWithSize<uint8_t> nocBuf;
ReturnErrorCodeIf(!nocBuf.Alloc(noc.size()), CHIP_ERROR_NO_MEMORY);
memcpy(nocBuf.Get(), noc.data(), noc.size());

Platform::ScopedMemoryBuffer<uint8_t> icacBuf;
Platform::ScopedMemoryBufferWithSize<uint8_t> 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<uint16_t>(noc.size());

mPendingIcac = std::move(icacBuf);
mPendingIcacSize = static_cast<uint16_t>(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.
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -453,21 +443,21 @@ CHIP_ERROR PersistentStorageOpCertStore::GetPendingCertificate(FabricIndex fabri
case CertChainElement::kRcac:
if (mPendingRcac.Get() != nullptr)
{
ByteSpan rcacSpan{ mPendingRcac.Get(), static_cast<size_t>(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<size_t>(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<size_t>(mPendingNocSize) };
ByteSpan nocSpan{ mPendingNoc.Get(), mPendingNoc.AllocatedSize() };
return CopySpanToMutableSpan(nocSpan, outCertificate);
}
break;
Expand Down
14 changes: 3 additions & 11 deletions src/credentials/PersistentStorageOpCertStore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,6 @@ class PersistentStorageOpCertStore : public OperationalCertificateStore
mPendingIcac.Free();
mPendingNoc.Free();

mPendingRcacSize = 0;
mPendingIcacSize = 0;
mPendingNocSize = 0;

mPendingFabricIndex = kUndefinedFabricIndex;
mStateFlags.ClearAll();
}
Expand All @@ -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<uint8_t> mPendingRcac;
Platform::ScopedMemoryBuffer<uint8_t> mPendingIcac;
Platform::ScopedMemoryBuffer<uint8_t> mPendingNoc;

uint16_t mPendingRcacSize = 0;
uint16_t mPendingIcacSize = 0;
uint16_t mPendingNocSize = 0;
Platform::ScopedMemoryBufferWithSize<uint8_t> mPendingRcac;
Platform::ScopedMemoryBufferWithSize<uint8_t> mPendingIcac;
Platform::ScopedMemoryBufferWithSize<uint8_t> mPendingNoc;

BitFlags<StateFlags> mStateFlags;
};
Expand Down
8 changes: 7 additions & 1 deletion src/lib/support/ScopedBuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,13 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer<T>
~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<T>::Free();
}

T * Release()
{
Expand Down