From bfa240f5f2f9305399df640359a020eebd7f21ec Mon Sep 17 00:00:00 2001 From: Michael Sandstedt Date: Tue, 18 Jan 2022 14:14:23 -0600 Subject: [PATCH] Allow for split commissioner / admin topology 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. To support such an configuration, this commit amends the commissioner to accept initialization to the kUndefinedFabricIndex. In this configuration, the commissioner does not boostrap itself with any credentials, and 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. Because CASESessionManager is using fabric association for operational discovery, discontinuation of the commissioning sequence occurs at attempted operational discovery. Fixes #13501 --- .../chip-tool/commands/common/CHIPCommand.cpp | 57 ++++++++++++------- .../chip-tool/commands/common/CHIPCommand.h | 14 ++++- src/app/CASESessionManager.cpp | 1 + src/controller/CHIPDeviceController.cpp | 39 ++++++++----- src/controller/CHIPDeviceController.h | 3 +- src/controller/CHIPDeviceControllerFactory.h | 2 +- 6 files changed, 77 insertions(+), 39 deletions(-) diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index d1293ccc7bc6bd..83b2a05c0241b0 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/examples/chip-tool/commands/common/CHIPCommand.cpp @@ -30,9 +30,10 @@ using DeviceControllerFactory = chip::Controller::DeviceControllerFactory; -constexpr chip::FabricId kIdentityAlphaFabricId = 1; -constexpr chip::FabricId kIdentityBetaFabricId = 2; -constexpr chip::FabricId kIdentityGammaFabricId = 3; +constexpr chip::FabricIndex kIdentityNullFabricId = chip::kUndefinedFabricIndex; +constexpr chip::FabricIndex kIdentityAlphaFabricId = 1; +constexpr chip::FabricIndex kIdentityBetaFabricId = 2; +constexpr chip::FabricIndex kIdentityGammaFabricId = 3; CHIP_ERROR CHIPCommand::Run() { @@ -51,6 +52,7 @@ CHIP_ERROR CHIPCommand::Run() factoryInitParams.listenPort = static_cast(mDefaultStorage.GetListenPort() + CurrentCommissionerIndex()); 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,7 @@ 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)) { ChipLogError(chipTool, "Unknown commissioner name: %s. Supported names are [%s, %s, %s]", name.c_str(), kIdentityAlpha, kIdentityBeta, kIdentityGamma); @@ -112,7 +115,7 @@ 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)) { ChipLogError(chipTool, "Unknown commissioner name: %s. Supported names are [%s, %s, %s]", name.c_str(), kIdentityAlpha, kIdentityBeta, kIdentityGamma); @@ -122,9 +125,9 @@ std::string CHIPCommand::GetIdentity() return name; } -uint16_t CHIPCommand::CurrentCommissionerIndex() +chip::FabricIndex CHIPCommand::CurrentCommissionerIndex() { - uint16_t index = 0; + chip::FabricIndex index; std::string name = GetIdentity(); if (name.compare(kIdentityAlpha) == 0) @@ -139,9 +142,16 @@ uint16_t CHIPCommand::CurrentCommissionerIndex() { index = kIdentityGammaFabricId; } + else if (name.compare(kIdentityNull) == 0) + { + index = 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; } @@ -156,7 +166,7 @@ CHIP_ERROR CHIPCommand::ShutdownCommissioner(std::string key) return mCommissioners[key].get()->Shutdown(); } -CHIP_ERROR CHIPCommand::InitializeCommissioner(std::string key, chip::FabricId fabricId) +CHIP_ERROR CHIPCommand::InitializeCommissioner(std::string key, chip::FabricIndex fabricIndex) { chip::Platform::ScopedMemoryBuffer noc; chip::Platform::ScopedMemoryBuffer icac; @@ -172,28 +182,33 @@ 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()); + + if (fabricIndex != chip::kUndefinedFabricIndex) + { // 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, + + 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(), fabricIndex, ephemeralKey, rcacSpan, icacSpan, nocSpan)); + commissionerParams.ephemeralKeypair = &ephemeralKey; + commissionerParams.controllerRCAC = rcacSpan; + commissionerParams.controllerICAC = icacSpan; + commissionerParams.controllerNOC = nocSpan; + } commissionerParams.storageDelegate = &mCommissionerStorage; - commissionerParams.fabricIndex = static_cast(fabricId); + commissionerParams.fabricIndex = fabricIndex; commissionerParams.operationalCredentialsDelegate = mCredIssuerCmds->GetCredentialIssuer(); - commissionerParams.ephemeralKeypair = &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..420396072bb989 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 { @@ -96,9 +106,9 @@ class CHIPCommand : public Command ChipDeviceCommissioner & CurrentCommissioner(); private: - CHIP_ERROR InitializeCommissioner(std::string key, chip::FabricId fabricId); + CHIP_ERROR InitializeCommissioner(std::string key, chip::FabricIndex fabricIndex); CHIP_ERROR ShutdownCommissioner(std::string key); - uint16_t CurrentCommissionerIndex(); + chip::FabricIndex CurrentCommissionerIndex(); 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 7a8f657ef5dd6c..f2656a5d885d80 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -142,10 +142,13 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) 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 (mFabricIndex != kUndefinedFabricIndex) + { + ReturnErrorOnFailure(ProcessControllerNOCChain(params)); + mFabricInfo = params.systemState->Fabrics()->FindFabricWithIndex(mFabricIndex); + VerifyOrReturnError(mFabricInfo != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + } DeviceProxyInitParams deviceInitParams = { .sessionManager = params.systemState->SessionMgr(), @@ -215,8 +218,6 @@ CHIP_ERROR DeviceController::ProcessControllerNOCChain(const ControllerInitParam ReturnErrorOnFailure(fabric->SetFabricInfo(newFabric)); mLocalId = fabric->GetPeerId(); - mVendorId = fabric->GetVendorId(); - mFabricId = fabric->GetFabricId(); ChipLogProgress(Controller, "Joined the fabric at index %d. Compressed fabric ID is: 0x" ChipLogFormatX64, mFabricIndex, @@ -273,7 +274,11 @@ 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 (%s)", ErrorStr(err)); + } } CHIP_ERROR DeviceController::InitializePairedDeviceList() @@ -773,11 +778,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 +810,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re mIsIPRendezvous = (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle); - device->Init(GetControllerDeviceInitParams(), remoteDeviceId, peerAddress, fabric->GetFabricIndex()); + device->Init(GetControllerDeviceInitParams(), remoteDeviceId, peerAddress, mFabricIndex); if (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle) { @@ -1214,8 +1216,11 @@ CHIP_ERROR DeviceCommissioner::ProcessOpCSR(DeviceProxy * proxy, const ByteSpan mOperationalCredentialsDelegate->SetNodeIdForNextNOCRequest(proxy->GetDeviceId()); - FabricInfo * fabric = mSystemState->Fabrics()->FindFabricWithIndex(mFabricIndex); - mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(fabric->GetFabricId()); + if (mFabricIndex != kUndefinedFabricIndex) + { + FabricInfo * fabric = mSystemState->Fabrics()->FindFabricWithIndex(mFabricIndex); + mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(fabric->GetFabricId()); + } return mOperationalCredentialsDelegate->GenerateNOCChain(NOCSRElements, AttestationSignature, dac, ByteSpan(), ByteSpan(), &mDeviceNOCChainCallback); @@ -1797,7 +1802,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 53e1d3dbb3651f..6da9ffbf1282b1 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -114,6 +114,7 @@ struct ControllerInitParams uint16_t controllerVendorId; + /* Set fabricIndex to kUndefinedFabricIndex to defeat self commissioning. */ FabricIndex fabricIndex = kMinValidFabricIndex; FabricId fabricId = kUndefinedFabricId; }; @@ -367,7 +368,7 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, void PersistNextKeyId(); - FabricIndex mFabricIndex = kMinValidFabricIndex; + FabricIndex mFabricIndex = kUndefinedFabricIndex; OperationalCredentialsDelegate * mOperationalCredentialsDelegate; diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h index 87e1fb79426545..e94c60ef7a18b5 100644 --- a/src/controller/CHIPDeviceControllerFactory.h +++ b/src/controller/CHIPDeviceControllerFactory.h @@ -55,7 +55,7 @@ struct SetupParams ByteSpan controllerICAC; ByteSpan controllerRCAC; - FabricIndex fabricIndex = kMinValidFabricIndex; + FabricIndex fabricIndex = kUndefinedFabricIndex; FabricId fabricId = kUndefinedFabricId; uint16_t controllerVendorId;