From a6e081ab54724a954a0296478744889d43e4cfa9 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Mon, 16 Sep 2024 21:34:03 -0700 Subject: [PATCH 1/5] [Fabric-Admin] Refactor to use API methods instead of PushCommand to talk to SDK --- examples/fabric-admin/BUILD.gn | 2 + .../commands/common/CHIPCommand.cpp | 3 + .../fabric-sync/FabricSyncCommand.cpp | 10 +- .../commands/fabric-sync/FabricSyncCommand.h | 2 +- .../OpenCommissioningWindowCommand.cpp | 8 - .../pairing/OpenCommissioningWindowCommand.h | 11 -- .../device_manager/DeviceManager.cpp | 60 ++++--- .../device_manager/DeviceManager.h | 6 +- .../device_manager/PairingManager.cpp | 147 ++++++++++++++++++ .../device_manager/PairingManager.h | 87 +++++++++++ examples/fabric-admin/rpc/RpcServer.cpp | 19 ++- 11 files changed, 282 insertions(+), 73 deletions(-) create mode 100644 examples/fabric-admin/device_manager/PairingManager.cpp create mode 100644 examples/fabric-admin/device_manager/PairingManager.h diff --git a/examples/fabric-admin/BUILD.gn b/examples/fabric-admin/BUILD.gn index d1add205f8826c..ab584459582657 100644 --- a/examples/fabric-admin/BUILD.gn +++ b/examples/fabric-admin/BUILD.gn @@ -88,6 +88,8 @@ static_library("fabric-admin-utils") { "device_manager/DeviceSubscriptionManager.h", "device_manager/DeviceSynchronization.cpp", "device_manager/DeviceSynchronization.h", + "device_manager/PairingManager.cpp", + "device_manager/PairingManager.h", "device_manager/UniqueIdGetter.cpp", "device_manager/UniqueIdGetter.h", ] diff --git a/examples/fabric-admin/commands/common/CHIPCommand.cpp b/examples/fabric-admin/commands/common/CHIPCommand.cpp index 0c5455439b22d9..4c955f846882da 100644 --- a/examples/fabric-admin/commands/common/CHIPCommand.cpp +++ b/examples/fabric-admin/commands/common/CHIPCommand.cpp @@ -21,6 +21,7 @@ #include "IcdManager.h" #include #include +#include #include #include #include @@ -181,6 +182,8 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack() mCredIssuerCmds->SetCredentialIssuerOption(CredentialIssuerCommands::CredentialIssuerOptions::kAllowTestCdSigningKey, allowTestCdSigningKey); + ReturnLogErrorOnFailure(PairingManager::Instance().Init(&CurrentCommissioner())); + return CHIP_NO_ERROR; } diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp index 85a3aada9f88ae..9a07d3b2e368eb 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp @@ -344,15 +344,7 @@ CHIP_ERROR FabricSyncDeviceCommand::RunCommand(EndpointId remoteId) return CHIP_NO_ERROR; } - OpenCommissioningWindowCommand * openCommand = - static_cast(CommandMgr().GetCommandByName("pairing", "open-commissioning-window")); - - if (openCommand == nullptr) - { - return CHIP_ERROR_NOT_IMPLEMENTED; - } - - openCommand->RegisterDelegate(this); + PairingManager::Instance().RegisterOpenCommissioningWindowDelegate(this); DeviceMgr().OpenRemoteDeviceCommissioningWindow(remoteId); diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h index 669edd75d653e6..e80f10133853f7 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h @@ -19,8 +19,8 @@ #pragma once #include -#include #include +#include // Constants constexpr uint32_t kCommissionPrepareTimeMs = 500; diff --git a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp index 53073160108036..cfe212fda00131 100644 --- a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp +++ b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp @@ -70,14 +70,6 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand() void OpenCommissioningWindowCommand::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err, chip::SetupPayload payload) { - OpenCommissioningWindowCommand * self = static_cast(context); - if (self->mDelegate) - { - self->mDelegate->OnCommissioningWindowOpened(remoteId, err, payload); - self->UnregisterDelegate(); - } - - LogErrorOnFailure(err); OnOpenBasicCommissioningWindowResponse(context, remoteId, err); } diff --git a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h index 7edcdba7115665..a8760a464827a9 100644 --- a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h +++ b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h @@ -22,13 +22,6 @@ #include #include -class CommissioningWindowDelegate -{ -public: - virtual void OnCommissioningWindowOpened(chip::NodeId deviceId, CHIP_ERROR err, chip::SetupPayload payload) = 0; - virtual ~CommissioningWindowDelegate() = default; -}; - class OpenCommissioningWindowCommand : public CHIPCommand { public: @@ -57,9 +50,6 @@ class OpenCommissioningWindowCommand : public CHIPCommand "params if absent"); } - void RegisterDelegate(CommissioningWindowDelegate * delegate) { mDelegate = delegate; } - void UnregisterDelegate() { mDelegate = nullptr; } - /////////// CHIPCommand Interface ///////// CHIP_ERROR RunCommand() override; @@ -71,7 +61,6 @@ class OpenCommissioningWindowCommand : public CHIPCommand NodeId mNodeId; chip::EndpointId mEndpointId; chip::Controller::CommissioningWindowOpener::CommissioningWindowOption mCommissioningWindowOption; - CommissioningWindowDelegate * mDelegate = nullptr; uint16_t mCommissioningWindowTimeout; uint32_t mIteration; uint16_t mDiscriminator; diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index ae8fa507ceaaa4..c04d5d6291f78e 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -30,13 +31,12 @@ using namespace chip::app::Clusters; namespace { -constexpr uint16_t kWindowTimeout = 300; -constexpr uint16_t kIteration = 1000; -constexpr uint16_t kSubscribeMinInterval = 0; -constexpr uint16_t kSubscribeMaxInterval = 60; -constexpr uint16_t kAggragatorEndpointId = 1; -constexpr uint16_t kMaxDiscriminatorLength = 4095; -constexpr uint8_t kEnhancedCommissioningMethod = 1; +constexpr uint16_t kWindowTimeout = 300; +constexpr uint16_t kIteration = 1000; +constexpr uint16_t kSubscribeMinInterval = 0; +constexpr uint16_t kSubscribeMaxInterval = 60; +constexpr uint16_t kAggragatorEndpointId = 1; +constexpr uint16_t kMaxDiscriminatorLength = 4095; } // namespace @@ -116,18 +116,17 @@ void DeviceManager::RemoveSyncedDevice(NodeId nodeId) } void DeviceManager::OpenDeviceCommissioningWindow(NodeId nodeId, uint32_t commissioningTimeout, uint32_t iterations, - uint32_t discriminator, const char * saltHex, const char * verifierHex) + uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier) { - ChipLogProgress(NotSpecified, "Open the commissioning window of device with NodeId:" ChipLogFormatX64, ChipLogValueX64(nodeId)); + ChipLogProgress(NotSpecified, "Opening commissioning window for Node ID: " ChipLogFormatX64, ChipLogValueX64(nodeId)); // Open the commissioning window of a device within its own fabric. - StringBuilder commandBuilder; - - commandBuilder.Add("pairing open-commissioning-window "); - commandBuilder.AddFormat("%lu %d %d %d %d %d --salt hex:%s --verifier hex:%s", nodeId, kRootEndpointId, - kEnhancedCommissioningMethod, commissioningTimeout, iterations, discriminator, saltHex, verifierHex); - - PushCommand(commandBuilder.c_str()); + CHIP_ERROR err = PairingManager::Instance().OpenCommissioningWindow(nodeId, kRootEndpointId, commissioningTimeout, iterations, + discriminator, salt, verifier); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to open commissioning window: %s", ErrorStr(err)); + } } void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpointId) @@ -141,11 +140,15 @@ void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpoin uint16_t discriminator = Crypto::GetRandU16() % (kMaxDiscriminatorLength + 1); // Include the upper limit kMaxDiscriminatorLength - commandBuilder.Add("pairing open-commissioning-window "); - commandBuilder.AddFormat("%lu %d %d %d %d %d", mRemoteBridgeNodeId, remoteEndpointId, kEnhancedCommissioningMethod, - kWindowTimeout, kIteration, discriminator); + ByteSpan emptySalt; + ByteSpan emptyVerifier; - PushCommand(commandBuilder.c_str()); + CHIP_ERROR err = PairingManager::Instance().OpenCommissioningWindow(mRemoteBridgeNodeId, remoteEndpointId, kWindowTimeout, + kIteration, discriminator, emptySalt, emptyVerifier); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to open commissioning window: %s", ErrorStr(err)); + } } void DeviceManager::PairRemoteFabricBridge(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, @@ -421,6 +424,7 @@ void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data) { CommissionerControl::Commands::ReverseOpenCommissioningWindow::DecodableType value; CHIP_ERROR error = app::DataModel::Decode(data, value); + if (error != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "Failed to decode command response value. Error: %" CHIP_ERROR_FORMAT, error.Format()); @@ -432,18 +436,12 @@ void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data) ChipLogProgress(NotSpecified, " commissioningTimeout: %u", value.commissioningTimeout); ChipLogProgress(NotSpecified, " discriminator: %u", value.discriminator); ChipLogProgress(NotSpecified, " iterations: %u", value.iterations); + ChipLogProgress(NotSpecified, " PAKEPasscodeVerifier size: %lu", value.PAKEPasscodeVerifier.size()); + ChipLogProgress(NotSpecified, " salt size: %lu", value.salt.size()); - char verifierHex[Crypto::kSpake2p_VerifierSerialized_Length * 2 + 1]; - Encoding::BytesToHex(value.PAKEPasscodeVerifier.data(), value.PAKEPasscodeVerifier.size(), verifierHex, sizeof(verifierHex), - Encoding::HexFlags::kNullTerminate); - ChipLogProgress(NotSpecified, " PAKEPasscodeVerifier: %s", verifierHex); - - char saltHex[Crypto::kSpake2p_Max_PBKDF_Salt_Length * 2 + 1]; - Encoding::BytesToHex(value.salt.data(), value.salt.size(), saltHex, sizeof(saltHex), Encoding::HexFlags::kNullTerminate); - ChipLogProgress(NotSpecified, " salt: %s", saltHex); - - OpenDeviceCommissioningWindow(mLocalBridgeNodeId, value.commissioningTimeout, value.iterations, value.discriminator, saltHex, - verifierHex); + OpenDeviceCommissioningWindow(mLocalBridgeNodeId, value.commissioningTimeout, value.iterations, value.discriminator, + ByteSpan(value.salt.data(), value.salt.size()), + ByteSpan(value.PAKEPasscodeVerifier.data(), value.PAKEPasscodeVerifier.size())); } void DeviceManager::HandleAttributeData(const app::ConcreteDataAttributePath & path, TLV::TLVReader & data) diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 5f617845f7dc76..622ceaf91ab91f 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -100,12 +100,12 @@ class DeviceManager : public PairingDelegate * for deriving the PAKE (Password Authenticated Key Exchange) verifier. * @param discriminator The device-specific discriminator, determined during commissioning, which helps * to uniquely identify the device among others. - * @param saltHex The hexadecimal-encoded salt used in the cryptographic operations for commissioning. - * @param verifierHex The hexadecimal-encoded PAKE verifier used to authenticate the commissioning process. + * @param salt The salt used in the cryptographic operations for commissioning. + * @param verifier The PAKE verifier used to authenticate the commissioning process. * */ void OpenDeviceCommissioningWindow(chip::NodeId nodeId, uint32_t commissioningTimeout, uint32_t iterations, - uint32_t discriminator, const char * saltHex, const char * verifierHex); + uint16_t discriminator, const chip::ByteSpan & salt, const chip::ByteSpan & verifier); /** * @brief Open the commissioning window of a device from another fabric via its fabric bridge. diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp new file mode 100644 index 00000000000000..0a711970f421cb --- /dev/null +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -0,0 +1,147 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "PairingManager.h" + +#include +#include +#include + +using namespace ::chip; + +Platform::UniquePtr PairingManager::mWindowOpener; + +PairingManager::PairingManager() : + mOnOpenCommissioningWindowCallback(OnOpenCommissioningWindowResponse, this), + mOnOpenCommissioningWindowVerifierCallback(OnOpenCommissioningWindowVerifierResponse, this) +{} + +CHIP_ERROR PairingManager::Init(chip::Controller::DeviceCommissioner * commissioner) +{ + mCommissioner = commissioner; + return CHIP_NO_ERROR; +} + +CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId endpointId, uint16_t commissioningTimeout, + uint32_t iterations, uint16_t discriminator, const ByteSpan & salt, + const ByteSpan & verifier) +{ + VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); + + auto * params = new CommissioningWindowParams(); + params->nodeId = nodeId; + params->endpointId = endpointId; + params->commissioningWindowTimeout = commissioningTimeout; + params->iteration = iterations; + params->discriminator = discriminator; + + if (!salt.empty()) + { + memcpy(params->saltBuffer, salt.data(), salt.size()); + params->salt = ByteSpan(params->saltBuffer, salt.size()); + } + + if (!verifier.empty()) + { + memcpy(params->verifierBuffer, verifier.data(), verifier.size()); + params->verifier = ByteSpan(params->verifierBuffer, verifier.size()); + } + + // Schedule work on the Matter thread + chip::DeviceLayer::PlatformMgr().ScheduleWork(OnOpenCommissioningWindow, reinterpret_cast(params)); + + return CHIP_NO_ERROR; +} + +void PairingManager::OnOpenCommissioningWindow(intptr_t context) +{ + auto * params = reinterpret_cast(context); + PairingManager & self = PairingManager::Instance(); + + if (self.mCommissioner == nullptr) + { + ChipLogError(AppServer, "Commissioner is null, cannot open commissioning window"); + return; + } + + mWindowOpener = Platform::MakeUnique(self.mCommissioner); + + if (!params->verifier.empty()) + { + if (params->salt.empty()) + { + ChipLogError(AppServer, "Salt is required when verifier is set"); + return; + } + + CHIP_ERROR err = mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowVerifierParams() + .SetNodeId(params->nodeId) + .SetEndpointId(params->endpointId) + .SetTimeout(params->commissioningWindowTimeout) + .SetIteration(params->iteration) + .SetDiscriminator(params->discriminator) + .SetVerifier(params->verifier) + .SetSalt(params->salt) + .SetCallback(&self.mOnOpenCommissioningWindowVerifierCallback)); + if (err != CHIP_NO_ERROR) + { + ChipLogError(AppServer, "Failed to open commissioning window with verifier: %s", ErrorStr(err)); + } + } + else + { + SetupPayload ignored; + CHIP_ERROR err = mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() + .SetNodeId(params->nodeId) + .SetEndpointId(params->endpointId) + .SetTimeout(params->commissioningWindowTimeout) + .SetIteration(params->iteration) + .SetDiscriminator(params->discriminator) + .SetSetupPIN(NullOptional) + .SetSalt(NullOptional) + .SetCallback(&self.mOnOpenCommissioningWindowCallback), + ignored); + if (err != CHIP_NO_ERROR) + { + ChipLogError(AppServer, "Failed to open commissioning window with passcode: %s", ErrorStr(err)); + } + } + + // Clean up params + delete params; +} + +void PairingManager::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err, chip::SetupPayload payload) +{ + PairingManager * self = static_cast(context); + if (self->mCommissioningWindowDelegate) + { + self->mCommissioningWindowDelegate->OnCommissioningWindowOpened(remoteId, err, payload); + self->UnregisterOpenCommissioningWindowDelegate(); + } + + OnOpenCommissioningWindowVerifierResponse(context, remoteId, err); +} + +void PairingManager::OnOpenCommissioningWindowVerifierResponse(void * context, NodeId remoteId, CHIP_ERROR err) +{ + LogErrorOnFailure(err); + + PairingManager * self = reinterpret_cast(context); + VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnOpenCommissioningWindowCommand: context is null")); +} diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h new file mode 100644 index 00000000000000..015392d4f8e08c --- /dev/null +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -0,0 +1,87 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +class CommissioningWindowDelegate +{ +public: + virtual void OnCommissioningWindowOpened(chip::NodeId deviceId, CHIP_ERROR err, chip::SetupPayload payload) = 0; + virtual ~CommissioningWindowDelegate() = default; +}; + +class PairingManager +{ +public: + static PairingManager & Instance() + { + static PairingManager instance; + return instance; + } + + CHIP_ERROR Init(chip::Controller::DeviceCommissioner * commissioner); + + CHIP_ERROR OpenCommissioningWindow(chip::NodeId nodeId, chip::EndpointId endpointId, uint16_t commissioningTimeout, + uint32_t iterations, uint16_t discriminator, const chip::ByteSpan & salt, + const chip::ByteSpan & verifier); + + void RegisterOpenCommissioningWindowDelegate(CommissioningWindowDelegate * delegate) + { + ChipLogProgress(NotSpecified, "yujuan: PairingManager::RegisterOpenCommissioningWindowDelegate"); + mCommissioningWindowDelegate = delegate; + } + + void UnregisterOpenCommissioningWindowDelegate() { mCommissioningWindowDelegate = nullptr; } + +private: + PairingManager(); + PairingManager(const PairingManager &) = delete; + PairingManager & operator=(const PairingManager &) = delete; + + chip::Controller::DeviceCommissioner * mCommissioner = nullptr; + + /////////// Open Commissioning Window Command Interface ///////// + struct CommissioningWindowParams + { + chip::NodeId nodeId; + chip::EndpointId endpointId; + uint16_t commissioningWindowTimeout; + uint32_t iteration; + uint16_t discriminator; + chip::Optional setupPIN; + uint8_t verifierBuffer[chip::Crypto::kSpake2p_VerifierSerialized_Length]; + chip::ByteSpan verifier; + uint8_t saltBuffer[chip::Crypto::kSpake2p_Max_PBKDF_Salt_Length]; + chip::ByteSpan salt; + }; + + CommissioningWindowDelegate * mCommissioningWindowDelegate = nullptr; + + static chip::Platform::UniquePtr mWindowOpener; + 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; +}; diff --git a/examples/fabric-admin/rpc/RpcServer.cpp b/examples/fabric-admin/rpc/RpcServer.cpp index 94bca111d9c421..69875fc5e81a27 100644 --- a/examples/fabric-admin/rpc/RpcServer.cpp +++ b/examples/fabric-admin/rpc/RpcServer.cpp @@ -86,18 +86,17 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate NodeId nodeId = request.node_id; uint32_t commissioningTimeout = request.commissioning_timeout; uint32_t iterations = request.iterations; - uint32_t discriminator = request.discriminator; + uint16_t discriminator = request.discriminator; - char saltHex[Crypto::kSpake2p_Max_PBKDF_Salt_Length * 2 + 1]; - Encoding::BytesToHex(request.salt.bytes, request.salt.size, saltHex, sizeof(saltHex), Encoding::HexFlags::kNullTerminate); - - char verifierHex[Crypto::kSpake2p_VerifierSerialized_Length * 2 + 1]; - Encoding::BytesToHex(request.verifier.bytes, request.verifier.size, verifierHex, sizeof(verifierHex), - Encoding::HexFlags::kNullTerminate); - - ChipLogProgress(NotSpecified, "Received OpenCommissioningWindow request: 0x%lx", nodeId); + // Log the request details for debugging + ChipLogProgress(NotSpecified, + "Received OpenCommissioningWindow request: NodeId 0x%lx, Timeout: %u, Iterations: %u, Discriminator: %u", + static_cast(nodeId), commissioningTimeout, iterations, discriminator); - DeviceMgr().OpenDeviceCommissioningWindow(nodeId, commissioningTimeout, iterations, discriminator, saltHex, verifierHex); + // Open the device commissioning window using raw binary data for salt and verifier + DeviceMgr().OpenDeviceCommissioningWindow(nodeId, commissioningTimeout, iterations, discriminator, + ByteSpan(request.salt.bytes, request.salt.size), + ByteSpan(request.verifier.bytes, request.verifier.size)); response.success = true; From 98dbd416df2097643cadbaeba432d4a4bb8a8426 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 17 Sep 2024 13:52:22 -0700 Subject: [PATCH 2/5] Address review comments --- .../commands/common/CHIPCommand.cpp | 2 +- .../fabric-sync/FabricSyncCommand.cpp | 2 +- .../device_manager/DeviceManager.cpp | 1 - .../device_manager/PairingManager.cpp | 105 +++++++++++------- .../device_manager/PairingManager.h | 53 +++++++-- 5 files changed, 110 insertions(+), 53 deletions(-) diff --git a/examples/fabric-admin/commands/common/CHIPCommand.cpp b/examples/fabric-admin/commands/common/CHIPCommand.cpp index 4c955f846882da..b18eb9f2de472a 100644 --- a/examples/fabric-admin/commands/common/CHIPCommand.cpp +++ b/examples/fabric-admin/commands/common/CHIPCommand.cpp @@ -182,7 +182,7 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack() mCredIssuerCmds->SetCredentialIssuerOption(CredentialIssuerCommands::CredentialIssuerOptions::kAllowTestCdSigningKey, allowTestCdSigningKey); - ReturnLogErrorOnFailure(PairingManager::Instance().Init(&CurrentCommissioner())); + PairingManager::Instance().Init(&CurrentCommissioner()); return CHIP_NO_ERROR; } diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp index 9a07d3b2e368eb..64d43cce11f3ac 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp @@ -344,7 +344,7 @@ CHIP_ERROR FabricSyncDeviceCommand::RunCommand(EndpointId remoteId) return CHIP_NO_ERROR; } - PairingManager::Instance().RegisterOpenCommissioningWindowDelegate(this); + PairingManager::Instance().SetOpenCommissioningWindowDelegate(this); DeviceMgr().OpenRemoteDeviceCommissioningWindow(remoteId); diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index c04d5d6291f78e..c35170ae509e1e 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -134,7 +134,6 @@ void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpoin // Open the commissioning window of a device from another fabric via its fabric bridge. // This method constructs and sends a command to open the commissioning window for a device // that is part of a different fabric, accessed through a fabric bridge. - StringBuilder commandBuilder; // Use random discriminator to have less chance of collision. uint16_t discriminator = diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 0a711970f421cb..addc2effb12c28 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -24,26 +24,35 @@ using namespace ::chip; -Platform::UniquePtr PairingManager::mWindowOpener; - PairingManager::PairingManager() : mOnOpenCommissioningWindowCallback(OnOpenCommissioningWindowResponse, this), mOnOpenCommissioningWindowVerifierCallback(OnOpenCommissioningWindowVerifierResponse, this) {} -CHIP_ERROR PairingManager::Init(chip::Controller::DeviceCommissioner * commissioner) +void PairingManager::Init(Controller::DeviceCommissioner * commissioner) { + VerifyOrDie(mCommissioner == nullptr); mCommissioner = commissioner; - return CHIP_NO_ERROR; } CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId endpointId, uint16_t commissioningTimeout, uint32_t iterations, uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier) { - VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); + if (mCommissioner == nullptr) + { + ChipLogError(NotSpecified, "Commissioner is null, cannot open commissioning window"); + return CHIP_ERROR_INCORRECT_STATE; + } + + // Check if a window is already open + if (mWindowOpener != nullptr) + { + ChipLogError(NotSpecified, "A commissioning window is already open"); + return CHIP_ERROR_INCORRECT_STATE; + } - auto * params = new CommissioningWindowParams(); + auto params = Platform::MakeUnique(); params->nodeId = nodeId; params->endpointId = endpointId; params->commissioningWindowTimeout = commissioningTimeout; @@ -52,87 +61,99 @@ CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId end if (!salt.empty()) { + if (salt.size() > Crypto::kSpake2p_Max_PBKDF_Salt_Length) + { + ChipLogError(NotSpecified, "Salt size exceeds buffer capacity"); + return CHIP_ERROR_BUFFER_TOO_SMALL; + } + memcpy(params->saltBuffer, salt.data(), salt.size()); params->salt = ByteSpan(params->saltBuffer, salt.size()); } if (!verifier.empty()) { + if (verifier.size() > Crypto::kSpake2p_VerifierSerialized_Length) + { + ChipLogError(NotSpecified, "Verifier size exceeds buffer capacity"); + return CHIP_ERROR_BUFFER_TOO_SMALL; + } + memcpy(params->verifierBuffer, verifier.data(), verifier.size()); params->verifier = ByteSpan(params->verifierBuffer, verifier.size()); } // Schedule work on the Matter thread - chip::DeviceLayer::PlatformMgr().ScheduleWork(OnOpenCommissioningWindow, reinterpret_cast(params)); - - return CHIP_NO_ERROR; + return DeviceLayer::PlatformMgr().ScheduleWork(OnOpenCommissioningWindow, reinterpret_cast(params.release())); } void PairingManager::OnOpenCommissioningWindow(intptr_t context) { - auto * params = reinterpret_cast(context); + Platform::UniquePtr params(reinterpret_cast(context)); PairingManager & self = PairingManager::Instance(); if (self.mCommissioner == nullptr) { - ChipLogError(AppServer, "Commissioner is null, cannot open commissioning window"); + ChipLogError(NotSpecified, "Commissioner is null, cannot open commissioning window"); return; } - mWindowOpener = Platform::MakeUnique(self.mCommissioner); + self.mWindowOpener = Platform::MakeUnique(self.mCommissioner); if (!params->verifier.empty()) { if (params->salt.empty()) { - ChipLogError(AppServer, "Salt is required when verifier is set"); + ChipLogError(NotSpecified, "Salt is required when verifier is set"); + self.mWindowOpener.reset(); return; } - CHIP_ERROR err = mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowVerifierParams() - .SetNodeId(params->nodeId) - .SetEndpointId(params->endpointId) - .SetTimeout(params->commissioningWindowTimeout) - .SetIteration(params->iteration) - .SetDiscriminator(params->discriminator) - .SetVerifier(params->verifier) - .SetSalt(params->salt) - .SetCallback(&self.mOnOpenCommissioningWindowVerifierCallback)); + CHIP_ERROR err = + self.mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowVerifierParams() + .SetNodeId(params->nodeId) + .SetEndpointId(params->endpointId) + .SetTimeout(params->commissioningWindowTimeout) + .SetIteration(params->iteration) + .SetDiscriminator(params->discriminator) + .SetVerifier(params->verifier) + .SetSalt(params->salt) + .SetCallback(&self.mOnOpenCommissioningWindowVerifierCallback)); if (err != CHIP_NO_ERROR) { - ChipLogError(AppServer, "Failed to open commissioning window with verifier: %s", ErrorStr(err)); + ChipLogError(NotSpecified, "Failed to open commissioning window with verifier: %s", ErrorStr(err)); + self.mWindowOpener.reset(); } } else { SetupPayload ignored; - CHIP_ERROR err = mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() - .SetNodeId(params->nodeId) - .SetEndpointId(params->endpointId) - .SetTimeout(params->commissioningWindowTimeout) - .SetIteration(params->iteration) - .SetDiscriminator(params->discriminator) - .SetSetupPIN(NullOptional) - .SetSalt(NullOptional) - .SetCallback(&self.mOnOpenCommissioningWindowCallback), - ignored); + CHIP_ERROR err = self.mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() + .SetNodeId(params->nodeId) + .SetEndpointId(params->endpointId) + .SetTimeout(params->commissioningWindowTimeout) + .SetIteration(params->iteration) + .SetDiscriminator(params->discriminator) + .SetSetupPIN(NullOptional) + .SetSalt(NullOptional) + .SetCallback(&self.mOnOpenCommissioningWindowCallback), + ignored); if (err != CHIP_NO_ERROR) { - ChipLogError(AppServer, "Failed to open commissioning window with passcode: %s", ErrorStr(err)); + ChipLogError(NotSpecified, "Failed to open commissioning window with passcode: %s", ErrorStr(err)); + self.mWindowOpener.reset(); } } - - // Clean up params - delete params; } -void PairingManager::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err, chip::SetupPayload payload) +void PairingManager::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err, SetupPayload payload) { + VerifyOrDie(context != nullptr); PairingManager * self = static_cast(context); if (self->mCommissioningWindowDelegate) { self->mCommissioningWindowDelegate->OnCommissioningWindowOpened(remoteId, err, payload); - self->UnregisterOpenCommissioningWindowDelegate(); + self->SetOpenCommissioningWindowDelegate(nullptr); } OnOpenCommissioningWindowVerifierResponse(context, remoteId, err); @@ -140,8 +161,10 @@ void PairingManager::OnOpenCommissioningWindowResponse(void * context, NodeId re void PairingManager::OnOpenCommissioningWindowVerifierResponse(void * context, NodeId remoteId, CHIP_ERROR err) { + VerifyOrDie(context != nullptr); + PairingManager * self = static_cast(context); LogErrorOnFailure(err); - PairingManager * self = reinterpret_cast(context); - VerifyOrReturn(self != nullptr, ChipLogError(NotSpecified, "OnOpenCommissioningWindowCommand: context is null")); + // Reset the window opener once the window operation is complete + self->mWindowOpener.reset(); } diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h index 015392d4f8e08c..3ff54cf1392095 100644 --- a/examples/fabric-admin/device_manager/PairingManager.h +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -29,6 +29,26 @@ class CommissioningWindowDelegate virtual ~CommissioningWindowDelegate() = 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 + * should only be one instance of it running at any given time. + * + * Usage: + * + * 1. The class should be initialized when the system starts up, typically by invoking the static + * instance method to get the singleton. + * 2. To open a commissioning window, the appropriate method should be called on the PairingManager instance. + * 3. The PairingManager will handle the lifecycle of the CommissioningWindowOpener and ensure that + * resources are cleaned up appropriately when pairing is complete or the process is aborted. + * + * Example: + * + * @code + * PairingManager& manager = PairingManager::Instance(); + * manager.OpenCommissioningWindow(); + * @endcode + */ class PairingManager { public: @@ -38,19 +58,28 @@ class PairingManager return instance; } - CHIP_ERROR Init(chip::Controller::DeviceCommissioner * commissioner); + void Init(chip::Controller::DeviceCommissioner * commissioner); + /** + * Opens a commissioning window on the specified node and endpoint. + * Only one commissioning window can be active at a time. If a commissioning + * window is already open, this function will return an error. + * + * @param nodeId The target node ID for commissioning. + * @param endpointId The target endpoint ID for commissioning. + * @param commissioningTimeout Timeout for the commissioning window in seconds. + * @param iterations Iterations for PBKDF calculations. + * @param discriminator Discriminator for commissioning. + * @param salt Optional salt for verifier-based commissioning. + * @param verifier Optional verifier for enhanced commissioning security. + * + * @return CHIP_ERROR_INCORRECT_STATE if a commissioning window is already open. + */ CHIP_ERROR OpenCommissioningWindow(chip::NodeId nodeId, chip::EndpointId endpointId, uint16_t commissioningTimeout, uint32_t iterations, uint16_t discriminator, const chip::ByteSpan & salt, const chip::ByteSpan & verifier); - void RegisterOpenCommissioningWindowDelegate(CommissioningWindowDelegate * delegate) - { - ChipLogProgress(NotSpecified, "yujuan: PairingManager::RegisterOpenCommissioningWindowDelegate"); - mCommissioningWindowDelegate = delegate; - } - - void UnregisterOpenCommissioningWindowDelegate() { mCommissioningWindowDelegate = nullptr; } + void SetOpenCommissioningWindowDelegate(CommissioningWindowDelegate * delegate) { mCommissioningWindowDelegate = delegate; } private: PairingManager(); @@ -76,7 +105,13 @@ class PairingManager CommissioningWindowDelegate * mCommissioningWindowDelegate = nullptr; - static chip::Platform::UniquePtr mWindowOpener; + /** + * 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; + static void OnOpenCommissioningWindow(intptr_t context); static void OnOpenCommissioningWindowResponse(void * context, chip::NodeId deviceId, CHIP_ERROR status, chip::SetupPayload payload); From 8bbdfc38193d0fdf8a370793c4c3aa365c3aca98 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 18 Sep 2024 09:19:37 -0700 Subject: [PATCH 3/5] Update examples/fabric-admin/device_manager/PairingManager.h Co-authored-by: Andrei Litvin --- examples/fabric-admin/device_manager/PairingManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h index 3ff54cf1392095..e33f21ee752af0 100644 --- a/examples/fabric-admin/device_manager/PairingManager.h +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -75,7 +75,7 @@ class PairingManager * * @return CHIP_ERROR_INCORRECT_STATE if a commissioning window is already open. */ - CHIP_ERROR OpenCommissioningWindow(chip::NodeId nodeId, chip::EndpointId endpointId, uint16_t commissioningTimeout, + CHIP_ERROR OpenCommissioningWindow(chip::NodeId nodeId, chip::EndpointId endpointId, uint16_t commissioningTimeoutSec, uint32_t iterations, uint16_t discriminator, const chip::ByteSpan & salt, const chip::ByteSpan & verifier); From 8cba7e0f2829cf49f81874850b7d807f0ded4e1c Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 18 Sep 2024 09:35:08 -0700 Subject: [PATCH 4/5] Update per review comments --- examples/fabric-admin/device_manager/DeviceManager.cpp | 6 +++--- examples/fabric-admin/device_manager/DeviceManager.h | 4 ++-- examples/fabric-admin/device_manager/PairingManager.cpp | 8 ++++---- examples/fabric-admin/device_manager/PairingManager.h | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index c35170ae509e1e..897b1b60e0b68b 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -115,14 +115,14 @@ void DeviceManager::RemoveSyncedDevice(NodeId nodeId) ChipLogValueX64(device->GetNodeId()), device->GetEndpointId()); } -void DeviceManager::OpenDeviceCommissioningWindow(NodeId nodeId, uint32_t commissioningTimeout, uint32_t iterations, +void DeviceManager::OpenDeviceCommissioningWindow(NodeId nodeId, uint32_t commissioningTimeoutSec, uint32_t iterations, uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier) { ChipLogProgress(NotSpecified, "Opening commissioning window for Node ID: " ChipLogFormatX64, ChipLogValueX64(nodeId)); // Open the commissioning window of a device within its own fabric. - CHIP_ERROR err = PairingManager::Instance().OpenCommissioningWindow(nodeId, kRootEndpointId, commissioningTimeout, iterations, - discriminator, salt, verifier); + CHIP_ERROR err = PairingManager::Instance().OpenCommissioningWindow(nodeId, kRootEndpointId, commissioningTimeoutSec, + iterations, discriminator, salt, verifier); if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "Failed to open commissioning window: %s", ErrorStr(err)); diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 622ceaf91ab91f..939329be82c691 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -94,7 +94,7 @@ class DeviceManager : public PairingDelegate * This function initiates the process to open the commissioning window for a device identified by the given node ID. * * @param nodeId The ID of the node that should open the commissioning window. - * @param commissioningTimeout The time in seconds before the commissioning window closes. This value determines + * @param commissioningTimeoutSec The time in seconds before the commissioning window closes. This value determines * how long the commissioning window remains open for incoming connections. * @param iterations The number of PBKDF (Password-Based Key Derivation Function) iterations to use * for deriving the PAKE (Password Authenticated Key Exchange) verifier. @@ -104,7 +104,7 @@ class DeviceManager : public PairingDelegate * @param verifier The PAKE verifier used to authenticate the commissioning process. * */ - void OpenDeviceCommissioningWindow(chip::NodeId nodeId, uint32_t commissioningTimeout, uint32_t iterations, + void OpenDeviceCommissioningWindow(chip::NodeId nodeId, uint32_t commissioningTimeoutSec, uint32_t iterations, uint16_t discriminator, const chip::ByteSpan & salt, const chip::ByteSpan & verifier); /** diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index addc2effb12c28..aa56af4bc6bf4e 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -35,7 +35,7 @@ void PairingManager::Init(Controller::DeviceCommissioner * commissioner) mCommissioner = commissioner; } -CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId endpointId, uint16_t commissioningTimeout, +CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId endpointId, uint16_t commissioningTimeoutSec, uint32_t iterations, uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier) { @@ -55,13 +55,13 @@ CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId end auto params = Platform::MakeUnique(); params->nodeId = nodeId; params->endpointId = endpointId; - params->commissioningWindowTimeout = commissioningTimeout; + params->commissioningWindowTimeout = commissioningTimeoutSec; params->iteration = iterations; params->discriminator = discriminator; if (!salt.empty()) { - if (salt.size() > Crypto::kSpake2p_Max_PBKDF_Salt_Length) + if (salt.size() > sizeof(params->saltBuffer)) { ChipLogError(NotSpecified, "Salt size exceeds buffer capacity"); return CHIP_ERROR_BUFFER_TOO_SMALL; @@ -73,7 +73,7 @@ CHIP_ERROR PairingManager::OpenCommissioningWindow(NodeId nodeId, EndpointId end if (!verifier.empty()) { - if (verifier.size() > Crypto::kSpake2p_VerifierSerialized_Length) + if (verifier.size() > sizeof(params->verifierBuffer)) { ChipLogError(NotSpecified, "Verifier size exceeds buffer capacity"); return CHIP_ERROR_BUFFER_TOO_SMALL; diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h index e33f21ee752af0..de1c0887f3b1b7 100644 --- a/examples/fabric-admin/device_manager/PairingManager.h +++ b/examples/fabric-admin/device_manager/PairingManager.h @@ -67,7 +67,7 @@ class PairingManager * * @param nodeId The target node ID for commissioning. * @param endpointId The target endpoint ID for commissioning. - * @param commissioningTimeout Timeout for the commissioning window in seconds. + * @param commissioningTimeoutSec Timeout for the commissioning window in seconds. * @param iterations Iterations for PBKDF calculations. * @param discriminator Discriminator for commissioning. * @param salt Optional salt for verifier-based commissioning. From ea3e2958e8bd970cb0c620b8898d4f3fdafab9b8 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 18 Sep 2024 09:37:43 -0700 Subject: [PATCH 5/5] Update commissioningTimeout to commissioningTimeoutSec --- examples/fabric-admin/rpc/RpcServer.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/fabric-admin/rpc/RpcServer.cpp b/examples/fabric-admin/rpc/RpcServer.cpp index 18e5d8aa43d07c..6e5e2ec8d66442 100644 --- a/examples/fabric-admin/rpc/RpcServer.cpp +++ b/examples/fabric-admin/rpc/RpcServer.cpp @@ -86,18 +86,18 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate pw::Status OpenCommissioningWindow(const chip_rpc_DeviceCommissioningWindowInfo & request, chip_rpc_OperationStatus & response) override { - NodeId nodeId = request.node_id; - uint32_t commissioningTimeout = request.commissioning_timeout; - uint32_t iterations = request.iterations; - uint16_t discriminator = request.discriminator; + NodeId nodeId = request.node_id; + uint32_t commissioningTimeoutSec = request.commissioning_timeout; + uint32_t iterations = request.iterations; + uint16_t discriminator = request.discriminator; // Log the request details for debugging ChipLogProgress(NotSpecified, "Received OpenCommissioningWindow request: NodeId 0x%lx, Timeout: %u, Iterations: %u, Discriminator: %u", - static_cast(nodeId), commissioningTimeout, iterations, discriminator); + static_cast(nodeId), commissioningTimeoutSec, iterations, discriminator); // Open the device commissioning window using raw binary data for salt and verifier - DeviceMgr().OpenDeviceCommissioningWindow(nodeId, commissioningTimeout, iterations, discriminator, + DeviceMgr().OpenDeviceCommissioningWindow(nodeId, commissioningTimeoutSec, iterations, discriminator, ByteSpan(request.salt.bytes, request.salt.size), ByteSpan(request.verifier.bytes, request.verifier.size));