Skip to content

Commit

Permalink
Implement shadow fail-safe data in FabricTable
Browse files Browse the repository at this point in the history
- FabricTable management during commissioning did not properly
  handle the fact that committing needs to only be done on
  CommissioningComplete, which prevented the AddTrustedRootCertificate,
  UpdateNOC and AddNOC command semantics to be implemented properly
  and prevented proper state observation of operational credential
  clusters server

Fixes project-chip#7695
Issue project-chip#8905
Fixes project-chip#18633
Issue project-chip#17208
Fixes project-chip#15585

This PR:

- Removes direct access to FabricInfo from everywhere, which caused
  possibly stale FabricInfo references during commissioning.
- Remove immediate committing of fabric table on UpdateNOC.
- Make Fabrics, NOCs and TrustedRootCertificates attributes reflect
  proper partial state during fail-safe, by using the shadow data
  capabilities of OperationalCertificateStore and by updates to
  FabricInfo
- Make it possible to unit test fabric table by providing the necessary
  lifecycle public APIs to test every modality of the commissioning flow
- Make Server and DeviceController use OperationalCertificateStore to
  allow proper external lifecycle management of the operational cert
  chain.
- Update all examples/controller code to new API
- Remove dangerous internal APIs from FabricTable and replace with
  direct accessors where needed
- Add more of the necessary spec validations to the UpdateNOC and AddNOC
  flows

Testing done:
- Updated all unit tests, all pass
- Cert tests still pass as before
- Working on further integration tests and unit tests as a follow-up
  noting that current state has not regressed on existing test coverage,
  and that new usage of OperationalCertificateStore class in FabricTable
  gains a large amount of additional coverage transitively via some
  of the existing tests making use of FabricTable.
  • Loading branch information
tcarmelveilleux committed Jun 21, 2022
1 parent 48fbfb6 commit 54b18f9
Show file tree
Hide file tree
Showing 46 changed files with 2,346 additions and 2,011 deletions.
21 changes: 13 additions & 8 deletions examples/chip-tool/commands/common/CHIPCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,13 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()

ReturnLogErrorOnFailure(mDefaultStorage.Init());
ReturnLogErrorOnFailure(mOperationalKeystore.Init(&mDefaultStorage));
ReturnLogErrorOnFailure(mOpCertStore.Init(&mDefaultStorage));

chip::Controller::FactoryInitParams factoryInitParams;

factoryInitParams.fabricIndependentStorage = &mDefaultStorage;
factoryInitParams.operationalKeystore = &mOperationalKeystore;
factoryInitParams.opCertStore = &mOpCertStore;

