From 35162b78a78dcb8504b8307f5cd7946807e2f2a8 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Fri, 7 Jun 2024 00:00:29 -0700 Subject: [PATCH 1/3] Allows opening a commissioning window on a bridged node that is not on Endpoint 0. --- .../administrator-commissioning-server.cpp | 3 ++ src/controller/CommissioningWindowOpener.cpp | 28 ++++++++++++-- src/controller/CommissioningWindowOpener.h | 38 +++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp b/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp index 83a0b04281b1ad..68179e44d9da48 100644 --- a/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp +++ b/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp @@ -91,6 +91,8 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback( auto & iterations = commandData.iterations; auto & salt = commandData.salt; + EndpointId endpointId = commandPath.mEndpointId; + Optional status = Optional::Missing(); InteractionModel::Status globalStatus = InteractionModel::Status::Success; Spake2pVerifier verifier; @@ -106,6 +108,7 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback( VerifyOrExit(failSafeContext.IsFailSafeFullyDisarmed(), status.Emplace(StatusCode::kBusy)); VerifyOrExit(!commissionMgr.IsCommissioningWindowOpen(), status.Emplace(StatusCode::kBusy)); + VerifyOrExit(endpointId == kRootEndpointId, status.Emplace(StatusCode::kPAKEParameterError)); VerifyOrExit(iterations >= kSpake2p_Min_PBKDF_Iterations, status.Emplace(StatusCode::kPAKEParameterError)); VerifyOrExit(iterations <= kSpake2p_Max_PBKDF_Iterations, status.Emplace(StatusCode::kPAKEParameterError)); VerifyOrExit(salt.size() >= kSpake2p_Min_PBKDF_Salt_Length, status.Emplace(StatusCode::kPAKEParameterError)); diff --git a/src/controller/CommissioningWindowOpener.cpp b/src/controller/CommissioningWindowOpener.cpp index 47666972137bcc..dd5da38bcfa4ee 100644 --- a/src/controller/CommissioningWindowOpener.cpp +++ b/src/controller/CommissioningWindowOpener.cpp @@ -47,6 +47,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenBasicCommissioningWindow(NodeId device mBasicCommissioningWindowCallback = callback; mCommissioningWindowCallback = nullptr; mNodeId = deviceId; + mEndpointId = kRootEndpointId; mCommissioningWindowTimeout = timeout; mNextStep = Step::kOpenCommissioningWindow; @@ -58,6 +59,28 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, S Optional salt, Callback::Callback * callback, SetupPayload & payload, bool readVIDPIDAttributes) +{ + return OpenCommissioningWindowImpl(deviceId, kRootEndpointId, timeout, iteration, discriminator, setupPIN, salt, callback, + payload, readVIDPIDAttributes); +} + +CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, EndpointId endpointId, Seconds16 timeout, + uint32_t iteration, uint16_t discriminator, + Optional setupPIN, Optional salt, + Callback::Callback * callback, + SetupPayload & payload) +{ + + VerifyOrReturnError(endpointId != kRootEndpointId, CHIP_ERROR_INVALID_ARGUMENT); + return OpenCommissioningWindowImpl(deviceId, endpointId, timeout, iteration, discriminator, setupPIN, salt, callback, payload, + false); +} + +CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowImpl(NodeId deviceId, EndpointId endpointId, Seconds16 timeout, + uint32_t iteration, uint16_t discriminator, + Optional setupPIN, Optional salt, + Callback::Callback * callback, + SetupPayload & payload, bool readVIDPIDAttributes) { VerifyOrReturnError(mNextStep == Step::kAcceptCommissioningStart, CHIP_ERROR_INCORRECT_STATE); @@ -103,6 +126,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, S mCommissioningWindowCallback = callback; mBasicCommissioningWindowCallback = nullptr; mNodeId = deviceId; + mEndpointId = endpointId; mCommissioningWindowTimeout = timeout; mPBKDFIterations = iteration; @@ -129,9 +153,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowInternal(Messaging: { ChipLogProgress(Controller, "OpenCommissioningWindow for device ID 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId)); - constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0; - - ClusterBase cluster(exchangeMgr, sessionHandle, kAdministratorCommissioningClusterEndpoint); + ClusterBase cluster(exchangeMgr, sessionHandle, mEndpointId); if (mCommissioningWindowOption != CommissioningWindowOption::kOriginalSetupCode) { diff --git a/src/controller/CommissioningWindowOpener.h b/src/controller/CommissioningWindowOpener.h index 10547dce3a662d..66355ff1dc2119 100644 --- a/src/controller/CommissioningWindowOpener.h +++ b/src/controller/CommissioningWindowOpener.h @@ -107,6 +107,39 @@ class CommissioningWindowOpener Callback::Callback * callback, SetupPayload & payload, bool readVIDPIDAttributes = false); + /** + * @brief + * Try to look up the bridged device on the bridge with the given node id + * of the bridge and endpoint id of the bridged device and ask it to re-enter + * commissioning mode with a PASE verifier derived from the given information + * and the given discriminator. The bridged device will exit commissioning mode + * after a successful commissioning, or after the given `timeout` time. + * + * @param[in] deviceId The device Id of the bridge. + * @param[in] endpoinId The endpoint Id of the bridged node on the bridge.* + * @param[in] timeout The commissioning mode should terminate after this much time. + * @param[in] iteration The PAKE iteration count associated with the PAKE Passcode ID and ephemeral + * PAKE passcode verifier to be used for this commissioning. + * @param[in] discriminator The long discriminator for the DNS-SD advertisement. + * @param[in] setupPIN The setup PIN to use, or NullOptional to use a randomly-generated one. + * @param[in] salt The salt to use, or NullOptional to use a + * randomly-generated one. If provided, must be at + * least kSpake2p_Min_PBKDF_Salt_Length bytes and + * at most kSpake2p_Max_PBKDF_Salt_Length bytes in + * length. + * @param[in] callback The function to be called on success or failure of opening of commissioning window. + * @param[out] payload The setup payload, not including the VID/PID bits, + * even if those were asked for, that is generated + * based on the passed-in information. The payload + * provided to the callback function, unlike this + * out parameter, will include the VID/PID bits if + * readVIDPIDAttributes is true. + * + */ + CHIP_ERROR OpenCommissioningWindow(NodeId deviceId, EndpointId endpointId, System::Clock::Seconds16 timeout, uint32_t iteration, + uint16_t discriminator, Optional setupPIN, Optional salt, + Callback::Callback * callback, SetupPayload & payload); + private: enum class Step : uint8_t { @@ -120,6 +153,10 @@ class CommissioningWindowOpener kOpenCommissioningWindow, }; + CHIP_ERROR OpenCommissioningWindowImpl(NodeId deviceId, EndpointId endpointId, System::Clock::Seconds16 timeout, + uint32_t iteration, uint16_t discriminator, Optional setupPIN, + Optional salt, Callback::Callback * callback, + SetupPayload & payload, bool readVIDPIDAttributes); CHIP_ERROR OpenCommissioningWindowInternal(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle); static void OnPIDReadResponse(void * context, uint16_t value); static void OnVIDReadResponse(void * context, VendorId value); @@ -137,6 +174,7 @@ class CommissioningWindowOpener Callback::Callback * mBasicCommissioningWindowCallback = nullptr; SetupPayload mSetupPayload; NodeId mNodeId = kUndefinedNodeId; + EndpointId mEndpointId = kInvalidEndpointId; System::Clock::Seconds16 mCommissioningWindowTimeout = System::Clock::kZero; CommissioningWindowOption mCommissioningWindowOption = CommissioningWindowOption::kOriginalSetupCode; Crypto::Spake2pVerifier mVerifier; // Used for non-basic commissioning. From 8b4756e1107dcd1f5982543aab71730f4df007e0 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Fri, 7 Jun 2024 11:54:27 -0700 Subject: [PATCH 2/3] Update src/controller/CommissioningWindowOpener.h Co-authored-by: Boris Zbarsky --- src/controller/CommissioningWindowOpener.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/CommissioningWindowOpener.h b/src/controller/CommissioningWindowOpener.h index 66355ff1dc2119..16f84193925b1a 100644 --- a/src/controller/CommissioningWindowOpener.h +++ b/src/controller/CommissioningWindowOpener.h @@ -174,7 +174,7 @@ class CommissioningWindowOpener Callback::Callback * mBasicCommissioningWindowCallback = nullptr; SetupPayload mSetupPayload; NodeId mNodeId = kUndefinedNodeId; - EndpointId mEndpointId = kInvalidEndpointId; + EndpointId mTargetEndpointId = kInvalidEndpointId; System::Clock::Seconds16 mCommissioningWindowTimeout = System::Clock::kZero; CommissioningWindowOption mCommissioningWindowOption = CommissioningWindowOption::kOriginalSetupCode; Crypto::Spake2pVerifier mVerifier; // Used for non-basic commissioning. From 5d735ec90981973352f2fb805c1b807159b4c162 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Fri, 7 Jun 2024 14:38:06 -0700 Subject: [PATCH 3/3] Address review comments --- src/controller/CommissioningWindowOpener.cpp | 72 ++++++++++---------- src/controller/CommissioningWindowOpener.h | 56 +++++++-------- 2 files changed, 60 insertions(+), 68 deletions(-) diff --git a/src/controller/CommissioningWindowOpener.cpp b/src/controller/CommissioningWindowOpener.cpp index dd5da38bcfa4ee..dbd305ffe61918 100644 --- a/src/controller/CommissioningWindowOpener.cpp +++ b/src/controller/CommissioningWindowOpener.cpp @@ -47,7 +47,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenBasicCommissioningWindow(NodeId device mBasicCommissioningWindowCallback = callback; mCommissioningWindowCallback = nullptr; mNodeId = deviceId; - mEndpointId = kRootEndpointId; + mTargetEndpointId = kRootEndpointId; mCommissioningWindowTimeout = timeout; mNextStep = Step::kOpenCommissioningWindow; @@ -60,58 +60,58 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, S Callback::Callback * callback, SetupPayload & payload, bool readVIDPIDAttributes) { - return OpenCommissioningWindowImpl(deviceId, kRootEndpointId, timeout, iteration, discriminator, setupPIN, salt, callback, - payload, readVIDPIDAttributes); + CommissioningWindowParams params = { + .deviceId = deviceId, + .endpointId = kRootEndpointId, + .timeout = timeout, + .iteration = iteration, + .discriminator = discriminator, + .setupPIN = setupPIN, + .salt = salt, + .callback = callback, + }; + + return OpenCommissioningWindowImpl(params, payload, readVIDPIDAttributes); } -CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, EndpointId endpointId, Seconds16 timeout, - uint32_t iteration, uint16_t discriminator, - Optional setupPIN, Optional salt, - Callback::Callback * callback, - SetupPayload & payload) +CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const CommissioningWindowParams & params, SetupPayload & payload) { - - VerifyOrReturnError(endpointId != kRootEndpointId, CHIP_ERROR_INVALID_ARGUMENT); - return OpenCommissioningWindowImpl(deviceId, endpointId, timeout, iteration, discriminator, setupPIN, salt, callback, payload, - false); + return OpenCommissioningWindowImpl(params, payload, false); } -CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowImpl(NodeId deviceId, EndpointId endpointId, Seconds16 timeout, - uint32_t iteration, uint16_t discriminator, - Optional setupPIN, Optional salt, - Callback::Callback * callback, - SetupPayload & payload, bool readVIDPIDAttributes) +CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowImpl(const CommissioningWindowParams & params, SetupPayload & payload, + bool readVIDPIDAttributes) { VerifyOrReturnError(mNextStep == Step::kAcceptCommissioningStart, CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(kSpake2p_Min_PBKDF_Iterations <= iteration && iteration <= kSpake2p_Max_PBKDF_Iterations, + VerifyOrReturnError(kSpake2p_Min_PBKDF_Iterations <= params.iteration && params.iteration <= kSpake2p_Max_PBKDF_Iterations, + CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(!params.salt.HasValue() || + (params.salt.Value().size() >= kSpake2p_Min_PBKDF_Salt_Length && + params.salt.Value().size() <= kSpake2p_Max_PBKDF_Salt_Length), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError( - !salt.HasValue() || - (salt.Value().size() >= kSpake2p_Min_PBKDF_Salt_Length && salt.Value().size() <= kSpake2p_Max_PBKDF_Salt_Length), - CHIP_ERROR_INVALID_ARGUMENT); mSetupPayload = SetupPayload(); - if (setupPIN.HasValue()) + if (params.setupPIN.HasValue()) { - if (!SetupPayload::IsValidSetupPIN(setupPIN.Value())) + if (!SetupPayload::IsValidSetupPIN(params.setupPIN.Value())) { return CHIP_ERROR_INVALID_ARGUMENT; } mCommissioningWindowOption = CommissioningWindowOption::kTokenWithProvidedPIN; - mSetupPayload.setUpPINCode = setupPIN.Value(); + mSetupPayload.setUpPINCode = params.setupPIN.Value(); } else { mCommissioningWindowOption = CommissioningWindowOption::kTokenWithRandomPIN; } - if (salt.HasValue()) + if (params.salt.HasValue()) { - memcpy(mPBKDFSaltBuffer, salt.Value().data(), salt.Value().size()); - mPBKDFSalt = ByteSpan(mPBKDFSaltBuffer, salt.Value().size()); + memcpy(mPBKDFSaltBuffer, params.salt.Value().data(), params.salt.Value().size()); + mPBKDFSalt = ByteSpan(mPBKDFSaltBuffer, params.salt.Value().size()); } else { @@ -120,17 +120,17 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowImpl(NodeId deviceI } mSetupPayload.version = 0; - mSetupPayload.discriminator.SetLongValue(discriminator); + mSetupPayload.discriminator.SetLongValue(params.discriminator); mSetupPayload.rendezvousInformation.SetValue(RendezvousInformationFlag::kOnNetwork); - mCommissioningWindowCallback = callback; + mCommissioningWindowCallback = params.callback; mBasicCommissioningWindowCallback = nullptr; - mNodeId = deviceId; - mEndpointId = endpointId; - mCommissioningWindowTimeout = timeout; - mPBKDFIterations = iteration; + mNodeId = params.deviceId; + mTargetEndpointId = params.endpointId; + mCommissioningWindowTimeout = params.timeout; + mPBKDFIterations = params.iteration; - bool randomSetupPIN = !setupPIN.HasValue(); + bool randomSetupPIN = !params.setupPIN.HasValue(); ReturnErrorOnFailure( PASESession::GeneratePASEVerifier(mVerifier, mPBKDFIterations, mPBKDFSalt, randomSetupPIN, mSetupPayload.setUpPINCode)); @@ -153,7 +153,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowInternal(Messaging: { ChipLogProgress(Controller, "OpenCommissioningWindow for device ID 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId)); - ClusterBase cluster(exchangeMgr, sessionHandle, mEndpointId); + ClusterBase cluster(exchangeMgr, sessionHandle, mTargetEndpointId); if (mCommissioningWindowOption != CommissioningWindowOption::kOriginalSetupCode) { diff --git a/src/controller/CommissioningWindowOpener.h b/src/controller/CommissioningWindowOpener.h index 16f84193925b1a..a6f32332d8a22f 100644 --- a/src/controller/CommissioningWindowOpener.h +++ b/src/controller/CommissioningWindowOpener.h @@ -42,6 +42,21 @@ typedef void (*OnOpenBasicCommissioningWindow)(void * context, NodeId deviceId, class CommissioningWindowOpener { public: + struct CommissioningWindowParams + { + NodeId deviceId; // The device Id of the node. + EndpointId endpointId; // The endpoint Id of the node. + System::Clock::Seconds16 timeout; // The duration for which the commissioning window should remain open. + uint32_t iteration; // The PAKE iteration count associated with the PAKE Passcode ID and ephemeral PAKE passcode + // verifier to be used for this commissioning. + uint16_t discriminator; // The long discriminator for the DNS-SD advertisement. + Optional setupPIN; // The setup PIN to use, or NullOptional to use a randomly-generated one. + Optional salt; // The salt to use, or NullOptional to use a randomly-generated one. If provided, must be at least + // kSpake2p_Min_PBKDF_Salt_Length bytes and at most kSpake2p_Max_PBKDF_Salt_Length bytes in length. + Callback::Callback * + callback; // The function to be called on success or failure of opening the commissioning window. + }; + CommissioningWindowOpener(DeviceController * controller) : mController(controller), mDeviceConnected(&OnDeviceConnectedCallback, this), mDeviceConnectionFailure(&OnDeviceConnectionFailureCallback, this) @@ -109,36 +124,15 @@ class CommissioningWindowOpener /** * @brief - * Try to look up the bridged device on the bridge with the given node id - * of the bridge and endpoint id of the bridged device and ask it to re-enter - * commissioning mode with a PASE verifier derived from the given information - * and the given discriminator. The bridged device will exit commissioning mode - * after a successful commissioning, or after the given `timeout` time. + * This method sends an OpenCommissioningWindow command with the given parameters + * to the specified endpoint of the given node. * - * @param[in] deviceId The device Id of the bridge. - * @param[in] endpoinId The endpoint Id of the bridged node on the bridge.* - * @param[in] timeout The commissioning mode should terminate after this much time. - * @param[in] iteration The PAKE iteration count associated with the PAKE Passcode ID and ephemeral - * PAKE passcode verifier to be used for this commissioning. - * @param[in] discriminator The long discriminator for the DNS-SD advertisement. - * @param[in] setupPIN The setup PIN to use, or NullOptional to use a randomly-generated one. - * @param[in] salt The salt to use, or NullOptional to use a - * randomly-generated one. If provided, must be at - * least kSpake2p_Min_PBKDF_Salt_Length bytes and - * at most kSpake2p_Max_PBKDF_Salt_Length bytes in - * length. - * @param[in] callback The function to be called on success or failure of opening of commissioning window. - * @param[out] payload The setup payload, not including the VID/PID bits, - * even if those were asked for, that is generated - * based on the passed-in information. The payload - * provided to the callback function, unlike this - * out parameter, will include the VID/PID bits if - * readVIDPIDAttributes is true. + * @param[in] params The parameters required to open the commissioning window. + * @param[out] payload The generated setup payload. * + * @return CHIP_ERROR Returns a CHIP_ERROR indicating the status of the operation. */ - CHIP_ERROR OpenCommissioningWindow(NodeId deviceId, EndpointId endpointId, System::Clock::Seconds16 timeout, uint32_t iteration, - uint16_t discriminator, Optional setupPIN, Optional salt, - Callback::Callback * callback, SetupPayload & payload); + CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowParams & params, SetupPayload & payload); private: enum class Step : uint8_t @@ -153,10 +147,8 @@ class CommissioningWindowOpener kOpenCommissioningWindow, }; - CHIP_ERROR OpenCommissioningWindowImpl(NodeId deviceId, EndpointId endpointId, System::Clock::Seconds16 timeout, - uint32_t iteration, uint16_t discriminator, Optional setupPIN, - Optional salt, Callback::Callback * callback, - SetupPayload & payload, bool readVIDPIDAttributes); + CHIP_ERROR OpenCommissioningWindowImpl(const CommissioningWindowParams & params, SetupPayload & payload, + bool readVIDPIDAttributes); CHIP_ERROR OpenCommissioningWindowInternal(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle); static void OnPIDReadResponse(void * context, uint16_t value); static void OnVIDReadResponse(void * context, VendorId value); @@ -174,7 +166,7 @@ class CommissioningWindowOpener Callback::Callback * mBasicCommissioningWindowCallback = nullptr; SetupPayload mSetupPayload; NodeId mNodeId = kUndefinedNodeId; - EndpointId mTargetEndpointId = kInvalidEndpointId; + EndpointId mTargetEndpointId = kInvalidEndpointId; System::Clock::Seconds16 mCommissioningWindowTimeout = System::Clock::kZero; CommissioningWindowOption mCommissioningWindowOption = CommissioningWindowOption::kOriginalSetupCode; Crypto::Spake2pVerifier mVerifier; // Used for non-basic commissioning.