From 87a978c40f5b98b2345d24da4d35f81d241d1235 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 1 Oct 2024 15:32:38 -0700 Subject: [PATCH 01/10] Refactor to use API methods instead of PushCommand for pairing --- .../commands/common/CHIPCommand.cpp | 3 +- .../commands/common/CHIPCommand.h | 3 +- .../fabric-sync/FabricSyncCommand.cpp | 46 +- .../commands/fabric-sync/FabricSyncCommand.h | 1 - .../commands/pairing/PairingCommand.cpp | 13 - .../commands/pairing/PairingCommand.h | 26 - .../device_manager/DeviceManager.cpp | 70 ++- .../device_manager/DeviceManager.h | 2 +- .../device_manager/PairingManager.cpp | 454 +++++++++++++++++- .../device_manager/PairingManager.h | 115 ++++- 10 files changed, 595 insertions(+), 138 deletions(-) diff --git a/examples/fabric-admin/commands/common/CHIPCommand.cpp b/examples/fabric-admin/commands/common/CHIPCommand.cpp index b18eb9f2de472a..6b9e9162e0c861 100644 --- a/examples/fabric-admin/commands/common/CHIPCommand.cpp +++ b/examples/fabric-admin/commands/common/CHIPCommand.cpp @@ -182,7 +182,8 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack() mCredIssuerCmds->SetCredentialIssuerOption(CredentialIssuerCommands::CredentialIssuerOptions::kAllowTestCdSigningKey, allowTestCdSigningKey); - PairingManager::Instance().Init(&CurrentCommissioner()); + ReturnLogErrorOnFailure(PairingManager::Instance().Init(&CurrentCommissioner(), mCredIssuerCmds)); + return CHIP_NO_ERROR; } diff --git a/examples/fabric-admin/commands/common/CHIPCommand.h b/examples/fabric-admin/commands/common/CHIPCommand.h index abd68f6344ea44..79d1fcb0fe5efd 100644 --- a/examples/fabric-admin/commands/common/CHIPCommand.h +++ b/examples/fabric-admin/commands/common/CHIPCommand.h @@ -120,6 +120,8 @@ class CHIPCommand : public Command StopWaiting(); } + static chip::app::DefaultICDClientStorage sICDClientStorage; + protected: // Will be called in a setting in which it's safe to touch the CHIP // stack. The rules for Run() are as follows: @@ -167,7 +169,6 @@ class CHIPCommand : public Command static chip::Crypto::RawKeySessionKeystore sSessionKeystore; static chip::Credentials::GroupDataProviderImpl sGroupDataProvider; - static chip::app::DefaultICDClientStorage sICDClientStorage; static chip::app::CheckInHandler sCheckInHandler; CredentialIssuerCommands * mCredIssuerCmds; diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp index 6e1b198ad9e924..c8d9f3da96b323 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp @@ -94,15 +94,8 @@ CHIP_ERROR FabricSyncAddBridgeCommand::RunCommand(NodeId remoteId) return CHIP_NO_ERROR; } - PairingCommand * pairingCommand = static_cast(CommandMgr().GetCommandByName("pairing", "already-discovered")); + PairingManager::Instance().SetCommissioningDelegate(this); - if (pairingCommand == nullptr) - { - ChipLogError(NotSpecified, "Pairing already-discovered command is not available"); - return CHIP_ERROR_NOT_IMPLEMENTED; - } - - pairingCommand->RegisterCommissioningDelegate(this); mBridgeNodeId = remoteId; DeviceMgr().PairRemoteFabricBridge(remoteId, mSetupPINCode, reinterpret_cast(mRemoteAddr.data()), mRemotePort); @@ -146,16 +139,7 @@ CHIP_ERROR FabricSyncRemoveBridgeCommand::RunCommand() mBridgeNodeId = bridgeNodeId; - PairingCommand * pairingCommand = static_cast(CommandMgr().GetCommandByName("pairing", "unpair")); - - if (pairingCommand == nullptr) - { - ChipLogError(NotSpecified, "Pairing unpair command is not available"); - return CHIP_ERROR_NOT_IMPLEMENTED; - } - - pairingCommand->RegisterPairingDelegate(this); - + PairingManager::Instance().SetPairingDelegate(this); DeviceMgr().UnpairRemoteFabricBridge(); return CHIP_NO_ERROR; @@ -203,10 +187,7 @@ CHIP_ERROR FabricSyncAddLocalBridgeCommand::RunCommand(NodeId deviceId) return CHIP_NO_ERROR; } - PairingCommand * pairingCommand = static_cast(CommandMgr().GetCommandByName("pairing", "already-discovered")); - VerifyOrDie(pairingCommand != nullptr); - - pairingCommand->RegisterCommissioningDelegate(this); + PairingManager::Instance().SetCommissioningDelegate(this); mLocalBridgeNodeId = deviceId; if (mSetupPINCode.HasValue()) @@ -259,16 +240,7 @@ CHIP_ERROR FabricSyncRemoveLocalBridgeCommand::RunCommand() mLocalBridgeNodeId = bridgeNodeId; - PairingCommand * pairingCommand = static_cast(CommandMgr().GetCommandByName("pairing", "unpair")); - - if (pairingCommand == nullptr) - { - ChipLogError(NotSpecified, "Pairing unpair command is not available"); - return CHIP_ERROR_NOT_IMPLEMENTED; - } - - pairingCommand->RegisterPairingDelegate(this); - + PairingManager::Instance().SetPairingDelegate(this); DeviceMgr().UnpairLocalFabricBridge(); return CHIP_NO_ERROR; @@ -287,15 +259,7 @@ void FabricSyncDeviceCommand::OnCommissioningWindowOpened(NodeId deviceId, CHIP_ { NodeId nodeId = DeviceMgr().GetNextAvailableNodeId(); - PairingCommand * pairingCommand = static_cast(CommandMgr().GetCommandByName("pairing", "code")); - - if (pairingCommand == nullptr) - { - ChipLogError(NotSpecified, "Pairing code command is not available"); - return; - } - - pairingCommand->RegisterCommissioningDelegate(this); + PairingManager::Instance().SetCommissioningDelegate(this); mAssignedNodeId = nodeId; usleep(kCommissionPrepareTimeMs * 1000); diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h index e80f10133853f7..b6c4fd5a2f7cb7 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h @@ -19,7 +19,6 @@ #pragma once #include -#include #include // Constants diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.cpp b/examples/fabric-admin/commands/pairing/PairingCommand.cpp index 2e1cec184c2e3e..c39cdd068af289 100644 --- a/examples/fabric-admin/commands/pairing/PairingCommand.cpp +++ b/examples/fabric-admin/commands/pairing/PairingCommand.cpp @@ -442,12 +442,6 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) ChipLogProgress(NotSpecified, "Device commissioning Failure: %s", ErrorStr(err)); } - if (mCommissioningDelegate) - { - mCommissioningDelegate->OnCommissioningComplete(nodeId, err); - this->UnregisterCommissioningDelegate(); - } - SetCommandExitStatus(err); } @@ -575,13 +569,6 @@ void PairingCommand::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E ChipLogProgress(NotSpecified, "Device unpair Failure: " ChipLogFormatX64 " %s", ChipLogValueX64(nodeId), ErrorStr(err)); } - PairingDelegate * pairingDelegate = command->GetPairingDelegate(); - if (pairingDelegate) - { - pairingDelegate->OnDeviceRemoved(nodeId, err); - command->UnregisterPairingDelegate(); - } - command->SetCommandExitStatus(err); } diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.h b/examples/fabric-admin/commands/pairing/PairingCommand.h index 73e717b93757c2..3c857ce89f0b32 100644 --- a/examples/fabric-admin/commands/pairing/PairingCommand.h +++ b/examples/fabric-admin/commands/pairing/PairingCommand.h @@ -45,20 +45,6 @@ enum class PairingNetworkType Thread, }; -class CommissioningDelegate -{ -public: - virtual void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR err) = 0; - virtual ~CommissioningDelegate() = default; -}; - -class PairingDelegate -{ -public: - virtual void OnDeviceRemoved(chip::NodeId deviceId, CHIP_ERROR err) = 0; - virtual ~PairingDelegate() = default; -}; - class PairingCommand : public CHIPCommand, public chip::Controller::DevicePairingDelegate, public chip::Controller::DeviceDiscoveryDelegate, @@ -226,15 +212,6 @@ class PairingCommand : public CHIPCommand, const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, chip::Credentials::AttestationVerificationResult attestationResult) override; - /////////// CommissioningDelegate ///////// - void RegisterCommissioningDelegate(CommissioningDelegate * delegate) { mCommissioningDelegate = delegate; } - void UnregisterCommissioningDelegate() { mCommissioningDelegate = nullptr; } - - /////////// PairingDelegate ///////// - void RegisterPairingDelegate(PairingDelegate * delegate) { mPairingDelegate = delegate; } - void UnregisterPairingDelegate() { mPairingDelegate = nullptr; } - PairingDelegate * GetPairingDelegate() { return mPairingDelegate; } - private: CHIP_ERROR RunInternal(NodeId remoteId); CHIP_ERROR Pair(NodeId remoteId, PeerAddress address); @@ -290,9 +267,6 @@ class PairingCommand : public CHIPCommand, chip::Platform::UniquePtr mCurrentFabricRemover; chip::Callback::Callback mCurrentFabricRemoveCallback; - CommissioningDelegate * mCommissioningDelegate = nullptr; - PairingDelegate * mPairingDelegate = nullptr; - static void OnCurrentFabricRemove(void * context, NodeId remoteNodeId, CHIP_ERROR status); void PersistIcdInfo(); }; diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index f34a8443feaa68..4f7b1c77551e80 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -20,7 +20,6 @@ #include #include -#include #include #include @@ -153,52 +152,42 @@ void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpoin void DeviceManager::PairRemoteFabricBridge(NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, uint16_t deviceRemotePort) { - StringBuilder commandBuilder; - - commandBuilder.Add("pairing already-discovered "); - commandBuilder.AddFormat("%lu %d %s %d", nodeId, setupPINCode, deviceRemoteIp, deviceRemotePort); - - PushCommand(commandBuilder.c_str()); + if (PairingManager::Instance().PairDevice(nodeId, setupPINCode, deviceRemoteIp, deviceRemotePort) != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to pair remote fabric bridge " ChipLogFormatX64, ChipLogValueX64(nodeId)); + } } void DeviceManager::PairRemoteDevice(NodeId nodeId, const char * payload) { - StringBuilder commandBuilder; - - commandBuilder.Add("pairing code "); - commandBuilder.AddFormat("%lu %s", nodeId, payload); - - PushCommand(commandBuilder.c_str()); + if (PairingManager::Instance().PairDeviceWithCode(nodeId, payload) != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to pair remote device " ChipLogFormatX64, ChipLogValueX64(nodeId)); + } } void DeviceManager::PairLocalFabricBridge(NodeId nodeId) { - StringBuilder commandBuilder; - - commandBuilder.Add("pairing already-discovered "); - commandBuilder.AddFormat("%lu %d ::1 %d", nodeId, mLocalBridgeSetupPinCode, mLocalBridgePort); - - PushCommand(commandBuilder.c_str()); + if (PairingManager::Instance().PairDevice(nodeId, mLocalBridgeSetupPinCode, "::1", mLocalBridgePort) != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to pair local fabric bridge " ChipLogFormatX64, ChipLogValueX64(nodeId)); + } } void DeviceManager::UnpairRemoteFabricBridge() { - StringBuilder commandBuilder; - - commandBuilder.Add("pairing unpair "); - commandBuilder.AddFormat("%lu", mRemoteBridgeNodeId); - - PushCommand(commandBuilder.c_str()); + if (PairingManager::Instance().UnpairDevice(mRemoteBridgeNodeId) != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to unpair remote bridge device " ChipLogFormatX64, ChipLogValueX64(mRemoteBridgeNodeId)); + } } void DeviceManager::UnpairLocalFabricBridge() { - StringBuilder commandBuilder; - - commandBuilder.Add("pairing unpair "); - commandBuilder.AddFormat("%lu", mLocalBridgeNodeId); - - PushCommand(commandBuilder.c_str()); + if (PairingManager::Instance().UnpairDevice(mLocalBridgeNodeId) != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to unpair local bridge device " ChipLogFormatX64, ChipLogValueX64(mLocalBridgeNodeId)); + } } void DeviceManager::SubscribeRemoteFabricBridge() @@ -390,20 +379,15 @@ void DeviceManager::HandleAttributePartsListUpdate(TLV::TLVReader & data) if (mAutoSyncEnabled) { - StringBuilder commandBuilder; - commandBuilder.Add("pairing unpair "); - commandBuilder.AddFormat("%lu", device->GetNodeId()); - - PairingCommand * pairingCommand = static_cast(CommandMgr().GetCommandByName("pairing", "unpair")); - - if (pairingCommand == nullptr) + NodeId nodeId = device->GetNodeId(); + if (PairingManager::Instance().UnpairDevice(nodeId) != CHIP_NO_ERROR) { - ChipLogError(NotSpecified, "Pairing code command is not available"); - return; + ChipLogError(NotSpecified, "Failed to unpair device " ChipLogFormatX64, ChipLogValueX64(nodeId)); + } + else + { + PairingManager::Instance().SetPairingDelegate(this); } - - pairingCommand->RegisterPairingDelegate(this); - PushCommand(commandBuilder.c_str()); } } } diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 939329be82c691..5e43e78e3a8aed 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -19,7 +19,7 @@ #pragma once #include -#include +#include #include #include diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index aa56af4bc6bf4e..3be046a4c5f9ed 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -18,21 +18,100 @@ #include "PairingManager.h" -#include -#include -#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#if defined(PW_RPC_ENABLED) +#include +#endif using namespace ::chip; +using namespace ::chip::Controller; + +namespace { + +CHIP_ERROR GetPayload(const char * setUpCode, SetupPayload & payload) +{ + VerifyOrReturnValue(setUpCode, CHIP_ERROR_INVALID_ARGUMENT); + bool isQRCode = strncmp(setUpCode, kQRCodePrefix, strlen(kQRCodePrefix)) == 0; + if (isQRCode) + { + ReturnErrorOnFailure(QRCodeSetupPayloadParser(setUpCode).populatePayload(payload)); + VerifyOrReturnError(payload.isValidQRCodePayload(), CHIP_ERROR_INVALID_ARGUMENT); + } + else + { + ReturnErrorOnFailure(ManualSetupPayloadParser(setUpCode).populatePayload(payload)); + VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT); + } + + return CHIP_NO_ERROR; +} + +bool ParseAddressWithInterface(const char * addressString, Inet::IPAddress & address, Inet::InterfaceId & interfaceId) +{ + struct addrinfo hints; + struct addrinfo * result; + int ret; + + memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_DGRAM; + ret = getaddrinfo(addressString, nullptr, &hints, &result); + if (ret < 0) + { + ChipLogError(NotSpecified, "Invalid address: %s", addressString); + return false; + } + + if (result->ai_family == AF_INET6) + { + struct sockaddr_in6 * addr = reinterpret_cast(result->ai_addr); + address = Inet::IPAddress::FromSockAddr(*addr); + interfaceId = Inet::InterfaceId(addr->sin6_scope_id); + } +#if INET_CONFIG_ENABLE_IPV4 + else if (result->ai_family == AF_INET) + { + address = Inet::IPAddress::FromSockAddr(*reinterpret_cast(result->ai_addr)); + interfaceId = Inet::InterfaceId::Null(); + } +#endif // INET_CONFIG_ENABLE_IPV4 + else + { + ChipLogError(NotSpecified, "Unsupported address: %s", addressString); + freeaddrinfo(result); + return false; + } + + freeaddrinfo(result); + return true; +} + +} // namespace PairingManager::PairingManager() : mOnOpenCommissioningWindowCallback(OnOpenCommissioningWindowResponse, this), - mOnOpenCommissioningWindowVerifierCallback(OnOpenCommissioningWindowVerifierResponse, this) + mOnOpenCommissioningWindowVerifierCallback(OnOpenCommissioningWindowVerifierResponse, this), + mCurrentFabricRemoveCallback(OnCurrentFabricRemove, this) {} -void PairingManager::Init(Controller::DeviceCommissioner * commissioner) +CHIP_ERROR PairingManager::Init(Controller::DeviceCommissioner * commissioner, CredentialIssuerCommands * credIssuerCmds) { - VerifyOrDie(mCommissioner == nullptr); + VerifyOrReturnError(commissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); mCommissioner = commissioner; + + VerifyOrReturnError(credIssuerCmds != nullptr, CHIP_ERROR_INCORRECT_STATE); + mCredIssuerCmds = credIssuerCmds; + + return CHIP_NO_ERROR; } CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId endpointId, uint16_t commissioningTimeoutSec, @@ -168,3 +247,366 @@ void PairingManager::OnOpenCommissioningWindowVerifierResponse(void * context, N // Reset the window opener once the window operation is complete self->mWindowOpener.reset(); } + +void PairingManager::OnStatusUpdate(DevicePairingDelegate::Status status) +{ + switch (status) + { + case DevicePairingDelegate::Status::SecurePairingSuccess: + ChipLogProgress(NotSpecified, "Secure Pairing Success"); + ChipLogProgress(NotSpecified, "CASE establishment successful"); + break; + case DevicePairingDelegate::Status::SecurePairingFailed: + ChipLogError(NotSpecified, "Secure Pairing Failed"); + break; + } +} + +void PairingManager::OnPairingComplete(CHIP_ERROR err) +{ + if (err == CHIP_NO_ERROR) + { + ChipLogProgress(NotSpecified, "Pairing Success"); + ChipLogProgress(NotSpecified, "PASE establishment successful"); + } + else + { + ChipLogProgress(NotSpecified, "Pairing Failure: %s", ErrorStr(err)); + } + + if (err != CHIP_NO_ERROR) + { + } +} + +void PairingManager::OnPairingDeleted(CHIP_ERROR err) +{ + if (err == CHIP_NO_ERROR) + { + ChipLogProgress(NotSpecified, "Pairing Deleted Success"); + } + else + { + ChipLogProgress(NotSpecified, "Pairing Deleted Failure: %s", ErrorStr(err)); + } +} + +void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) +{ + if (err == CHIP_NO_ERROR) + { + // print to console + fprintf(stderr, "New device with Node ID: 0x%lx has been successfully added.\n", nodeId); + // CurrentCommissioner() has a lifetime that is the entire life of the application itself + // so it is safe to provide to StartDeviceSynchronization. + DeviceSynchronizer::Instance().StartDeviceSynchronization(mCommissioner, nodeId, mDeviceIsICD); + } + else + { + // When ICD device commissioning fails, the ICDClientInfo stored in OnICDRegistrationComplete needs to be removed. + if (mDeviceIsICD) + { + CHIP_ERROR deleteEntryError = + CHIPCommand::sICDClientStorage.DeleteEntry(ScopedNodeId(nodeId, mCommissioner->GetFabricIndex())); + if (deleteEntryError != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to delete ICD entry: %s", ErrorStr(err)); + } + } + ChipLogProgress(NotSpecified, "Device commissioning Failure: %s", ErrorStr(err)); + } + + if (mCommissioningDelegate) + { + mCommissioningDelegate->OnCommissioningComplete(nodeId, err); + this->SetCommissioningDelegate(nullptr); + } +} + +void PairingManager::OnReadCommissioningInfo(const Controller::ReadCommissioningInfo & info) +{ + ChipLogProgress(AppServer, "OnReadCommissioningInfo - vendorId=0x%04X productId=0x%04X", info.basic.vendorId, + info.basic.productId); + + // The string in CharSpan received from the device is not null-terminated, we use std::string here for coping and + // appending a numm-terminator at the end of the string. + std::string userActiveModeTriggerInstruction; + + // Note: the callback doesn't own the buffer, should make a copy if it will be used it later. + if (info.icd.userActiveModeTriggerInstruction.size() != 0) + { + userActiveModeTriggerInstruction = + std::string(info.icd.userActiveModeTriggerInstruction.data(), info.icd.userActiveModeTriggerInstruction.size()); + } + + if (info.icd.userActiveModeTriggerHint.HasAny()) + { + ChipLogProgress(AppServer, "OnReadCommissioningInfo - LIT UserActiveModeTriggerHint=0x%08x", + info.icd.userActiveModeTriggerHint.Raw()); + ChipLogProgress(AppServer, "OnReadCommissioningInfo - LIT UserActiveModeTriggerInstruction=%s", + userActiveModeTriggerInstruction.c_str()); + } + ChipLogProgress(AppServer, "OnReadCommissioningInfo ICD - IdleModeDuration=%u activeModeDuration=%u activeModeThreshold=%u", + info.icd.idleModeDuration, info.icd.activeModeDuration, info.icd.activeModeThreshold); +} + +void PairingManager::OnICDRegistrationComplete(ScopedNodeId nodeId, uint32_t icdCounter) +{ + char icdSymmetricKeyHex[Crypto::kAES_CCM128_Key_Length * 2 + 1]; + + Encoding::BytesToHex(mICDSymmetricKey.Value().data(), mICDSymmetricKey.Value().size(), icdSymmetricKeyHex, + sizeof(icdSymmetricKeyHex), Encoding::HexFlags::kNullTerminate); + + app::ICDClientInfo clientInfo; + clientInfo.peer_node = nodeId; + clientInfo.monitored_subject = mICDMonitoredSubject.Value(); + clientInfo.start_icd_counter = icdCounter; + + CHIP_ERROR err = CHIPCommand::sICDClientStorage.SetKey(clientInfo, mICDSymmetricKey.Value()); + if (err == CHIP_NO_ERROR) + { + err = CHIPCommand::sICDClientStorage.StoreEntry(clientInfo); + } + + if (err != CHIP_NO_ERROR) + { + CHIPCommand::sICDClientStorage.RemoveKey(clientInfo); + ChipLogError(NotSpecified, "Failed to persist symmetric key for " ChipLogFormatX64 ": %s", + ChipLogValueX64(nodeId.GetNodeId()), err.AsString()); + return; + } + + mDeviceIsICD = true; + + ChipLogProgress(NotSpecified, "Saved ICD Symmetric key for " ChipLogFormatX64, ChipLogValueX64(nodeId.GetNodeId())); + ChipLogProgress(NotSpecified, + "ICD Registration Complete for device " ChipLogFormatX64 " / Check-In NodeID: " ChipLogFormatX64 + " / Monitored Subject: " ChipLogFormatX64 " / Symmetric Key: %s / ICDCounter %u", + ChipLogValueX64(nodeId.GetNodeId()), ChipLogValueX64(mICDCheckInNodeId.Value()), + ChipLogValueX64(mICDMonitoredSubject.Value()), icdSymmetricKeyHex, icdCounter); +} + +void PairingManager::OnICDStayActiveComplete(ScopedNodeId deviceId, uint32_t promisedActiveDuration) +{ + ChipLogProgress(NotSpecified, "ICD Stay Active Complete for device " ChipLogFormatX64 " / promisedActiveDuration: %u", + ChipLogValueX64(deviceId.GetNodeId()), promisedActiveDuration); +} + +void PairingManager::OnDiscoveredDevice(const Dnssd::CommissionNodeData & nodeData) +{ + // Ignore nodes with closed commissioning window + VerifyOrReturn(nodeData.commissioningMode != 0); + + auto & resolutionData = nodeData; + + const uint16_t port = resolutionData.port; + char buf[Inet::IPAddress::kMaxStringLength]; + resolutionData.ipAddress[0].ToString(buf); + ChipLogProgress(NotSpecified, "Discovered Device: %s:%u", buf, port); + + // Stop Mdns discovery. + auto err = mCommissioner->StopCommissionableDiscovery(); + + // Some platforms does not implement a mechanism to stop mdns browse, so + // we just ignore CHIP_ERROR_NOT_IMPLEMENTED instead of bailing out. + if (CHIP_NO_ERROR != err && CHIP_ERROR_NOT_IMPLEMENTED != err) + { + return; + } + + mCommissioner->RegisterDeviceDiscoveryDelegate(nullptr); + + auto interfaceId = resolutionData.ipAddress[0].IsIPv6LinkLocal() ? resolutionData.interfaceId : Inet::InterfaceId::Null(); + auto peerAddress = Transport::PeerAddress::UDP(resolutionData.ipAddress[0], port, interfaceId); + err = Pair(mNodeId, peerAddress); + if (CHIP_NO_ERROR != err) + { + } +} + +Optional PairingManager::FailSafeExpiryTimeoutSecs() const +{ + // No manual input, so do not need to extend. + return Optional(); +} + +bool PairingManager::ShouldWaitAfterDeviceAttestation() +{ + // If there is a vendor ID and product ID, request OnDeviceAttestationCompleted(). + // Currently this is added in the case that the example is performing reverse commissioning, + // but it would be an improvement to store that explicitly. + // TODO: Issue #35297 - [Fabric Sync] Improve where we get VID and PID when validating CCTRL CommissionNode command + SetupPayload payload; + CHIP_ERROR err = GetPayload(mOnboardingPayload, payload); + return err == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0); +} + +void PairingManager::OnDeviceAttestationCompleted(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device, + const Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, + Credentials::AttestationVerificationResult attestationResult) +{ + SetupPayload payload; + CHIP_ERROR parse_error = GetPayload(mOnboardingPayload, payload); + if (parse_error == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0)) + { + if (payload.vendorID == 0 || payload.productID == 0) + { + ChipLogProgress(NotSpecified, + "Failed validation: vendorID or productID must not be 0." + "Requested VID: %u, Requested PID: %u.", + payload.vendorID, payload.productID); + deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( + device, Credentials::AttestationVerificationResult::kInvalidArgument); + return; + } + + if (payload.vendorID != info.BasicInformationVendorId() || payload.productID != info.BasicInformationProductId()) + { + ChipLogProgress(NotSpecified, + "Failed validation of vendorID or productID." + "Requested VID: %u, Requested PID: %u," + "Detected VID: %u, Detected PID %u.", + payload.vendorID, payload.productID, info.BasicInformationVendorId(), info.BasicInformationProductId()); + deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( + device, + payload.vendorID == info.BasicInformationVendorId() + ? Credentials::AttestationVerificationResult::kDacProductIdMismatch + : Credentials::AttestationVerificationResult::kDacVendorIdMismatch); + return; + } + + // NOTE: This will log errors even if the attestion was successful. + auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); + if (CHIP_NO_ERROR != err) + { + } + return; + } + + // Don't bypass attestation, continue with error. + auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); + if (CHIP_NO_ERROR != err) + { + } +} + +CommissioningParameters PairingManager::GetCommissioningParameters() +{ + auto params = CommissioningParameters(); + params.SetSkipCommissioningComplete(false); + params.SetDeviceAttestationDelegate(this); + + if (mICDRegistration.ValueOr(false)) + { + params.SetICDRegistrationStrategy(ICDRegistrationStrategy::kBeforeComplete); + + if (!mICDSymmetricKey.HasValue()) + { + Crypto::DRBG_get_bytes(mRandomGeneratedICDSymmetricKey, sizeof(mRandomGeneratedICDSymmetricKey)); + mICDSymmetricKey.SetValue(ByteSpan(mRandomGeneratedICDSymmetricKey)); + } + if (!mICDCheckInNodeId.HasValue()) + { + mICDCheckInNodeId.SetValue(mCommissioner->GetNodeId()); + } + if (!mICDMonitoredSubject.HasValue()) + { + mICDMonitoredSubject.SetValue(mICDCheckInNodeId.Value()); + } + if (!mICDClientType.HasValue()) + { + mICDClientType.SetValue(app::Clusters::IcdManagement::ClientTypeEnum::kPermanent); + } + // These Optionals must have values now. + // The commissioner will verify these values. + params.SetICDSymmetricKey(mICDSymmetricKey.Value()); + if (mICDStayActiveDurationMsec.HasValue()) + { + params.SetICDStayActiveDurationMsec(mICDStayActiveDurationMsec.Value()); + } + params.SetICDCheckInNodeId(mICDCheckInNodeId.Value()); + params.SetICDMonitoredSubject(mICDMonitoredSubject.Value()); + params.SetICDClientType(mICDClientType.Value()); + } + + return params; +} + +CHIP_ERROR PairingManager::Pair(NodeId remoteId, Transport::PeerAddress address) +{ + auto params = RendezvousParameters().SetSetupPINCode(mSetupPINCode).SetDiscriminator(mDiscriminator).SetPeerAddress(address); + + CHIP_ERROR err = CHIP_NO_ERROR; + auto commissioningParams = GetCommissioningParameters(); + err = CurrentCommissioner().PairDevice(remoteId, params, commissioningParams); + + return err; +} + +void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_ERROR err) +{ + PairingManager * self = reinterpret_cast(context); + VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnCurrentFabricRemove: context is null")); + + if (err == CHIP_NO_ERROR) + { + // print to console + fprintf(stderr, "Device with Node ID: 0x%lx has been successfully removed.\n", nodeId); + +#if defined(PW_RPC_ENABLED) + app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(self->CurrentCommissioner().GetFabricIndex(), nodeId); + RemoveSynchronizedDevice(nodeId); +#endif + } + else + { + ChipLogProgress(NotSpecified, "Device unpair Failure: " ChipLogFormatX64 " %s", ChipLogValueX64(nodeId), ErrorStr(err)); + } +} + +void PairingManager::InitCommand() +{ + mCommissioner->RegisterPairingDelegate(this); + // Clear the CATs in OperationalCredentialsIssuer + mCredIssuerCmds->SetCredentialIssuerCATValues(kUndefinedCATs); + mDeviceIsICD = false; +} + +CHIP_ERROR PairingManager::PairDeviceWithCode(NodeId nodeId, const char * payload) +{ + InitCommand(); + + CommissioningParameters commissioningParams = GetCommissioningParameters(); + + auto discoveryType = DiscoveryType::kDiscoveryNetworkOnly; + + mNodeId = nodeId; + mOnboardingPayload = payload; + return mCommissioner->PairDevice(mNodeId, mOnboardingPayload, commissioningParams, discoveryType); +} + +CHIP_ERROR PairingManager::PairDevice(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, + uint16_t deviceRemotePort) +{ + InitCommand(); + mSetupPINCode = setupPINCode; + + Inet::IPAddress address; + Inet::InterfaceId interfaceId; + + if (!ParseAddressWithInterface(deviceRemoteIp, address, interfaceId)) + { + ChipLogError(NotSpecified, "Invalid IP address: %s", deviceRemoteIp); + return CHIP_ERROR_INVALID_ADDRESS; + } + + return Pair(nodeId, Transport::PeerAddress::UDP(address, deviceRemotePort, interfaceId)); +} + +CHIP_ERROR PairingManager::UnpairDevice(NodeId nodeId) +{ + InitCommand(); + + mCurrentFabricRemover = Platform::MakeUnique(mCommissioner); + return mCurrentFabricRemover->RemoveCurrentFabric(nodeId, &mCurrentFabricRemoveCallback); +} diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h index de1c0887f3b1b7..c3394562a6f7ba 100644 --- a/examples/fabric-admin/device_manager/PairingManager.h +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -18,8 +18,11 @@ #pragma once +#include #include +#include #include +#include #include class CommissioningWindowDelegate @@ -29,6 +32,20 @@ class CommissioningWindowDelegate virtual ~CommissioningWindowDelegate() = default; }; +class CommissioningDelegate +{ +public: + virtual void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR err) = 0; + virtual ~CommissioningDelegate() = default; +}; + +class PairingDelegate +{ +public: + virtual void OnDeviceRemoved(chip::NodeId deviceId, CHIP_ERROR err) = 0; + virtual ~PairingDelegate() = default; +}; + /** * The PairingManager class is responsible for managing the commissioning and pairing process * of Matter devices. PairingManager is designed to be used as a singleton, meaning that there @@ -49,7 +66,9 @@ class CommissioningWindowDelegate * manager.OpenCommissioningWindow(); * @endcode */ -class PairingManager +class PairingManager : public chip::Controller::DevicePairingDelegate, + public chip::Controller::DeviceDiscoveryDelegate, + public chip::Credentials::DeviceAttestationDelegate { public: static PairingManager & Instance() @@ -58,7 +77,14 @@ class PairingManager return instance; } - void Init(chip::Controller::DeviceCommissioner * commissioner); + CHIP_ERROR Init(chip::Controller::DeviceCommissioner * commissioner, CredentialIssuerCommands * credIssuerCmds); + + void SetOpenCommissioningWindowDelegate(CommissioningWindowDelegate * delegate) { mCommissioningWindowDelegate = delegate; } + void SetCommissioningDelegate(CommissioningDelegate * delegate) { mCommissioningDelegate = delegate; } + void SetPairingDelegate(PairingDelegate * delegate) { mPairingDelegate = delegate; } + PairingDelegate * GetPairingDelegate() { return mPairingDelegate; } + + chip::Controller::DeviceCommissioner & CurrentCommissioner() { return *mCommissioner; }; /** * Opens a commissioning window on the specified node and endpoint. @@ -79,7 +105,36 @@ class PairingManager uint32_t iterations, uint16_t discriminator, const chip::ByteSpan & salt, const chip::ByteSpan & verifier); - void SetOpenCommissioningWindowDelegate(CommissioningWindowDelegate * delegate) { mCommissioningWindowDelegate = delegate; } + /** + * Pairs a device using a setup code payload. + * + * @param nodeId The target node ID for pairing. + * @param payload The setup code payload, which typically contains device-specific pairing information. + * + * @return CHIP_NO_ERROR on successful initiation of the pairing process, or an appropriate CHIP_ERROR if pairing fails. + */ + CHIP_ERROR PairDeviceWithCode(chip::NodeId nodeId, const char * payload); + + /** + * Pairs a device using its setup PIN code and remote IP address. + * + * @param nodeId The target node ID for pairing. + * @param setupPINCode The setup PIN code for the device, used for establishing a secure connection. + * @param deviceRemoteIp The IP address of the remote device. + * @param deviceRemotePort The port number on which the device is listening for pairing requests. + * + * @return CHIP_NO_ERROR if the pairing process is initiated successfully, or an appropriate CHIP_ERROR if pairing fails. + */ + CHIP_ERROR PairDevice(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, uint16_t deviceRemotePort); + + /** + * Unpairs a device with the specified node ID. + * + * @param nodeId The node ID of the device to be unpaired. + * + * @return CHIP_NO_ERROR if the device is successfully unpaired, or an appropriate CHIP_ERROR if the process fails. + */ + CHIP_ERROR UnpairDevice(chip::NodeId nodeId); private: PairingManager(); @@ -87,6 +142,7 @@ class PairingManager PairingManager & operator=(const PairingManager &) = delete; chip::Controller::DeviceCommissioner * mCommissioner = nullptr; + CredentialIssuerCommands * mCredIssuerCmds; /////////// Open Commissioning Window Command Interface ///////// struct CommissioningWindowParams @@ -111,12 +167,61 @@ class PairingManager * The pointer is reset when the commissioning window is closed or when an error occurs. */ chip::Platform::UniquePtr mWindowOpener; + chip::Callback::Callback mOnOpenCommissioningWindowCallback; + chip::Callback::Callback mOnOpenCommissioningWindowVerifierCallback; static void OnOpenCommissioningWindow(intptr_t context); static void OnOpenCommissioningWindowResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status, chip::SetupPayload payload); static void OnOpenCommissioningWindowVerifierResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status); - chip::Callback::Callback mOnOpenCommissioningWindowCallback; - chip::Callback::Callback mOnOpenCommissioningWindowVerifierCallback; + /////////// DevicePairingDelegate Interface ///////// + void OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status) override; + void OnPairingComplete(CHIP_ERROR error) override; + void OnPairingDeleted(CHIP_ERROR error) override; + void OnReadCommissioningInfo(const chip::Controller::ReadCommissioningInfo & info) override; + void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR error) override; + void OnICDRegistrationComplete(chip::ScopedNodeId deviceId, uint32_t icdCounter) override; + void OnICDStayActiveComplete(chip::ScopedNodeId deviceId, uint32_t promisedActiveDuration) override; + + /////////// DeviceDiscoveryDelegate Interface ///////// + void OnDiscoveredDevice(const chip::Dnssd::CommissionNodeData & nodeData) override; + + /////////// DeviceAttestationDelegate Interface ///////// + chip::Optional FailSafeExpiryTimeoutSecs() const override; + bool ShouldWaitAfterDeviceAttestation() override; + void OnDeviceAttestationCompleted(chip::Controller::DeviceCommissioner * deviceCommissioner, chip::DeviceProxy * device, + const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, + chip::Credentials::AttestationVerificationResult attestationResult) override; + + /////////// Pairing Command Interface ///////// + CommissioningDelegate * mCommissioningDelegate = nullptr; + PairingDelegate * mPairingDelegate = nullptr; + + chip::NodeId mNodeId = chip::kUndefinedNodeId; + uint16_t mRemotePort = 0; + uint16_t mDiscriminator = 0; + uint32_t mSetupPINCode = 0; + const char * mOnboardingPayload = nullptr; + + // For ICD + bool mDeviceIsICD = false; + uint8_t mRandomGeneratedICDSymmetricKey[chip::Crypto::kAES_CCM128_Key_Length]; + + chip::Optional mICDRegistration; + chip::Optional mICDCheckInNodeId; + chip::Optional mICDClientType; + chip::Optional mICDSymmetricKey; + chip::Optional mICDMonitoredSubject; + chip::Optional mICDStayActiveDurationMsec; + + // For Unpair + chip::Platform::UniquePtr mCurrentFabricRemover; + chip::Callback::Callback mCurrentFabricRemoveCallback; + + chip::Controller::CommissioningParameters GetCommissioningParameters(); + void InitCommand(); + CHIP_ERROR Pair(chip::NodeId remoteId, chip::Transport::PeerAddress address); + + static void OnCurrentFabricRemove(void * context, chip::NodeId remoteNodeId, CHIP_ERROR status); }; From bd09308b5de8817ae54a7a09829184032f68e4b4 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 1 Oct 2024 22:34:44 +0000 Subject: [PATCH 02/10] Restyled by whitespace --- examples/fabric-admin/commands/common/CHIPCommand.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/fabric-admin/commands/common/CHIPCommand.cpp b/examples/fabric-admin/commands/common/CHIPCommand.cpp index 6b9e9162e0c861..a027598b3d9b36 100644 --- a/examples/fabric-admin/commands/common/CHIPCommand.cpp +++ b/examples/fabric-admin/commands/common/CHIPCommand.cpp @@ -183,7 +183,7 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack() allowTestCdSigningKey); ReturnLogErrorOnFailure(PairingManager::Instance().Init(&CurrentCommissioner(), mCredIssuerCmds)); - + return CHIP_NO_ERROR; } From 4272595e36c4ab1d5f1c32733579ed10d34e5362 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 1 Oct 2024 22:34:48 +0000 Subject: [PATCH 03/10] Restyled by clang-format --- examples/fabric-admin/commands/common/CHIPCommand.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/fabric-admin/commands/common/CHIPCommand.cpp b/examples/fabric-admin/commands/common/CHIPCommand.cpp index a027598b3d9b36..c23ea82f202cf7 100644 --- a/examples/fabric-admin/commands/common/CHIPCommand.cpp +++ b/examples/fabric-admin/commands/common/CHIPCommand.cpp @@ -184,7 +184,6 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack() ReturnLogErrorOnFailure(PairingManager::Instance().Init(&CurrentCommissioner(), mCredIssuerCmds)); - return CHIP_NO_ERROR; } From 2f1c8f7ef6f7af7a456ca042a41ee73587e32676 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 2 Oct 2024 00:26:51 -0700 Subject: [PATCH 04/10] Schedule work on the Matter thread --- .../commands/fabric-sync/FabricSyncCommand.h | 1 - .../device_manager/PairingManager.cpp | 95 +++++++++++++++---- .../device_manager/PairingManager.h | 23 ++++- 3 files changed, 100 insertions(+), 19 deletions(-) diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h index b6c4fd5a2f7cb7..16ee40e5b40f3e 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h @@ -23,7 +23,6 @@ // Constants constexpr uint32_t kCommissionPrepareTimeMs = 500; -constexpr uint16_t kMaxManualCodeLength = 21; class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDelegate { diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 3be046a4c5f9ed..29854a76ef151e 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -564,7 +564,7 @@ void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E } } -void PairingManager::InitCommand() +void PairingManager::InitPairingCommand() { mCommissioner->RegisterPairingDelegate(this); // Clear the CATs in OperationalCredentialsIssuer @@ -574,39 +574,102 @@ void PairingManager::InitCommand() CHIP_ERROR PairingManager::PairDeviceWithCode(NodeId nodeId, const char * payload) { - InitCommand(); + if (payload == nullptr || strlen(payload) > kMaxManualCodeLength) + { + ChipLogError(NotSpecified, "PairDeviceWithCode failed: Invalid pairing payload"); + return CHIP_ERROR_INVALID_STRING_LENGTH; + } - CommissioningParameters commissioningParams = GetCommissioningParameters(); + auto params = Platform::MakeUnique(); + params->nodeId = nodeId; - auto discoveryType = DiscoveryType::kDiscoveryNetworkOnly; + Platform::CopyString(params->payloadBuffer, kMaxManualCodeLength, payload); - mNodeId = nodeId; - mOnboardingPayload = payload; - return mCommissioner->PairDevice(mNodeId, mOnboardingPayload, commissioningParams, discoveryType); + // Schedule work on the Matter thread + return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDeviceWithCode, reinterpret_cast(params.release())); +} + +void PairingManager::OnPairDeviceWithCode(intptr_t context) +{ + Platform::UniquePtr params(reinterpret_cast(context)); + PairingManager & self = PairingManager::Instance(); + + self.InitPairingCommand(); + + CommissioningParameters commissioningParams = self.GetCommissioningParameters(); + auto discoveryType = DiscoveryType::kDiscoveryNetworkOnly; + + self.mNodeId = params->nodeId; + self.mOnboardingPayload = params->payloadBuffer; + + CHIP_ERROR err = self.mCommissioner->PairDevice(params->nodeId, params->payloadBuffer, commissioningParams, discoveryType); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to pair device with code, error: %s", ErrorStr(err)); + } } CHIP_ERROR PairingManager::PairDevice(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, uint16_t deviceRemotePort) { - InitCommand(); - mSetupPINCode = setupPINCode; + if (deviceRemoteIp == nullptr || strlen(deviceRemoteIp) > Inet::IPAddress::kMaxStringLength) + { + ChipLogError(NotSpecified, "PairDevice failed: Invalid device remote IP address"); + return CHIP_ERROR_INVALID_STRING_LENGTH; + } + + auto params = Platform::MakeUnique(); + params->nodeId = nodeId; + params->setupPINCode = setupPINCode; + params->deviceRemotePort = deviceRemotePort; + + Platform::CopyString(params->ipAddrBuffer, Inet::IPAddress::kMaxStringLength, deviceRemoteIp); + + // Schedule work on the Matter thread + return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDevice, reinterpret_cast(params.release())); +} + +void PairingManager::OnPairDevice(intptr_t context) +{ + Platform::UniquePtr params(reinterpret_cast(context)); + PairingManager & self = PairingManager::Instance(); + + self.InitPairingCommand(); + self.mSetupPINCode = params->setupPINCode; Inet::IPAddress address; Inet::InterfaceId interfaceId; - if (!ParseAddressWithInterface(deviceRemoteIp, address, interfaceId)) + if (!ParseAddressWithInterface(params->ipAddrBuffer, address, interfaceId)) { - ChipLogError(NotSpecified, "Invalid IP address: %s", deviceRemoteIp); - return CHIP_ERROR_INVALID_ADDRESS; + ChipLogError(NotSpecified, "Invalid IP address: %s", params->ipAddrBuffer); + return; } - return Pair(nodeId, Transport::PeerAddress::UDP(address, deviceRemotePort, interfaceId)); + CHIP_ERROR err = self.Pair(params->nodeId, Transport::PeerAddress::UDP(address, params->deviceRemotePort, interfaceId)); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to pair device, error: %s", ErrorStr(err)); + } } CHIP_ERROR PairingManager::UnpairDevice(NodeId nodeId) { - InitCommand(); + // Schedule work on the Matter thread + return DeviceLayer::PlatformMgr().ScheduleWork(OnUnpairDevice, static_cast(nodeId)); +} + +void PairingManager::OnUnpairDevice(intptr_t context) +{ + NodeId nodeId = static_cast(context); + PairingManager & self = PairingManager::Instance(); - mCurrentFabricRemover = Platform::MakeUnique(mCommissioner); - return mCurrentFabricRemover->RemoveCurrentFabric(nodeId, &mCurrentFabricRemoveCallback); + self.InitPairingCommand(); + + self.mCurrentFabricRemover = Platform::MakeUnique(self.mCommissioner); + CHIP_ERROR err = self.mCurrentFabricRemover->RemoveCurrentFabric(nodeId, &self.mCurrentFabricRemoveCallback); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to unpair device, error: %s", ErrorStr(err)); + } } diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h index c3394562a6f7ba..6023c8e0080aa7 100644 --- a/examples/fabric-admin/device_manager/PairingManager.h +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -25,6 +25,9 @@ #include #include +// Constants +constexpr uint16_t kMaxManualCodeLength = 21; + class CommissioningWindowDelegate { public: @@ -195,11 +198,24 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, chip::Credentials::AttestationVerificationResult attestationResult) override; /////////// Pairing Command Interface ///////// + struct PairDeviceWithCodeParams + { + chip::NodeId nodeId; + char payloadBuffer[kMaxManualCodeLength + 1]; + }; + + struct PairDeviceParams + { + chip::NodeId nodeId; + uint32_t setupPINCode; + uint16_t deviceRemotePort; + char ipAddrBuffer[chip::Inet::IPAddress::kMaxStringLength]; + }; + CommissioningDelegate * mCommissioningDelegate = nullptr; PairingDelegate * mPairingDelegate = nullptr; chip::NodeId mNodeId = chip::kUndefinedNodeId; - uint16_t mRemotePort = 0; uint16_t mDiscriminator = 0; uint32_t mSetupPINCode = 0; const char * mOnboardingPayload = nullptr; @@ -220,8 +236,11 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, chip::Callback::Callback mCurrentFabricRemoveCallback; chip::Controller::CommissioningParameters GetCommissioningParameters(); - void InitCommand(); + void InitPairingCommand(); CHIP_ERROR Pair(chip::NodeId remoteId, chip::Transport::PeerAddress address); static void OnCurrentFabricRemove(void * context, chip::NodeId remoteNodeId, CHIP_ERROR status); + static void OnPairDeviceWithCode(intptr_t context); + static void OnPairDevice(intptr_t context); + static void OnUnpairDevice(intptr_t context); }; From 5b39a9547c2085410332e46191b7abaf35661f2e Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 3 Oct 2024 09:32:41 -0700 Subject: [PATCH 05/10] Update examples/fabric-admin/device_manager/PairingManager.cpp Co-authored-by: Terence Hampson --- examples/fabric-admin/device_manager/PairingManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 29854a76ef151e..090dc0216fb568 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -106,9 +106,9 @@ PairingManager::PairingManager() : CHIP_ERROR PairingManager::Init(Controller::DeviceCommissioner * commissioner, CredentialIssuerCommands * credIssuerCmds) { VerifyOrReturnError(commissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); - mCommissioner = commissioner; - VerifyOrReturnError(credIssuerCmds != nullptr, CHIP_ERROR_INCORRECT_STATE); + + mCommissioner = commissioner; mCredIssuerCmds = credIssuerCmds; return CHIP_NO_ERROR; From a30749722be8711d8e48401acb5a337fd25af4ee Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 3 Oct 2024 09:34:48 -0700 Subject: [PATCH 06/10] Update examples/fabric-admin/device_manager/PairingManager.cpp Co-authored-by: Terence Hampson --- examples/fabric-admin/device_manager/PairingManager.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 090dc0216fb568..0bdfc641d2e5f5 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -274,9 +274,6 @@ void PairingManager::OnPairingComplete(CHIP_ERROR err) ChipLogProgress(NotSpecified, "Pairing Failure: %s", ErrorStr(err)); } - if (err != CHIP_NO_ERROR) - { - } } void PairingManager::OnPairingDeleted(CHIP_ERROR err) From 1e454d2986ab2ee108b1e7c90cc5e2da1631a23e Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 3 Oct 2024 09:35:20 -0700 Subject: [PATCH 07/10] Update examples/fabric-admin/device_manager/PairingManager.cpp Co-authored-by: Terence Hampson --- examples/fabric-admin/device_manager/PairingManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 0bdfc641d2e5f5..f5d90ec18b5b9a 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -316,7 +316,7 @@ void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) if (mCommissioningDelegate) { mCommissioningDelegate->OnCommissioningComplete(nodeId, err); - this->SetCommissioningDelegate(nullptr); + SetCommissioningDelegate(nullptr); } } From e5522ee654192d6494dc160aa2b15685b74549f7 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 3 Oct 2024 09:35:40 -0700 Subject: [PATCH 08/10] Update examples/fabric-admin/device_manager/PairingManager.cpp Co-authored-by: Terence Hampson --- examples/fabric-admin/device_manager/PairingManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index f5d90ec18b5b9a..64475c652b8552 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -326,7 +326,7 @@ void PairingManager::OnReadCommissioningInfo(const Controller::ReadCommissioning info.basic.productId); // The string in CharSpan received from the device is not null-terminated, we use std::string here for coping and - // appending a numm-terminator at the end of the string. + // appending a null-terminator at the end of the string. std::string userActiveModeTriggerInstruction; // Note: the callback doesn't own the buffer, should make a copy if it will be used it later. From 78aac89b869ffa4d9137b427202a16efbe1076be Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 3 Oct 2024 10:58:47 -0700 Subject: [PATCH 09/10] Address review comments --- .../device_manager/PairingManager.cpp | 58 ++++++---- .../device_manager/PairingManager.h | 100 +++++++++--------- 2 files changed, 89 insertions(+), 69 deletions(-) diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 64475c652b8552..c46a0ca000897b 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -108,7 +108,7 @@ CHIP_ERROR PairingManager::Init(Controller::DeviceCommissioner * commissioner, C VerifyOrReturnError(commissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(credIssuerCmds != nullptr, CHIP_ERROR_INCORRECT_STATE); - mCommissioner = commissioner; + mCommissioner = commissioner; mCredIssuerCmds = credIssuerCmds; return CHIP_NO_ERROR; @@ -253,7 +253,6 @@ void PairingManager::OnStatusUpdate(DevicePairingDelegate::Status status) switch (status) { case DevicePairingDelegate::Status::SecurePairingSuccess: - ChipLogProgress(NotSpecified, "Secure Pairing Success"); ChipLogProgress(NotSpecified, "CASE establishment successful"); break; case DevicePairingDelegate::Status::SecurePairingFailed: @@ -266,14 +265,12 @@ void PairingManager::OnPairingComplete(CHIP_ERROR err) { if (err == CHIP_NO_ERROR) { - ChipLogProgress(NotSpecified, "Pairing Success"); ChipLogProgress(NotSpecified, "PASE establishment successful"); } else { ChipLogProgress(NotSpecified, "Pairing Failure: %s", ErrorStr(err)); } - } void PairingManager::OnPairingDeleted(CHIP_ERROR err) @@ -293,8 +290,9 @@ void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) if (err == CHIP_NO_ERROR) { // print to console - fprintf(stderr, "New device with Node ID: 0x%lx has been successfully added.\n", nodeId); - // CurrentCommissioner() has a lifetime that is the entire life of the application itself + fprintf(stderr, "New device with Node ID: " ChipLogFormatX64 "has been successfully added.\n", ChipLogValueX64(nodeId)); + + // mCommissioner has a lifetime that is the entire life of the application itself // so it is safe to provide to StartDeviceSynchronization. DeviceSynchronizer::Instance().StartDeviceSynchronization(mCommissioner, nodeId, mDeviceIsICD); } @@ -418,6 +416,7 @@ void PairingManager::OnDiscoveredDevice(const Dnssd::CommissionNodeData & nodeDa err = Pair(mNodeId, peerAddress); if (CHIP_NO_ERROR != err) { + ChipLogProgress(NotSpecified, "Failed to pair device: " ChipLogFormatX64 " %s", ChipLogValueX64(mNodeId), ErrorStr(err)); } } @@ -473,17 +472,19 @@ void PairingManager::OnDeviceAttestationCompleted(Controller::DeviceCommissioner } // NOTE: This will log errors even if the attestion was successful. - auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); + CHIP_ERROR err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); if (CHIP_NO_ERROR != err) { + ChipLogError(NotSpecified, "Failed to continue commissioning after device attestation, error: %s", ErrorStr(err)); } return; } // Don't bypass attestation, continue with error. - auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); + CHIP_ERROR err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); if (CHIP_NO_ERROR != err) { + ChipLogError(NotSpecified, "Failed to continue commissioning after device attestation, error: %s", ErrorStr(err)); } } @@ -548,11 +549,13 @@ void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E if (err == CHIP_NO_ERROR) { // print to console - fprintf(stderr, "Device with Node ID: 0x%lx has been successfully removed.\n", nodeId); + fprintf(stderr, "Device with Node ID: " ChipLogFormatX64 "has been successfully removed.\n", ChipLogValueX64(nodeId)); #if defined(PW_RPC_ENABLED) - app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(self->CurrentCommissioner().GetFabricIndex(), nodeId); - RemoveSynchronizedDevice(nodeId); + FabricIndex fabricIndex = self->CurrentCommissioner().GetFabricIndex(); + app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(fabricIndex, nodeId); + ScopedNodeId scopedNodeId(nodeId, fabricIndex); + RemoveSynchronizedDevice(scopedNodeId); #endif } else @@ -571,16 +574,17 @@ void PairingManager::InitPairingCommand() CHIP_ERROR PairingManager::PairDeviceWithCode(NodeId nodeId, const char * payload) { - if (payload == nullptr || strlen(payload) > kMaxManualCodeLength) + if (payload == nullptr || strlen(payload) > kMaxManualCodeLength + 1) { ChipLogError(NotSpecified, "PairDeviceWithCode failed: Invalid pairing payload"); return CHIP_ERROR_INVALID_STRING_LENGTH; } - auto params = Platform::MakeUnique(); - params->nodeId = nodeId; + auto params = Platform::MakeUnique(); + VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY); - Platform::CopyString(params->payloadBuffer, kMaxManualCodeLength, payload); + params->nodeId = nodeId; + Platform::CopyString(params->payloadBuffer, sizeof(params->payloadBuffer), payload); // Schedule work on the Matter thread return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDeviceWithCode, reinterpret_cast(params.release())); @@ -615,12 +619,14 @@ CHIP_ERROR PairingManager::PairDevice(chip::NodeId nodeId, uint32_t setupPINCode return CHIP_ERROR_INVALID_STRING_LENGTH; } - auto params = Platform::MakeUnique(); + auto params = Platform::MakeUnique(); + VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY); + params->nodeId = nodeId; params->setupPINCode = setupPINCode; params->deviceRemotePort = deviceRemotePort; - Platform::CopyString(params->ipAddrBuffer, Inet::IPAddress::kMaxStringLength, deviceRemoteIp); + Platform::CopyString(params->ipAddrBuffer, sizeof(params->ipAddrBuffer), deviceRemoteIp); // Schedule work on the Matter thread return DeviceLayer::PlatformMgr().ScheduleWork(OnPairDevice, reinterpret_cast(params.release())); @@ -652,19 +658,31 @@ void PairingManager::OnPairDevice(intptr_t context) CHIP_ERROR PairingManager::UnpairDevice(NodeId nodeId) { + auto params = Platform::MakeUnique(); + VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY); + + params->nodeId = nodeId; + // Schedule work on the Matter thread - return DeviceLayer::PlatformMgr().ScheduleWork(OnUnpairDevice, static_cast(nodeId)); + return DeviceLayer::PlatformMgr().ScheduleWork(OnUnpairDevice, reinterpret_cast(params.release())); } void PairingManager::OnUnpairDevice(intptr_t context) { - NodeId nodeId = static_cast(context); + Platform::UniquePtr params(reinterpret_cast(context)); PairingManager & self = PairingManager::Instance(); self.InitPairingCommand(); self.mCurrentFabricRemover = Platform::MakeUnique(self.mCommissioner); - CHIP_ERROR err = self.mCurrentFabricRemover->RemoveCurrentFabric(nodeId, &self.mCurrentFabricRemoveCallback); + + if (!self.mCurrentFabricRemover) + { + ChipLogError(NotSpecified, "Failed to unpair device, mCurrentFabricRemover is null"); + return; + } + + CHIP_ERROR err = self.mCurrentFabricRemover->RemoveCurrentFabric(params->nodeId, &self.mCurrentFabricRemoveCallback); if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "Failed to unpair device, error: %s", ErrorStr(err)); diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h index 6023c8e0080aa7..563d129079d3f1 100644 --- a/examples/fabric-admin/device_manager/PairingManager.h +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -26,7 +26,7 @@ #include // Constants -constexpr uint16_t kMaxManualCodeLength = 21; +constexpr uint16_t kMaxManualCodeLength = 22; class CommissioningWindowDelegate { @@ -140,14 +140,6 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, CHIP_ERROR UnpairDevice(chip::NodeId nodeId); private: - PairingManager(); - PairingManager(const PairingManager &) = delete; - PairingManager & operator=(const PairingManager &) = delete; - - chip::Controller::DeviceCommissioner * mCommissioner = nullptr; - CredentialIssuerCommands * mCredIssuerCmds; - - /////////// Open Commissioning Window Command Interface ///////// struct CommissioningWindowParams { chip::NodeId nodeId; @@ -162,21 +154,34 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, chip::ByteSpan salt; }; - CommissioningWindowDelegate * mCommissioningWindowDelegate = nullptr; + struct PairDeviceWithCodeParams + { + chip::NodeId nodeId; + char payloadBuffer[kMaxManualCodeLength + 1]; + }; - /** - * Holds the unique_ptr to the current CommissioningWindowOpener. - * Only one commissioning window opener can be active at a time. - * The pointer is reset when the commissioning window is closed or when an error occurs. - */ - chip::Platform::UniquePtr mWindowOpener; - chip::Callback::Callback mOnOpenCommissioningWindowCallback; - chip::Callback::Callback mOnOpenCommissioningWindowVerifierCallback; + struct PairDeviceParams + { + chip::NodeId nodeId; + uint32_t setupPINCode; + uint16_t deviceRemotePort; + char ipAddrBuffer[chip::Inet::IPAddress::kMaxStringLength]; + }; - static void OnOpenCommissioningWindow(intptr_t context); - static void OnOpenCommissioningWindowResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status, - chip::SetupPayload payload); - static void OnOpenCommissioningWindowVerifierResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status); + struct UnpairDeviceParams + { + chip::NodeId nodeId; + }; + + // Constructors + PairingManager(); + PairingManager(const PairingManager &) = delete; + PairingManager & operator=(const PairingManager &) = delete; + + // Private member functions (static and non-static) + chip::Controller::CommissioningParameters GetCommissioningParameters(); + void InitPairingCommand(); + CHIP_ERROR Pair(chip::NodeId remoteId, chip::Transport::PeerAddress address); /////////// DevicePairingDelegate Interface ///////// void OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status) override; @@ -197,31 +202,28 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, chip::Credentials::AttestationVerificationResult attestationResult) override; - /////////// Pairing Command Interface ///////// - struct PairDeviceWithCodeParams - { - chip::NodeId nodeId; - char payloadBuffer[kMaxManualCodeLength + 1]; - }; + static void OnOpenCommissioningWindow(intptr_t context); + static void OnOpenCommissioningWindowResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status, + chip::SetupPayload payload); + static void OnOpenCommissioningWindowVerifierResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status); + static void OnCurrentFabricRemove(void * context, chip::NodeId remoteNodeId, CHIP_ERROR status); + static void OnPairDeviceWithCode(intptr_t context); + static void OnPairDevice(intptr_t context); + static void OnUnpairDevice(intptr_t context); - struct PairDeviceParams - { - chip::NodeId nodeId; - uint32_t setupPINCode; - uint16_t deviceRemotePort; - char ipAddrBuffer[chip::Inet::IPAddress::kMaxStringLength]; - }; + // Private data members + chip::Controller::DeviceCommissioner * mCommissioner = nullptr; + CredentialIssuerCommands * mCredIssuerCmds = nullptr; - CommissioningDelegate * mCommissioningDelegate = nullptr; - PairingDelegate * mPairingDelegate = nullptr; + CommissioningWindowDelegate * mCommissioningWindowDelegate = nullptr; + CommissioningDelegate * mCommissioningDelegate = nullptr; + PairingDelegate * mPairingDelegate = nullptr; chip::NodeId mNodeId = chip::kUndefinedNodeId; uint16_t mDiscriminator = 0; uint32_t mSetupPINCode = 0; const char * mOnboardingPayload = nullptr; - - // For ICD - bool mDeviceIsICD = false; + bool mDeviceIsICD = false; uint8_t mRandomGeneratedICDSymmetricKey[chip::Crypto::kAES_CCM128_Key_Length]; chip::Optional mICDRegistration; @@ -231,16 +233,16 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, chip::Optional mICDMonitoredSubject; chip::Optional mICDStayActiveDurationMsec; + /** + * Holds the unique_ptr to the current CommissioningWindowOpener. + * Only one commissioning window opener can be active at a time. + * The pointer is reset when the commissioning window is closed or when an error occurs. + */ + chip::Platform::UniquePtr mWindowOpener; + chip::Callback::Callback mOnOpenCommissioningWindowCallback; + chip::Callback::Callback mOnOpenCommissioningWindowVerifierCallback; + // For Unpair chip::Platform::UniquePtr mCurrentFabricRemover; chip::Callback::Callback mCurrentFabricRemoveCallback; - - chip::Controller::CommissioningParameters GetCommissioningParameters(); - void InitPairingCommand(); - CHIP_ERROR Pair(chip::NodeId remoteId, chip::Transport::PeerAddress address); - - static void OnCurrentFabricRemove(void * context, chip::NodeId remoteNodeId, CHIP_ERROR status); - static void OnPairDeviceWithCode(intptr_t context); - static void OnPairDevice(intptr_t context); - static void OnUnpairDevice(intptr_t context); }; From 6575f83dd24e7ca3bcf4b5cd8cb03f20d720370b Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 3 Oct 2024 15:10:10 -0700 Subject: [PATCH 10/10] Cleanup CCTRL attestation logic from PairingCommand --- .../commands/pairing/PairingCommand.cpp | 98 ++----------------- .../commands/pairing/PairingCommand.h | 1 - 2 files changed, 7 insertions(+), 92 deletions(-) diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.cpp b/examples/fabric-admin/commands/pairing/PairingCommand.cpp index 28c22afcd2364a..37578472b70fbd 100644 --- a/examples/fabric-admin/commands/pairing/PairingCommand.cpp +++ b/examples/fabric-admin/commands/pairing/PairingCommand.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include @@ -41,28 +40,6 @@ using namespace ::chip; using namespace ::chip::Controller; -namespace { - -CHIP_ERROR GetPayload(const char * setUpCode, SetupPayload & payload) -{ - VerifyOrReturnValue(setUpCode, CHIP_ERROR_INVALID_ARGUMENT); - bool isQRCode = strncmp(setUpCode, kQRCodePrefix, strlen(kQRCodePrefix)) == 0; - if (isQRCode) - { - ReturnErrorOnFailure(QRCodeSetupPayloadParser(setUpCode).populatePayload(payload)); - VerifyOrReturnError(payload.isValidQRCodePayload(), CHIP_ERROR_INVALID_ARGUMENT); - } - else - { - ReturnErrorOnFailure(ManualSetupPayloadParser(setUpCode).populatePayload(payload)); - VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT); - } - - return CHIP_NO_ERROR; -} - -} // namespace - CHIP_ERROR PairingCommand::RunCommand() { CurrentCommissioner().RegisterPairingDelegate(this); @@ -128,7 +105,10 @@ CommissioningParameters PairingCommand::GetCommissioningParameters() { auto params = CommissioningParameters(); params.SetSkipCommissioningComplete(mSkipCommissioningComplete.ValueOr(false)); - params.SetDeviceAttestationDelegate(this); + if (mBypassAttestationVerifier.ValueOr(false)) + { + params.SetDeviceAttestationDelegate(this); + } switch (mNetworkType) { @@ -579,77 +559,13 @@ Optional PairingCommand::FailSafeExpiryTimeoutSecs() const return Optional(); } -bool PairingCommand::ShouldWaitAfterDeviceAttestation() -{ - // If there is a vendor ID and product ID, request OnDeviceAttestationCompleted(). - // Currently this is added in the case that the example is performing reverse commissioning, - // but it would be an improvement to store that explicitly. - // TODO: Issue #35297 - [Fabric Sync] Improve where we get VID and PID when validating CCTRL CommissionNode command - SetupPayload payload; - CHIP_ERROR err = GetPayload(mOnboardingPayload, payload); - return err == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0); -} - void PairingCommand::OnDeviceAttestationCompleted(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device, const Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, Credentials::AttestationVerificationResult attestationResult) { - SetupPayload payload; - CHIP_ERROR parse_error = GetPayload(mOnboardingPayload, payload); - if (parse_error == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0)) - { - if (payload.vendorID == 0 || payload.productID == 0) - { - ChipLogProgress(NotSpecified, - "Failed validation: vendorID or productID must not be 0." - "Requested VID: %u, Requested PID: %u.", - payload.vendorID, payload.productID); - deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( - device, Credentials::AttestationVerificationResult::kInvalidArgument); - return; - } - - if (payload.vendorID != info.BasicInformationVendorId() || payload.productID != info.BasicInformationProductId()) - { - ChipLogProgress(NotSpecified, - "Failed validation of vendorID or productID." - "Requested VID: %u, Requested PID: %u," - "Detected VID: %u, Detected PID %u.", - payload.vendorID, payload.productID, info.BasicInformationVendorId(), info.BasicInformationProductId()); - deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( - device, - payload.vendorID == info.BasicInformationVendorId() - ? Credentials::AttestationVerificationResult::kDacProductIdMismatch - : Credentials::AttestationVerificationResult::kDacVendorIdMismatch); - return; - } - - // NOTE: This will log errors even if the attestion was successful. - auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); - if (CHIP_NO_ERROR != err) - { - SetCommandExitStatus(err); - } - return; - } - - // OnDeviceAttestationCompleted() is called if ShouldWaitAfterDeviceAttestation() returns true - // or if there is an attestation error. The conditions for ShouldWaitAfterDeviceAttestation() have - // already been checked, so the call to OnDeviceAttestationCompleted() was an error. - if (mBypassAttestationVerifier.ValueOr(false)) - { - // Bypass attestation verification, continue with success - auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( - device, Credentials::AttestationVerificationResult::kSuccess); - if (CHIP_NO_ERROR != err) - { - SetCommandExitStatus(err); - } - return; - } - - // Don't bypass attestation, continue with error. - auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); + // Bypass attestation verification, continue with success + auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( + device, Credentials::AttestationVerificationResult::kSuccess); if (CHIP_NO_ERROR != err) { SetCommandExitStatus(err); diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.h b/examples/fabric-admin/commands/pairing/PairingCommand.h index 3c857ce89f0b32..aafd8888dc61a3 100644 --- a/examples/fabric-admin/commands/pairing/PairingCommand.h +++ b/examples/fabric-admin/commands/pairing/PairingCommand.h @@ -207,7 +207,6 @@ class PairingCommand : public CHIPCommand, /////////// DeviceAttestationDelegate Interface ///////// chip::Optional FailSafeExpiryTimeoutSecs() const override; - bool ShouldWaitAfterDeviceAttestation() override; void OnDeviceAttestationCompleted(chip::Controller::DeviceCommissioner * deviceCommissioner, chip::DeviceProxy * device, const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, chip::Credentials::AttestationVerificationResult attestationResult) override;