// Init group data provider that will be used for all group keys and IPKs for the
// chip-tool-configured fabrics. This is OK to do once since the fabric tables
Expand Down Expand Up @@ -126,22 +128,25 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
// Initialize Group Data, including IPK
for (auto it = mCommissioners.begin(); it != mCommissioners.end(); it++)
{
chip::FabricInfo * fabric = it->second->GetFabricInfo();
if ((nullptr != fabric) && (0 != it->first.compare(kIdentityNull)))
{
uint8_t compressed_fabric_id[sizeof(uint64_t)];
chip::MutableByteSpan compressed_fabric_id_span(compressed_fabric_id);
ReturnLogErrorOnFailure(fabric->GetCompressedId(compressed_fabric_id_span));
const chip::Controller::DeviceCommissioner * controller = it->second.get();

chip::FabricIndex fabricIndex = controller->GetFabricIndex();

uint8_t compressed_fabric_id[sizeof(uint64_t)];
chip::MutableByteSpan compressed_fabric_id_span(compressed_fabric_id);
CHIP_ERROR err = controller->GetCompressedFabricIdBytes(compressed_fabric_id_span);

if ((err == CHIP_NO_ERROR) && (0 != it->first.compare(kIdentityNull)))
{
ReturnLogErrorOnFailure(
chip::GroupTesting::InitData(&mGroupDataProvider, fabric->GetFabricIndex(), compressed_fabric_id_span));
chip::GroupTesting::InitData(&mGroupDataProvider, fabricIndex, compressed_fabric_id_span));

// Configure the default IPK for all fabrics used by CHIP-tool. The epoch
// key is the same, but the derived keys will be different for each fabric.
// This has to be done here after we know the Compressed Fabric ID of all
// chip-tool-managed fabrics
chip::ByteSpan defaultIpk = chip::GroupTesting::DefaultIpkValue::GetDefaultIpk();
ReturnLogErrorOnFailure(chip::Credentials::SetSingleIpkEpochKey(&mGroupDataProvider, fabric->GetFabricIndex(),
ReturnLogErrorOnFailure(chip::Credentials::SetSingleIpkEpochKey(&mGroupDataProvider, fabricIndex,
defaultIpk, compressed_fabric_id_span));
}
}
Expand Down
2 changes: 2 additions & 0 deletions examples/chip-tool/commands/common/CHIPCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <commands/common/CredentialIssuerCommands.h>
#include <commands/example/ExampleCredentialIssuerCommands.h>
#include <credentials/GroupDataProviderImpl.h>
#include <credentials/PersistentStorageOpCertStore.h>
#include <crypto/PersistentStorageOperationalKeystore.h>

#pragma once
Expand Down Expand Up @@ -121,6 +122,7 @@ class CHIPCommand : public Command
PersistentStorage mCommissionerStorage;
#endif // CONFIG_USE_LOCAL_STORAGE
chip::PersistentStorageOperationalKeystore mOperationalKeystore;
chip::Credentials::PersistentStorageOpCertStore mOpCertStore;

chip::Credentials::GroupDataProviderImpl mGroupDataProvider{ kMaxGroupsPerFabric, kMaxGroupKeysPerFabric };
CredentialIssuerCommands * mCredIssuerCmds;
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/group/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class AddKeySet : public CHIPCommand
chip::Credentials::GroupDataProvider * groupDataProvider = chip::Credentials::GetGroupDataProvider();
uint8_t compressed_fabric_id[sizeof(uint64_t)];
chip::MutableByteSpan compressed_fabric_id_span(compressed_fabric_id);
ReturnLogErrorOnFailure(CurrentCommissioner().GetFabricInfo()->GetCompressedId(compressed_fabric_id_span));
ReturnLogErrorOnFailure(CurrentCommissioner().GetCompressedFabricIdBytes(compressed_fabric_id_span));

if ((keyPolicy != chip::Credentials::GroupDataProvider::SecurityPolicy::kCacheAndSync &&
keyPolicy != chip::Credentials::GroupDataProvider::SecurityPolicy::kTrustFirst) ||
Expand Down
12 changes: 6 additions & 6 deletions examples/platform/linux/CommissionerMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,20 +171,20 @@ CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort)
ReturnErrorOnFailure(factory.Init(factoryParams));
ReturnErrorOnFailure(factory.SetupCommissioner(params, gCommissioner));

chip::FabricInfo * fabricInfo = gCommissioner.GetFabricInfo();
VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_INTERNAL);
FabricIndex fabricIndex = gCommissioner.GetFabricIndex();
VerifyOrReturnError(fabricInfo != kUndefinedFabricIndex, CHIP_ERROR_INTERNAL);

uint8_t compressedFabricId[sizeof(uint64_t)] = { 0 };
MutableByteSpan compressedFabricIdSpan(compressedFabricId);
ReturnErrorOnFailure(fabricInfo->GetCompressedId(compressedFabricIdSpan));
ReturnErrorOnFailure(gCommissioner.GetCompressedFabricIdBytes(compressedFabricIdSpan));
ChipLogProgress(Support, "Setting up group data for Fabric Index %u with Compressed Fabric ID:",
static_cast<unsigned>(fabricInfo->GetFabricIndex()));
static_cast<unsigned>(fabricIndex));
ChipLogByteSpan(Support, compressedFabricIdSpan);

// TODO: Once ExampleOperationalCredentialsIssuer has support, set default IPK on it as well so
// that commissioned devices get the IPK set from real values rather than "test-only" internal hookups.
ByteSpan defaultIpk = chip::GroupTesting::DefaultIpkValue::GetDefaultIpk();
ReturnLogErrorOnFailure(chip::Credentials::SetSingleIpkEpochKey(&gGroupDataProvider, fabricInfo->GetFabricIndex(), defaultIpk,
ReturnLogErrorOnFailure(chip::Credentials::SetSingleIpkEpochKey(&gGroupDataProvider, fabricIndex, defaultIpk,
compressedFabricIdSpan));

gCommissionerDiscoveryController.SetUserDirectedCommissioningServer(gCommissioner.GetUserDirectedCommissioningServer());
Expand All @@ -194,7 +194,7 @@ CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort)
app::DnssdServer::Instance().AdvertiseOperational();

ChipLogProgress(Support, "InitCommissioner nodeId=0x" ChipLogFormatX64 " fabricIndex=%d",
ChipLogValueX64(gCommissioner.GetNodeId()), fabricInfo->GetFabricIndex());
ChipLogValueX64(gCommissioner.GetNodeId()), fabricIndex);

return CHIP_NO_ERROR;
}
Expand Down
Loading

0 comments on commit 54b18f9

Please sign in to comment.