Skip to content

Commit

Permalink
Remove direct operational certs access from FabricInfo (#19531)
Browse files Browse the repository at this point in the history 
* Remove direct operational certs access from FabricInfo

To support moving to non-permanent storage, need to ensure
there is never direct access to certificates from FabricInfo classes
outside the FabricTable which owns all validations. This prevents
dangling FabricInfo instances and enables the changes needed to
make the fail-safe work to spec for AddNOC, UpdateNOC and
AddTrustedRootCertificate.

Issue #15585
Issue #7695

- Always go through the FabricTable, don't allow going directly via
  FabricInfo
- Updated CASESession to go through FabricTable also
- Getters for certs and root public key are now copying operations,
  rather than updating a ByteSpan to internally owned data (which
  may be stale!)
- First step towards moving to spec-compliant lifecycle for UpdateNOC
  with the same model as OperationalKeystore
- No functional changes, only structural changes

Testing done:
- Cert tests still pass
- Unit tests still pass

* Restyled by clang-format

* Rename GetXXX to FetchXXX

* Fix lint

* Fix Darwin CI

* Fix build on Darwin

* Apply review comments

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@tcarmelveilleux @restyled-commits @bzbarsky-apple
3 people authored Jun 13, 2022
1 parent dc3862a commit 8cbfd2f
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 127 deletions.
63 changes: 39 additions & 24 deletions src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ using namespace ::chip::Transport;
using namespace chip::app;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::OperationalCredentials;
using namespace chip::Credentials;
using namespace chip::Protocols::InteractionModel;

namespace {
Expand Down Expand Up @@ -125,22 +126,28 @@ CHIP_ERROR OperationalCredentialsAttrAccess::ReadNOCs(EndpointId endpoint, Attri
auto accessingFabricIndex = aEncoder.AccessingFabricIndex();

return aEncoder.EncodeList([accessingFabricIndex](const auto & encoder) -> CHIP_ERROR {
for (auto & fabricInfo : Server::GetInstance().GetFabricTable())
const auto & fabricTable = Server::GetInstance().GetFabricTable();
for (auto & fabricInfo : fabricTable)
{
Clusters::OperationalCredentials::Structs::NOCStruct::Type noc;
uint8_t nocBuf[kMaxCHIPCertLength];
uint8_t icacBuf[kMaxCHIPCertLength];
MutableByteSpan nocSpan{ nocBuf };
MutableByteSpan icacSpan{ icacBuf };
FabricIndex fabricIndex = fabricInfo.GetFabricIndex();

noc.fabricIndex = fabricInfo.GetFabricIndex();
noc.fabricIndex = fabricIndex;

if (accessingFabricIndex == fabricInfo.GetFabricIndex())
if (accessingFabricIndex == fabricIndex)
{
ByteSpan icac;

ReturnErrorOnFailure(fabricInfo.GetNOCCert(noc.noc));
ReturnErrorOnFailure(fabricInfo.GetICACert(icac));
ReturnErrorOnFailure(fabricTable.FetchNOCCert(fabricIndex, nocSpan));
ReturnErrorOnFailure(fabricTable.FetchICACert(fabricIndex, icacSpan));

if (!icac.empty())
noc.noc = nocSpan;
if (!icacSpan.empty())
{
noc.icac.SetNonNull(icac);
noc.icac.SetNonNull(icacSpan);
}
}

Expand All @@ -166,20 +173,23 @@ CHIP_ERROR OperationalCredentialsAttrAccess::ReadCommissionedFabrics(EndpointId
CHIP_ERROR OperationalCredentialsAttrAccess::ReadFabricsList(EndpointId endpoint, AttributeValueEncoder & aEncoder)
{
return aEncoder.EncodeList([](const auto & encoder) -> CHIP_ERROR {
for (auto & fabricInfo : Server::GetInstance().GetFabricTable())
const auto & fabricTable = Server::GetInstance().GetFabricTable();

for (auto & fabricInfo : fabricTable)
{
Clusters::OperationalCredentials::Structs::FabricDescriptor::Type fabricDescriptor;
FabricIndex fabricIndex = fabricInfo.GetFabricIndex();

fabricDescriptor.fabricIndex = fabricInfo.GetFabricIndex();
fabricDescriptor.fabricIndex = fabricIndex;
fabricDescriptor.nodeId = fabricInfo.GetPeerId().GetNodeId();
fabricDescriptor.vendorId = static_cast<chip::VendorId>(fabricInfo.GetVendorId());
fabricDescriptor.fabricId = fabricInfo.GetFabricId();

fabricDescriptor.label = fabricInfo.GetFabricLabel();

Credentials::P256PublicKeySpan pubKey;
ReturnErrorOnFailure(fabricInfo.GetRootPubkey(pubKey));
fabricDescriptor.rootPublicKey = pubKey;
Crypto::P256PublicKey pubKey;
ReturnErrorOnFailure(fabricTable.FetchRootPubkey(fabricIndex, pubKey));
fabricDescriptor.rootPublicKey = ByteSpan{ pubKey.ConstBytes(), pubKey.Length() };

ReturnErrorOnFailure(encoder.Encode(fabricDescriptor));
}
Expand All @@ -192,11 +202,14 @@ CHIP_ERROR OperationalCredentialsAttrAccess::ReadRootCertificates(EndpointId end
{
// It is OK to have duplicates.
return aEncoder.EncodeList([](const auto & encoder) -> CHIP_ERROR {
for (auto & fabricInfo : Server::GetInstance().GetFabricTable())
const auto & fabricTable = Server::GetInstance().GetFabricTable();

for (auto & fabricInfo : fabricTable)
{
ByteSpan cert;
ReturnErrorOnFailure(fabricInfo.GetRootCert(cert));
ReturnErrorOnFailure(encoder.Encode(cert));
uint8_t certBuf[kMaxCHIPCertLength];
MutableByteSpan cert{ certBuf };
ReturnErrorOnFailure(fabricTable.FetchRootCert(fabricInfo.GetFabricIndex(), cert));
ReturnErrorOnFailure(encoder.Encode(ByteSpan{ cert }));
}

return CHIP_NO_ERROR;
Expand Down Expand Up @@ -842,8 +855,7 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *

auto & fabricTable = Server::GetInstance().GetFabricTable();
FailSafeContext & failSafeContext = DeviceControlServer::DeviceControlSvr().GetFailSafeContext();
FabricInfo * fabric = RetrieveCurrentFabric(commandObj);
ByteSpan rcac;
FabricInfo * fabricInfo = RetrieveCurrentFabric(commandObj);

VerifyOrExit(NOCValue.size() <= Credentials::kMaxCHIPCertLength, nonDefaultStatus = Status::InvalidCommand);
VerifyOrExit(!ICACValue.HasValue() || ICACValue.Value().size() <= Credentials::kMaxCHIPCertLength,
Expand All @@ -858,8 +870,8 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *
VerifyOrExit(failSafeContext.IsCsrRequestForUpdateNoc(), nonDefaultStatus = Status::ConstraintError);

// If current fabric is not available, command was invoked over PASE which is not legal
VerifyOrExit(fabric != nullptr, nocResponse = ConvertToNOCResponseStatus(CHIP_ERROR_INSUFFICIENT_PRIVILEGE));
fabricIndex = fabric->GetFabricIndex();
VerifyOrExit(fabricInfo != nullptr, nocResponse = ConvertToNOCResponseStatus(CHIP_ERROR_INSUFFICIENT_PRIVILEGE));
fabricIndex = fabricInfo->GetFabricIndex();

// Initialize fields of gFabricBeingCommissioned:
// - the root certificate, fabric label, and vendor id are copied from the existing fabric record
Expand All @@ -868,16 +880,19 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *
// The remaining operation id and fabric id fields will be extracted from the new operational
// credentials and set by following SetFabricInfo() call.
{
err = fabric->GetRootCert(rcac);
uint8_t rcacBuf[kMaxCHIPCertLength];
MutableByteSpan rcac{ rcacBuf };

err = fabricTable.FetchRootCert(fabricIndex, rcac);
VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));

err = gFabricBeingCommissioned.SetRootCert(rcac);
VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));

err = gFabricBeingCommissioned.SetFabricLabel(fabric->GetFabricLabel());
err = gFabricBeingCommissioned.SetFabricLabel(fabricInfo->GetFabricLabel());
VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));

gFabricBeingCommissioned.SetVendorId(fabric->GetVendorId());
gFabricBeingCommissioned.SetVendorId(fabricInfo->GetVendorId());

err = gFabricBeingCommissioned.SetNOCCert(NOCValue);
VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err));
Expand Down
5 changes: 3 additions & 2 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams &
FabricInfo newFabric;
constexpr uint32_t chipCertAllocatedLen = kMaxCHIPCertLength;
chip::Platform::ScopedMemoryBuffer<uint8_t> chipCert;
Credentials::P256PublicKeySpan rootPublicKey;
Credentials::P256PublicKeySpan rootPublicKeySpan;
FabricId fabricId;

// There are three possibilities here in terms of what happens with our
Expand All @@ -175,6 +175,8 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams &

ReturnErrorOnFailure(ConvertX509CertToChipCert(params.controllerRCAC, chipCertSpan));
ReturnErrorOnFailure(newFabric.SetRootCert(chipCertSpan));
ReturnErrorOnFailure(Credentials::ExtractPublicKeyFromChipCert(chipCertSpan, rootPublicKeySpan));
Crypto::P256PublicKey rootPublicKey{ rootPublicKeySpan };

if (params.controllerICAC.empty())
{
Expand All @@ -194,7 +196,6 @@ CHIP_ERROR DeviceController::InitControllerNOCChain(const ControllerInitParams &
ReturnErrorOnFailure(newFabric.SetNOCCert(chipCertSpan));
ReturnErrorOnFailure(ExtractFabricIdFromCert(chipCertSpan, &fabricId));

ReturnErrorOnFailure(newFabric.GetRootPubkey(rootPublicKey));
mFabricInfo = params.systemState->Fabrics()->FindFabric(rootPublicKey, fabricId);
if (mFabricInfo != nullptr)
{
Expand Down
9 changes: 9 additions & 0 deletions src/controller/CHIPDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,15 @@ class DLL_EXPORT DeviceController : public AbstractDnssdDiscoveryController

FabricInfo * GetFabricInfo() { return mFabricInfo; }

const FabricTable * GetFabricTable() const
{
if (mSystemState == nullptr)
{
return nullptr;
}
return mSystemState->Fabrics();
}

void ReleaseOperationalDevice(NodeId remoteDeviceId);

OperationalCredentialsDelegate * GetOperationalCredentialsDelegate() { return mOperationalCredentialsDelegate; }
Expand Down
91 changes: 69 additions & 22 deletions src/credentials/FabricTable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,23 @@ CHIP_ERROR FabricInfo::VerifyCredentials(const ByteSpan & noc, const ByteSpan &
}

ReturnErrorOnFailure(GeneratePeerId(rcac, fabricId, nodeId, &nocPeerId));
nocPubkey = P256PublicKey(certificates.GetLastCert()[0].mPublicKey);
nocPubkey = certificates.GetLastCert()[0].mPublicKey;

return CHIP_NO_ERROR;
}

CHIP_ERROR FabricInfo::FetchRootPubkey(Crypto::P256PublicKey & outPublicKey) const
{
P256PublicKeySpan publicKeySpan;
CHIP_ERROR err = Credentials::ExtractPublicKeyFromChipCert(mRootCert, publicKeySpan);
if (err == CHIP_NO_ERROR)
{
outPublicKey = publicKeySpan;
}

return err;
}

FabricTable::~FabricTable()
{
// Remove all links to every delegate
Expand All @@ -452,24 +464,25 @@ FabricTable::~FabricTable()
}
}

FabricInfo * FabricTable::FindFabric(P256PublicKeySpan rootPubKey, FabricId fabricId)
FabricInfo * FabricTable::FindFabric(const Crypto::P256PublicKey & rootPubKey, FabricId fabricId)
{
for (auto & fabric : mStates)
{
if (!fabric.IsInitialized())
{
continue;
}
P256PublicKeySpan candidatePubKey;
if (fabric.GetRootPubkey(candidatePubKey) != CHIP_NO_ERROR)
P256PublicKey candidatePubKey;
if (fabric.FetchRootPubkey(candidatePubKey) != CHIP_NO_ERROR)
{
continue;
}
if (rootPubKey.data_equal(candidatePubKey) && fabricId == fabric.GetFabricId())
if (rootPubKey.Matches(candidatePubKey) && fabricId == fabric.GetFabricId())
{
return &fabric;
}
}

return nullptr;
}

Expand Down Expand Up @@ -526,6 +539,34 @@ FabricInfo * FabricTable::FindFabricWithCompressedId(CompressedFabricId fabricId
return nullptr;
}

CHIP_ERROR FabricTable::FetchRootCert(FabricIndex fabricIndex, MutableByteSpan & outCert) const
{
const FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
ReturnErrorCodeIf(fabricInfo == nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX);
return fabricInfo->FetchRootCert(outCert);
}

CHIP_ERROR FabricTable::FetchICACert(FabricIndex fabricIndex, MutableByteSpan & outCert) const
{
const FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
ReturnErrorCodeIf(fabricInfo == nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX);
return fabricInfo->FetchICACert(outCert);
}

CHIP_ERROR FabricTable::FetchNOCCert(FabricIndex fabricIndex, MutableByteSpan & outCert) const
{
const FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
ReturnErrorCodeIf(fabricInfo == nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX);
return fabricInfo->FetchNOCCert(outCert);
}

CHIP_ERROR FabricTable::FetchRootPubkey(FabricIndex fabricIndex, Crypto::P256PublicKey & outPublicKey) const
{
const FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
ReturnErrorCodeIf(fabricInfo == nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX);
return fabricInfo->FetchRootPubkey(outPublicKey);
}

CHIP_ERROR FabricTable::Store(FabricIndex fabricIndex)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -658,19 +699,22 @@ CHIP_ERROR FabricTable::AddNewFabric(FabricInfo & newFabric, FabricIndex * outpu
// comparison.
FabricId fabricId;
{
ByteSpan noc;
ReturnErrorOnFailure(newFabric.GetNOCCert(noc));
uint8_t nocBuf[kMaxCHIPCertLength];
MutableByteSpan nocSpan{ nocBuf };
ReturnErrorOnFailure(newFabric.FetchNOCCert(nocSpan));
NodeId unused;
ReturnErrorOnFailure(ExtractNodeIdFabricIdFromOpCert(noc, &unused, &fabricId));
ReturnErrorOnFailure(ExtractNodeIdFabricIdFromOpCert(nocSpan, &unused, &fabricId));
}

for (auto & existingFabric : *this)
{
if (existingFabric.GetFabricId() == fabricId)
{
P256PublicKeySpan existingRootKey, newRootKey;
ReturnErrorOnFailure(existingFabric.GetRootPubkey(existingRootKey));
ReturnErrorOnFailure(newFabric.GetRootPubkey(newRootKey));
if (existingRootKey.data_equal(newRootKey))
P256PublicKey existingRootKey, newRootKey;
ReturnErrorOnFailure(existingFabric.FetchRootPubkey(existingRootKey));
ReturnErrorOnFailure(newFabric.FetchRootPubkey(newRootKey));

if (existingRootKey.Matches(newRootKey))
{
return CHIP_ERROR_FABRIC_EXISTS;
}
Expand Down Expand Up @@ -973,27 +1017,30 @@ CHIP_ERROR FabricTable::SetLastKnownGoodChipEpochTime(System::Clock::Seconds32 l
continue;
}
{
ByteSpan rcac;
SuccessOrExit(err = fabric.GetRootCert(rcac));
uint8_t rcacBuf[kMaxCHIPCertLength];
MutableByteSpan rcacSpan{ rcacBuf };
SuccessOrExit(err = fabric.FetchRootCert(rcacSpan));
chip::System::Clock::Seconds32 rcacNotBefore;
SuccessOrExit(err = Credentials::ExtractNotBeforeFromChipCert(rcac, rcacNotBefore));
SuccessOrExit(err = Credentials::ExtractNotBeforeFromChipCert(rcacSpan, rcacNotBefore));
latestNotBefore = rcacNotBefore > latestNotBefore ? rcacNotBefore : latestNotBefore;
}
{
ByteSpan icac;
SuccessOrExit(err = fabric.GetICACert(icac));
if (!icac.empty())
uint8_t icacBuf[kMaxCHIPCertLength];
MutableByteSpan icacSpan{ icacBuf };
SuccessOrExit(err = fabric.FetchICACert(icacSpan));
if (!icacSpan.empty())
{
chip::System::Clock::Seconds32 icacNotBefore;
ReturnErrorOnFailure(Credentials::ExtractNotBeforeFromChipCert(icac, icacNotBefore));
ReturnErrorOnFailure(Credentials::ExtractNotBeforeFromChipCert(icacSpan, icacNotBefore));
latestNotBefore = icacNotBefore > latestNotBefore ? icacNotBefore : latestNotBefore;
}
}
{
ByteSpan noc;
SuccessOrExit(err = fabric.GetNOCCert(noc));
uint8_t nocBuf[kMaxCHIPCertLength];
MutableByteSpan nocSpan{ nocBuf };
SuccessOrExit(err = fabric.FetchNOCCert(nocSpan));
chip::System::Clock::Seconds32 nocNotBefore;
ReturnErrorOnFailure(Credentials::ExtractNotBeforeFromChipCert(noc, nocNotBefore));
ReturnErrorOnFailure(Credentials::ExtractNotBeforeFromChipCert(nocSpan, nocNotBefore));
latestNotBefore = nocNotBefore > latestNotBefore ? nocNotBefore : latestNotBefore;
}
}
Expand Down
Loading

0 comments on commit 8cbfd2f

Please sign in to comment.