From 54b5b69e553a1143c534a294091dc81f0a7a63f7 Mon Sep 17 00:00:00 2001 From: Michael Sandstedt Date: Thu, 27 Jan 2022 10:51:40 -0600 Subject: [PATCH] Allow for split commissioner / admin topology (#14261) It is possible to implement Matter commissioning such that a commissioner node is not on any fabric, and delegates operational communication and invocation of the CommissioningComplete command to a different administrator node. In this configuration, the commissioner only communicates by PASE and may not ever have received operational credentials. However, such an architecture is not currently supported, as commissioner instances must self-commission to a fabric as a part of initialization. To support such a split commissioner / admin architecture, this amends the commissioner to accept initialization without operational credentials. In this configuration, the commissioner is capable of conducting the commissioning procedure until these are needed. When it is found that credentials aren't available, the commissioner gracefully discontinues commissioning. As part of this change set, fabric index is also no longer specified as part of controller or commissioner initialization. chip-tool had previously specified a fabric index and reused this as fabric ID. That is brittle though because it is not always practical for chip-tool to know the fabric index ahead of time before initialization. Instead, the operational key pair and NOC chain are optionally passed to commissioner / controller initialization. If passed, these are added or merged into the fabric table. Merge works by identifying an existing, matching fabric by comparing root public key and fabric ID. Once the new certificates are merged or added, the controller / commissioner extracts the fabric index from the fabric table. Fixes #13501 --- .../chip-tool/commands/common/CHIPCommand.cpp | 72 +++++++++------- .../chip-tool/commands/common/CHIPCommand.h | 12 ++- src/app/CASESessionManager.cpp | 1 + src/controller/CHIPDeviceController.cpp | 84 ++++++++++++------- src/controller/CHIPDeviceController.h | 20 ++--- .../CHIPDeviceControllerFactory.cpp | 2 - src/controller/CHIPDeviceControllerFactory.h | 3 - src/credentials/CHIPCert.cpp | 9 ++ src/credentials/CHIPCert.h | 15 +++- src/credentials/FabricTable.cpp | 43 ++++++++-- src/credentials/FabricTable.h | 5 +- src/credentials/tests/TestChipCert.cpp | 24 ++++++ 12 files changed, 202 insertions(+), 88 deletions(-) diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index b4a4ed759b7524..3fd71824945e8e 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/examples/chip-tool/commands/common/CHIPCommand.cpp @@ -30,6 +30,7 @@ using DeviceControllerFactory = chip::Controller::DeviceControllerFactory; +constexpr chip::FabricId kIdentityNullFabricId = chip::kUndefinedFabricId; constexpr chip::FabricId kIdentityAlphaFabricId = 1; constexpr chip::FabricId kIdentityBetaFabricId = 2; constexpr chip::FabricId kIdentityGammaFabricId = 3; @@ -48,9 +49,10 @@ CHIP_ERROR CHIPCommand::Run() chip::Controller::FactoryInitParams factoryInitParams; factoryInitParams.fabricStorage = &mFabricStorage; - factoryInitParams.listenPort = static_cast(mDefaultStorage.GetListenPort() + CurrentCommissionerIndex()); + factoryInitParams.listenPort = static_cast(mDefaultStorage.GetListenPort() + CurrentCommissionerId()); ReturnLogErrorOnFailure(DeviceControllerFactory::GetInstance().Init(factoryInitParams)); + ReturnLogErrorOnFailure(InitializeCommissioner(kIdentityNull, kIdentityNullFabricId)); ReturnLogErrorOnFailure(InitializeCommissioner(kIdentityAlpha, kIdentityAlphaFabricId)); ReturnLogErrorOnFailure(InitializeCommissioner(kIdentityBeta, kIdentityBetaFabricId)); ReturnLogErrorOnFailure(InitializeCommissioner(kIdentityGamma, kIdentityGammaFabricId)); @@ -65,6 +67,7 @@ CHIP_ERROR CHIPCommand::Run() // since the CHIP thread and event queue have been stopped, preventing any thread // races. // + ReturnLogErrorOnFailure(ShutdownCommissioner(kIdentityNull)); ReturnLogErrorOnFailure(ShutdownCommissioner(kIdentityAlpha)); ReturnLogErrorOnFailure(ShutdownCommissioner(kIdentityBeta)); ReturnLogErrorOnFailure(ShutdownCommissioner(kIdentityGamma)); @@ -99,7 +102,8 @@ void CHIPCommand::StopTracing() void CHIPCommand::SetIdentity(const char * identity) { std::string name = std::string(identity); - if (name.compare(kIdentityAlpha) != 0 && name.compare(kIdentityBeta) != 0 && name.compare(kIdentityGamma) != 0) + if (name.compare(kIdentityAlpha) != 0 && name.compare(kIdentityBeta) != 0 && name.compare(kIdentityGamma) != 0 && + name.compare(kIdentityNull) != 0) { ChipLogError(chipTool, "Unknown commissioner name: %s. Supported names are [%s, %s, %s]", name.c_str(), kIdentityAlpha, kIdentityBeta, kIdentityGamma); @@ -112,7 +116,8 @@ void CHIPCommand::SetIdentity(const char * identity) std::string CHIPCommand::GetIdentity() { std::string name = mCommissionerName.HasValue() ? mCommissionerName.Value() : kIdentityAlpha; - if (name.compare(kIdentityAlpha) != 0 && name.compare(kIdentityBeta) != 0 && name.compare(kIdentityGamma) != 0) + if (name.compare(kIdentityAlpha) != 0 && name.compare(kIdentityBeta) != 0 && name.compare(kIdentityGamma) != 0 && + name.compare(kIdentityNull) != 0) { ChipLogError(chipTool, "Unknown commissioner name: %s. Supported names are [%s, %s, %s]", name.c_str(), kIdentityAlpha, kIdentityBeta, kIdentityGamma); @@ -122,27 +127,34 @@ std::string CHIPCommand::GetIdentity() return name; } -uint16_t CHIPCommand::CurrentCommissionerIndex() +chip::FabricId CHIPCommand::CurrentCommissionerId() { - uint16_t index = 0; + chip::FabricId id; std::string name = GetIdentity(); if (name.compare(kIdentityAlpha) == 0) { - index = kIdentityAlphaFabricId; + id = kIdentityAlphaFabricId; } else if (name.compare(kIdentityBeta) == 0) { - index = kIdentityBetaFabricId; + id = kIdentityBetaFabricId; } else if (name.compare(kIdentityGamma) == 0) { - index = kIdentityGammaFabricId; + id = kIdentityGammaFabricId; + } + else if (name.compare(kIdentityNull) == 0) + { + id = kIdentityNullFabricId; + } + else + { + VerifyOrDieWithMsg(false, chipTool, "Unknown commissioner name: %s. Supported names are [%s, %s, %s]", name.c_str(), + kIdentityAlpha, kIdentityBeta, kIdentityGamma); } - VerifyOrDieWithMsg(index != 0, chipTool, "Unknown commissioner name: %s. Supported names are [%s, %s, %s]", name.c_str(), - kIdentityAlpha, kIdentityBeta, kIdentityGamma); - return index; + return id; } chip::Controller::DeviceCommissioner & CHIPCommand::CurrentCommissioner() @@ -172,28 +184,32 @@ CHIP_ERROR CHIPCommand::InitializeCommissioner(std::string key, chip::FabricId f VerifyOrReturnError(icac.Alloc(chip::Controller::kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY); VerifyOrReturnError(rcac.Alloc(chip::Controller::kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY); - chip::MutableByteSpan nocSpan(noc.Get(), chip::Controller::kMaxCHIPDERCertLength); - chip::MutableByteSpan icacSpan(icac.Get(), chip::Controller::kMaxCHIPDERCertLength); - chip::MutableByteSpan rcacSpan(rcac.Get(), chip::Controller::kMaxCHIPDERCertLength); - chip::Crypto::P256Keypair ephemeralKey; - ReturnLogErrorOnFailure(ephemeralKey.Initialize()); - // TODO - OpCreds should only be generated for pairing command - // store the credentials in persistent storage, and - // generate when not available in the storage. - ReturnLogErrorOnFailure(mCommissionerStorage.Init(key.c_str())); - ReturnLogErrorOnFailure(mCredIssuerCmds->InitializeCredentialsIssuer(mCommissionerStorage)); - ReturnLogErrorOnFailure(mCredIssuerCmds->GenerateControllerNOCChain(mCommissionerStorage.GetLocalNodeId(), fabricId, - ephemeralKey, rcacSpan, icacSpan, nocSpan)); + if (fabricId != chip::kUndefinedFabricId) + { + + // TODO - OpCreds should only be generated for pairing command + // store the credentials in persistent storage, and + // generate when not available in the storage. + ReturnLogErrorOnFailure(mCommissionerStorage.Init(key.c_str())); + ReturnLogErrorOnFailure(mCredIssuerCmds->InitializeCredentialsIssuer(mCommissionerStorage)); + + chip::MutableByteSpan nocSpan(noc.Get(), chip::Controller::kMaxCHIPDERCertLength); + chip::MutableByteSpan icacSpan(icac.Get(), chip::Controller::kMaxCHIPDERCertLength); + chip::MutableByteSpan rcacSpan(rcac.Get(), chip::Controller::kMaxCHIPDERCertLength); + + ReturnLogErrorOnFailure(ephemeralKey.Initialize()); + ReturnLogErrorOnFailure(mCredIssuerCmds->GenerateControllerNOCChain(mCommissionerStorage.GetLocalNodeId(), fabricId, + ephemeralKey, rcacSpan, icacSpan, nocSpan)); + commissionerParams.operationalKeypair = &ephemeralKey; + commissionerParams.controllerRCAC = rcacSpan; + commissionerParams.controllerICAC = icacSpan; + commissionerParams.controllerNOC = nocSpan; + } commissionerParams.storageDelegate = &mCommissionerStorage; - commissionerParams.fabricIndex = static_cast(fabricId); commissionerParams.operationalCredentialsDelegate = mCredIssuerCmds->GetCredentialIssuer(); - commissionerParams.operationalKeypair = &ephemeralKey; - commissionerParams.controllerRCAC = rcacSpan; - commissionerParams.controllerICAC = icacSpan; - commissionerParams.controllerNOC = nocSpan; commissionerParams.controllerVendorId = chip::VendorId::TestVendor1; ReturnLogErrorOnFailure(DeviceControllerFactory::GetInstance().SetupCommissioner(commissionerParams, *(commissioner.get()))); diff --git a/examples/chip-tool/commands/common/CHIPCommand.h b/examples/chip-tool/commands/common/CHIPCommand.h index 930f5cd31c10a3..2b5d185d9299f2 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.h +++ b/examples/chip-tool/commands/common/CHIPCommand.h @@ -30,6 +30,16 @@ class PersistentStorage; constexpr const char kIdentityAlpha[] = "alpha"; constexpr const char kIdentityBeta[] = "beta"; constexpr const char kIdentityGamma[] = "gamma"; +// The null fabric commissioner is a commissioner that isn't on a fabric. +// This is a legal configuration in which the commissioner delegates +// operational communication and invocation of the commssioning complete +// command to a separate on-fabric administrator node. +// +// The null-fabric-commissioner identity is provided here to demonstrate the +// commissioner portion of such an architecture. The null-fabric-commissioner +// can carry a commissioning flow up until the point of operational channel +// (CASE) communcation. +constexpr const char kIdentityNull[] = "null-fabric-commissioner"; class CHIPCommand : public Command { @@ -98,7 +108,7 @@ class CHIPCommand : public Command private: CHIP_ERROR InitializeCommissioner(std::string key, chip::FabricId fabricId); CHIP_ERROR ShutdownCommissioner(std::string key); - uint16_t CurrentCommissionerIndex(); + chip::FabricId CurrentCommissionerId(); std::map> mCommissioners; chip::Optional mCommissionerName; diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index 26d7e8cf75173e..a183dd424d95a5 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -69,6 +69,7 @@ void CASESessionManager::ReleaseSession(PeerId peerId) CHIP_ERROR CASESessionManager::ResolveDeviceAddress(FabricInfo * fabric, NodeId nodeId) { + VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INCORRECT_STATE); return mConfig.dnsResolver->ResolveNodeId(fabric->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny, Dnssd::Resolver::CacheBypass::On); } diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 675ce4d7dea3fe..6701d1c36f0dc5 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -141,11 +141,11 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) VerifyOrReturnError(params.operationalCredentialsDelegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); mOperationalCredentialsDelegate = params.operationalCredentialsDelegate; - mFabricIndex = params.fabricIndex; - ReturnErrorOnFailure(ProcessControllerNOCChain(params)); - - mFabricInfo = params.systemState->Fabrics()->FindFabricWithIndex(mFabricIndex); - VerifyOrReturnError(mFabricInfo != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + mVendorId = params.controllerVendorId; + if (params.operationalKeypair != nullptr || !params.controllerNOC.empty() || !params.controllerRCAC.empty()) + { + ReturnErrorOnFailure(ProcessControllerNOCChain(params)); + } DeviceProxyInitParams deviceInitParams = { .sessionManager = params.systemState->SessionMgr(), @@ -179,12 +179,13 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) CHIP_ERROR DeviceController::ProcessControllerNOCChain(const ControllerInitParams & params) { FabricInfo newFabric; - - ReturnErrorCodeIf(params.operationalKeypair == nullptr, CHIP_ERROR_INVALID_ARGUMENT); - newFabric.SetOperationalKeypair(params.operationalKeypair); - constexpr uint32_t chipCertAllocatedLen = kMaxCHIPCertLength; chip::Platform::ScopedMemoryBuffer chipCert; + Credentials::P256PublicKeySpan rootPublicKey; + FabricId fabricId; + + ReturnErrorOnFailure(newFabric.SetOperationalKeypair(params.operationalKeypair)); + newFabric.SetVendorId(params.controllerVendorId); ReturnErrorCodeIf(!chipCert.Alloc(chipCertAllocatedLen), CHIP_ERROR_NO_MEMORY); MutableByteSpan chipCertSpan(chipCert.Get(), chipCertAllocatedLen); @@ -208,19 +209,27 @@ CHIP_ERROR DeviceController::ProcessControllerNOCChain(const ControllerInitParam ReturnErrorOnFailure(ConvertX509CertToChipCert(params.controllerNOC, chipCertSpan)); ReturnErrorOnFailure(newFabric.SetNOCCert(chipCertSpan)); - newFabric.SetVendorId(params.controllerVendorId); + ReturnErrorOnFailure(ExtractFabricIdFromCert(chipCertSpan, &fabricId)); - FabricInfo * fabric = params.systemState->Fabrics()->FindFabricWithIndex(mFabricIndex); - ReturnErrorCodeIf(fabric == nullptr, CHIP_ERROR_INCORRECT_STATE); - - ReturnErrorOnFailure(fabric->SetFabricInfo(newFabric)); - mLocalId = fabric->GetPeerId(); - mVendorId = fabric->GetVendorId(); + ReturnErrorOnFailure(newFabric.GetRootPubkey(rootPublicKey)); + mFabricInfo = params.systemState->Fabrics()->FindFabric(rootPublicKey, fabricId); + if (mFabricInfo != nullptr) + { + ReturnErrorOnFailure(mFabricInfo->SetFabricInfo(newFabric)); + } + else + { + FabricIndex fabricIndex; + ReturnErrorOnFailure(params.systemState->Fabrics()->AddNewFabric(newFabric, &fabricIndex)); + mFabricInfo = params.systemState->Fabrics()->FindFabricWithIndex(fabricIndex); + ReturnErrorCodeIf(mFabricInfo == nullptr, CHIP_ERROR_INCORRECT_STATE); + } - mFabricId = fabric->GetFabricId(); + mLocalId = mFabricInfo->GetPeerId(); + mFabricId = mFabricInfo->GetFabricId(); - ChipLogProgress(Controller, "Joined the fabric at index %d. Compressed fabric ID is: 0x" ChipLogFormatX64, mFabricIndex, - ChipLogValueX64(GetCompressedFabricId())); + ChipLogProgress(Controller, "Joined the fabric at index %d. Compressed fabric ID is: 0x" ChipLogFormatX64, + mFabricInfo->GetFabricIndex(), ChipLogValueX64(GetCompressedFabricId())); return CHIP_NO_ERROR; } @@ -235,7 +244,10 @@ CHIP_ERROR DeviceController::Shutdown() mStorageDelegate = nullptr; - mSystemState->Fabrics()->ReleaseFabricIndex(mFabricIndex); + if (mFabricInfo != nullptr) + { + mFabricInfo->Reset(); + } mSystemState->Release(); mSystemState = nullptr; @@ -263,7 +275,7 @@ bool DeviceController::DoesDevicePairingExist(const PeerId & deviceId) void DeviceController::ReleaseOperationalDevice(NodeId remoteDeviceId) { - VerifyOrReturn(mState == State::Initialized, + VerifyOrReturn(mState == State::Initialized && mFabricInfo != nullptr, ChipLogError(Controller, "ReleaseOperationalDevice was called in incorrect state")); mCASESessionManager->ReleaseSession(mFabricInfo->GetPeerIdForNode(remoteDeviceId)); } @@ -273,7 +285,13 @@ void DeviceController::OnFirstMessageDeliveryFailed(const SessionHandle & sessio VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnFirstMessageDeliveryFailed was called in incorrect state")); VerifyOrReturn(session->GetSessionType() == Transport::Session::SessionType::kSecure); - UpdateDevice(session->AsSecureSession()->GetPeerNodeId()); + CHIP_ERROR err = UpdateDevice(session->AsSecureSession()->GetPeerNodeId()); + if (err != CHIP_NO_ERROR) + { + ChipLogError(Controller, + "OnFirstMessageDeliveryFailed was called, but UpdateDevice did not succeed (%" CHIP_ERROR_FORMAT ")", + err.Format()); + } } CHIP_ERROR DeviceController::InitializePairedDeviceList() @@ -773,11 +791,8 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re uint16_t keyID = 0; - FabricInfo * fabric = mSystemState->Fabrics()->FindFabricWithIndex(mFabricIndex); - VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE); VerifyOrExit(mDeviceBeingCommissioned == nullptr, err = CHIP_ERROR_INCORRECT_STATE); - VerifyOrExit(fabric != nullptr, err = CHIP_ERROR_INCORRECT_STATE); err = InitializePairedDeviceList(); SuccessOrExit(err); @@ -808,7 +823,10 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re mIsIPRendezvous = (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle); - device->Init(GetControllerDeviceInitParams(), remoteDeviceId, peerAddress, fabric->GetFabricIndex()); + { + FabricIndex fabricIndex = mFabricInfo != nullptr ? mFabricInfo->GetFabricIndex() : kUndefinedFabricIndex; + device->Init(GetControllerDeviceInitParams(), remoteDeviceId, peerAddress, fabricIndex); + } if (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle) { @@ -1221,8 +1239,10 @@ CHIP_ERROR DeviceCommissioner::ProcessOpCSR(DeviceProxy * proxy, const ByteSpan mOperationalCredentialsDelegate->SetNodeIdForNextNOCRequest(proxy->GetDeviceId()); - FabricInfo * fabric = mSystemState->Fabrics()->FindFabricWithIndex(mFabricIndex); - mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(fabric->GetFabricId()); + if (mFabricInfo != nullptr) + { + mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(mFabricInfo->GetFabricId()); + } return mOperationalCredentialsDelegate->GenerateNOCChain(NOCSRElements, AttestationSignature, dac, ByteSpan(), ByteSpan(), &mDeviceNOCChainCallback); @@ -1821,7 +1841,13 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio } break; case CommissioningStage::kFindOperational: { - UpdateDevice(proxy->GetDeviceId()); + CHIP_ERROR err = UpdateDevice(proxy->GetDeviceId()); + if (err != CHIP_NO_ERROR) + { + ChipLogError(Controller, "Unable to proceed to operational discovery\n"); + CommissioningStageComplete(err); + return; + } } break; case CommissioningStage::kSendComplete: { diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 323c3f81426279..220f9c74079b2f 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -113,9 +113,6 @@ struct ControllerInitParams ByteSpan controllerRCAC; uint16_t controllerVendorId; - - FabricIndex fabricIndex = kMinValidFabricIndex; - FabricId fabricId = kUndefinedFabricId; }; class DLL_EXPORT DevicePairingDelegate @@ -220,7 +217,7 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, virtual CHIP_ERROR GetConnectedDevice(NodeId deviceId, Callback::Callback * onConnection, Callback::Callback * onFailure) { - VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mState == State::Initialized && mFabricInfo != nullptr, CHIP_ERROR_INCORRECT_STATE); return mCASESessionManager->FindOrEstablishSession(mFabricInfo->GetPeerIdForNode(deviceId), onConnection, onFailure); } @@ -234,7 +231,7 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, */ CHIP_ERROR UpdateDevice(NodeId deviceId) { - VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mState == State::Initialized && mFabricInfo != nullptr, CHIP_ERROR_INCORRECT_STATE); return mCASESessionManager->ResolveDeviceAddress(mFabricInfo, deviceId); } @@ -351,9 +348,9 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, SerializableU64Set mPairedDevices; bool mPairedDevicesInitialized; - PeerId mLocalId = PeerId(); - FabricId mFabricId = kUndefinedFabricId; - FabricInfo * mFabricInfo; + PeerId mLocalId = PeerId(); + FabricId mFabricId = kUndefinedFabricId; + FabricInfo * mFabricInfo = nullptr; PersistentStorageDelegate * mStorageDelegate = nullptr; #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD @@ -372,8 +369,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, void PersistNextKeyId(); - FabricIndex mFabricIndex = kMinValidFabricIndex; - OperationalCredentialsDelegate * mOperationalCredentialsDelegate; SessionIDAllocator mIDAllocator; @@ -404,7 +399,10 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, CHIP_ERROR OpenCommissioningWindowInternal(); - PeerId GetPeerIdWithCommissioningWindowOpen() { return mFabricInfo->GetPeerIdForNode(mDeviceWithCommissioningWindowOpen); } + PeerId GetPeerIdWithCommissioningWindowOpen() + { + return mFabricInfo ? mFabricInfo->GetPeerIdForNode(mDeviceWithCommissioningWindowOpen) : PeerId(); + } // TODO - Support opening commissioning window simultaneously on multiple devices Callback::Callback * mCommissioningWindowCallback = nullptr; diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index fdcb2ad0c50837..3e8dc7233ff9a7 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/src/controller/CHIPDeviceControllerFactory.cpp @@ -172,8 +172,6 @@ void DeviceControllerFactory::PopulateInitParams(ControllerInitParams & controll controllerParams.controllerNOC = params.controllerNOC; controllerParams.controllerICAC = params.controllerICAC; controllerParams.controllerRCAC = params.controllerRCAC; - controllerParams.fabricIndex = params.fabricIndex; - controllerParams.fabricId = params.fabricId; controllerParams.storageDelegate = params.storageDelegate; controllerParams.systemState = mSystemState; diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h index fd9ca576416834..fcc44fd5bf6c77 100644 --- a/src/controller/CHIPDeviceControllerFactory.h +++ b/src/controller/CHIPDeviceControllerFactory.h @@ -55,9 +55,6 @@ struct SetupParams ByteSpan controllerICAC; ByteSpan controllerRCAC; - FabricIndex fabricIndex = kMinValidFabricIndex; - FabricId fabricId = kUndefinedFabricId; - uint16_t controllerVendorId; // The Device Pairing Delegated used to initialize a Commissioner diff --git a/src/credentials/CHIPCert.cpp b/src/credentials/CHIPCert.cpp index b892b346e97bcb..546915729179d1 100644 --- a/src/credentials/CHIPCert.cpp +++ b/src/credentials/CHIPCert.cpp @@ -1084,6 +1084,15 @@ CHIP_ERROR ExtractCATsFromOpCert(const ChipCertificateData & opcert, CATValues & return CHIP_NO_ERROR; } +CHIP_ERROR ExtractFabricIdFromCert(const ByteSpan & opcert, FabricId * fabricId) +{ + ChipCertificateSet certSet; + ChipCertificateData certData; + ReturnErrorOnFailure(certSet.Init(&certData, 1)); + ReturnErrorOnFailure(certSet.LoadCert(opcert, BitFlags())); + return ExtractFabricIdFromCert(certData, fabricId); +} + CHIP_ERROR ExtractNodeIdFabricIdFromOpCert(const ByteSpan & opcert, NodeId * nodeId, FabricId * fabricId) { ChipCertificateSet certSet; diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h index d5d67ae5029024..ef722b7ff9c05d 100644 --- a/src/credentials/CHIPCert.h +++ b/src/credentials/CHIPCert.h @@ -775,9 +775,8 @@ CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, ASN1::AS CHIP_ERROR ConvertECDSASignatureDERToRaw(ASN1::ASN1Reader & reader, chip::TLV::TLVWriter & writer, uint64_t tag); /** - * Extract the FabricID from a CHIP certificate in ByteSpan TLV-encoded - * form. This does not perform any sort of validation on the certificate - * structure other than parsing it. + * Extract the Fabric ID from an operational certificate that has already been + * parsed. * * This function can be used to extract Fabric ID from an ICA certificate. * These certificates may not contain a NodeID, so ExtractNodeIdFabricIdFromOpCert() @@ -838,6 +837,16 @@ CHIP_ERROR ExtractCATsFromOpCert(const ByteSpan & opcert, CATValues & cats); */ CHIP_ERROR ExtractCATsFromOpCert(const ChipCertificateData & opcert, CATValues & cats); +/** + * Extract the and Fabric ID from an operational certificate in ByteSpan TLV-encoded + * form. This does not perform any sort of validation on the certificate + * structure other than parsing it. + * + * Can return any error that can be returned from parsing the cert or from the + * ChipCertificateData* version of ExtractNodeIdFabricIdFromOpCert. + */ +CHIP_ERROR ExtractFabricIdFromCert(const ByteSpan & opcert, FabricId * fabricId); + /** * Extract Node ID and Fabric ID from an operational certificate in ByteSpan TLV-encoded * form. This does not perform any sort of validation on the certificate diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp index cb80de5c8cac0f..114c954e3f1487 100644 --- a/src/credentials/FabricTable.cpp +++ b/src/credentials/FabricTable.cpp @@ -241,6 +241,7 @@ CHIP_ERROR FabricInfo::GenerateKey(FabricIndex id, char * key, size_t len) CHIP_ERROR FabricInfo::SetOperationalKeypair(const P256Keypair * keyPair) { + VerifyOrReturnError(keyPair != nullptr, CHIP_ERROR_INVALID_ARGUMENT); P256SerializedKeypair serialized; ReturnErrorOnFailure(keyPair->Serialize(serialized)); if (mOperationalKey == nullptr) @@ -402,6 +403,30 @@ void FabricTable::ReleaseFabricIndex(FabricIndex fabricIndex) } } +FabricInfo * FabricTable::FindFabric(P256PublicKeySpan rootPubKey, FabricId fabricId) +{ + static_assert(kMaxValidFabricIndex <= UINT8_MAX, "Cannot create more fabrics than UINT8_MAX"); + for (FabricIndex i = kMinValidFabricIndex; i <= kMaxValidFabricIndex; i++) + { + FabricInfo * fabric = FindFabricWithIndex(i); + if (fabric == nullptr) + { + continue; + } + P256PublicKeySpan candidatePubKey; + if (fabric->GetRootPubkey(candidatePubKey) != CHIP_NO_ERROR) + { + continue; + } + if (rootPubKey.data_equal(candidatePubKey) && fabricId == fabric->GetFabricId()) + { + LoadFromStorage(fabric); + return fabric; + } + } + return nullptr; +} + FabricInfo * FabricTable::FindFabricWithIndex(FabricIndex fabricIndex) { if (fabricIndex >= kMinValidFabricIndex && fabricIndex <= kMaxValidFabricIndex) @@ -446,21 +471,21 @@ void FabricTable::Reset() } } -CHIP_ERROR FabricTable::Store(FabricIndex id) +CHIP_ERROR FabricTable::Store(FabricIndex index) { CHIP_ERROR err = CHIP_NO_ERROR; FabricInfo * fabric = nullptr; VerifyOrExit(mStorage != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); - fabric = FindFabricWithIndex(id); + fabric = FindFabricWithIndex(index); VerifyOrExit(fabric != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); err = fabric->CommitToStorage(mStorage); exit: if (err == CHIP_NO_ERROR && mDelegate != nullptr) { - ChipLogProgress(Discovery, "Fabric (%d) persisted to storage. Calling OnFabricPersistedToStorage", id); + ChipLogProgress(Discovery, "Fabric (%d) persisted to storage. Calling OnFabricPersistedToStorage", index); mDelegate->OnFabricPersistedToStorage(fabric); } return err; @@ -561,21 +586,21 @@ CHIP_ERROR FabricTable::AddNewFabric(FabricInfo & newFabric, FabricIndex * outpu return CHIP_ERROR_NO_MEMORY; } -CHIP_ERROR FabricTable::Delete(FabricIndex id) +CHIP_ERROR FabricTable::Delete(FabricIndex index) { FabricInfo * fabric = nullptr; CHIP_ERROR err = CHIP_NO_ERROR; bool fabricIsInitialized = false; VerifyOrExit(mStorage != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); - fabric = FindFabricWithIndex(id); + fabric = FindFabricWithIndex(index); fabricIsInitialized = fabric != nullptr && fabric->IsInitialized(); - err = FabricInfo::DeleteFromStorage(mStorage, id); // Delete from storage regardless + err = FabricInfo::DeleteFromStorage(mStorage, index); // Delete from storage regardless exit: if (err == CHIP_NO_ERROR) { - ReleaseFabricIndex(id); + ReleaseFabricIndex(index); if (mDelegate != nullptr && fabricIsInitialized) { if (mFabricCount == 0) @@ -586,8 +611,8 @@ CHIP_ERROR FabricTable::Delete(FabricIndex id) { mFabricCount--; } - ChipLogProgress(Discovery, "Fabric (%d) deleted. Calling OnFabricDeletedFromStorage", id); - mDelegate->OnFabricDeletedFromStorage(id); + ChipLogProgress(Discovery, "Fabric (%d) deleted. Calling OnFabricDeletedFromStorage", index); + mDelegate->OnFabricDeletedFromStorage(index); } } return CHIP_NO_ERROR; diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h index 6ecf4f1cb0f4ee..f4b67b7e46415e 100644 --- a/src/credentials/FabricTable.h +++ b/src/credentials/FabricTable.h @@ -402,10 +402,10 @@ class DLL_EXPORT FabricTable { public: FabricTable() { Reset(); } - CHIP_ERROR Store(FabricIndex id); + CHIP_ERROR Store(FabricIndex index); CHIP_ERROR LoadFromStorage(FabricInfo * info); - CHIP_ERROR Delete(FabricIndex id); + CHIP_ERROR Delete(FabricIndex index); void DeleteAllFabrics(); /** @@ -421,6 +421,7 @@ class DLL_EXPORT FabricTable void ReleaseFabricIndex(FabricIndex fabricIndex); + FabricInfo * FindFabric(Credentials::P256PublicKeySpan rootPubKey, FabricId fabricId); FabricInfo * FindFabricWithIndex(FabricIndex fabricIndex); FabricInfo * FindFabricWithCompressedId(CompressedFabricId fabricId); diff --git a/src/credentials/tests/TestChipCert.cpp b/src/credentials/tests/TestChipCert.cpp index fba1e46f7b2a72..8dd132c1146b9f 100644 --- a/src/credentials/tests/TestChipCert.cpp +++ b/src/credentials/tests/TestChipCert.cpp @@ -1159,6 +1159,19 @@ static void TestChipCert_ExtractNodeIdFabricId(nlTestSuite * inSuite, void * inC certSet.Release(); } + // Test fabric ID extraction from the raw ByteSpan form. + for (auto & testCase : sTestCases) + { + ByteSpan cert; + CHIP_ERROR err = GetTestCert(testCase.Cert, sNullLoadFlag, cert); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + FabricId fabricId; + err = ExtractFabricIdFromCert(cert, &fabricId); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, fabricId == testCase.ExpectedFabricId); + } + // Test fabric ID extraction from the parsed form. for (auto & testCase : sTestCases) { @@ -1175,6 +1188,17 @@ static void TestChipCert_ExtractNodeIdFabricId(nlTestSuite * inSuite, void * inC certSet.Release(); } + // Test fabric ID extraction from the raw ByteSpan form of ICA Cert that doesn't have FabricId. + { + ByteSpan cert; + CHIP_ERROR err = GetTestCert(TestCert::kICA01, sNullLoadFlag, cert); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + FabricId fabricId; + err = ExtractFabricIdFromCert(cert, &fabricId); + NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT); + } + // Test extraction from the parsed form of ICA Cert that doesn't have FabricId. { CHIP_ERROR err = certSet.Init(1);