From 3814419a9db7a7ad34f75b9a30962053e01fc8a3 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 27 Oct 2021 09:58:08 -0700 Subject: [PATCH] Detect OpenCommissioningWindow errors in controller apps (#10918) * Detect OpenCommissioningWindow errors in controller apps * fix build * address review comments --- .../commands/pairing/PairingCommand.cpp | 17 +++++++++--- .../commands/pairing/PairingCommand.h | 6 ++++- src/controller/CHIPDevice.cpp | 26 +++++++++++++++++-- src/controller/CHIPDevice.h | 8 +++++- src/controller/CHIPDeviceController.cpp | 7 ++--- src/controller/CHIPDeviceController.h | 25 +++++++++++++++++- src/lib/core/CHIPError.h | 9 +++++++ 7 files changed, 87 insertions(+), 11 deletions(-) diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp index 8e7c3b60d6156f..efd4581ca57211 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.cpp +++ b/examples/chip-tool/commands/pairing/PairingCommand.cpp @@ -155,11 +155,22 @@ CHIP_ERROR PairingCommand::Unpair(NodeId remoteId) return err; } +void PairingCommand::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err, chip::SetupPayload payload) +{ + PairingCommand * command = reinterpret_cast(context); + if (err != CHIP_NO_ERROR) + { + ChipLogError(chipTool, + "Failed in opening commissioning window on the device: 0x" ChipLogFormatX64 ", error %" CHIP_ERROR_FORMAT, + ChipLogValueX64(remoteId), err.Format()); + } + command->SetCommandExitStatus(err); +} + CHIP_ERROR PairingCommand::OpenCommissioningWindow() { - CHIP_ERROR err = mController.OpenCommissioningWindow(mNodeId, mTimeout, mIteration, mDiscriminator, mCommissioningWindowOption); - SetCommandExitStatus(err); - return err; + return mController.OpenCommissioningWindowWithCallback(mNodeId, mTimeout, mIteration, mDiscriminator, + mCommissioningWindowOption, &mOnOpenCommissioningWindowCallback); } void PairingCommand::OnStatusUpdate(DevicePairingDelegate::Status status) diff --git a/examples/chip-tool/commands/pairing/PairingCommand.h b/examples/chip-tool/commands/pairing/PairingCommand.h index 6fcf1a99c65930..c3c23cfffffefa 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.h +++ b/examples/chip-tool/commands/pairing/PairingCommand.h @@ -59,7 +59,9 @@ class PairingCommand : public CHIPCommand, CHIPCommand(commandName), mPairingMode(mode), mNetworkType(networkType), mFilterType(filterType), mRemoteAddr{ IPAddress::Any, chip::Inet::InterfaceId::Null() }, - mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this) + mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), + mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this), + mOnOpenCommissioningWindowCallback(OnOpenCommissioningWindowResponse, this) { AddArgument("node-id", 0, UINT64_MAX, &mNodeId); @@ -214,7 +216,9 @@ class PairingCommand : public CHIPCommand, static void OnDeviceConnectedFn(void * context, chip::Controller::Device * device); static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error); + static void OnOpenCommissioningWindowResponse(void * context, NodeId deviceId, CHIP_ERROR status, chip::SetupPayload payload); chip::Callback::Callback mOnDeviceConnectedCallback; chip::Callback::Callback mOnDeviceConnectionFailureCallback; + chip::Callback::Callback mOnOpenCommissioningWindowCallback; }; diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index 684b2fbf2fe919..a3f345f7083841 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -308,11 +308,29 @@ void Device::OnResponseTimeout(Messaging::ExchangeContext * ec) {} void Device::OnOpenPairingWindowSuccessResponse(void * context) { ChipLogProgress(Controller, "Successfully opened pairing window on the device"); + Device * device = reinterpret_cast(context); + if (device->mCommissioningWindowCallback != nullptr) + { + device->mCommissioningWindowCallback->mCall(device->mCommissioningWindowCallback->mContext, device->GetDeviceId(), + CHIP_NO_ERROR, device->mSetupPayload); + } } void Device::OnOpenPairingWindowFailureResponse(void * context, uint8_t status) { ChipLogError(Controller, "Failed to open pairing window on the device. Status %d", status); + Device * device = reinterpret_cast(context); + if (device->mCommissioningWindowCallback != nullptr) + { + CHIP_ERROR error = CHIP_ERROR_INVALID_PASE_PARAMETER; + // TODO - Use cluster enum chip::app::Clusters::AdministratorCommissioning::StatusCode::kBusy + if (status == 1) + { + error = CHIP_ERROR_ANOTHER_COMMISSIONING_IN_PROGRESS; + } + device->mCommissioningWindowCallback->mCall(device->mCommissioningWindowCallback->mContext, device->GetDeviceId(), error, + SetupPayload()); + } } CHIP_ERROR Device::ComputePASEVerifier(uint32_t iterations, uint32_t setupPincode, const ByteSpan & salt, @@ -325,7 +343,8 @@ CHIP_ERROR Device::ComputePASEVerifier(uint32_t iterations, uint32_t setupPincod } CHIP_ERROR Device::OpenCommissioningWindow(uint16_t timeout, uint32_t iteration, CommissioningWindowOption option, - const ByteSpan & salt, SetupPayload & setupPayload) + const ByteSpan & salt, Callback::Callback * callback, + SetupPayload & setupPayload) { constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0; @@ -335,6 +354,7 @@ CHIP_ERROR Device::OpenCommissioningWindow(uint16_t timeout, uint32_t iteration, Callback::Cancelable * successCallback = mOpenPairingSuccessCallback.Cancel(); Callback::Cancelable * failureCallback = mOpenPairingFailureCallback.Cancel(); + mCommissioningWindowCallback = callback; if (option != CommissioningWindowOption::kOriginalSetupCode) { bool randomSetupPIN = (option == CommissioningWindowOption::kTokenWithRandomPIN); @@ -361,6 +381,8 @@ CHIP_ERROR Device::OpenCommissioningWindow(uint16_t timeout, uint32_t iteration, setupPayload.version = 0; setupPayload.rendezvousInformation = RendezvousInformationFlags(RendezvousInformationFlag::kOnNetwork); + mSetupPayload = setupPayload; + return CHIP_NO_ERROR; } @@ -368,7 +390,7 @@ CHIP_ERROR Device::OpenPairingWindow(uint16_t timeout, CommissioningWindowOption { ByteSpan salt(reinterpret_cast(kSpake2pKeyExchangeSalt), strlen(kSpake2pKeyExchangeSalt)); - return OpenCommissioningWindow(timeout, kPBKDFMinimumIterations, option, salt, setupPayload); + return OpenCommissioningWindow(timeout, kPBKDFMinimumIterations, option, salt, nullptr, setupPayload); } void Device::UpdateSession(bool connected) diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h index 9b7cfed06c9d0d..d98ae89f63d3d1 100644 --- a/src/controller/CHIPDevice.h +++ b/src/controller/CHIPDevice.h @@ -103,6 +103,7 @@ class Device; typedef void (*OnDeviceConnected)(void * context, Device * device); typedef void (*OnDeviceConnectionFailure)(void * context, NodeId deviceId, CHIP_ERROR error); +typedef void (*OnOpenCommissioningWindow)(void * context, NodeId deviceId, CHIP_ERROR status, SetupPayload payload); class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEstablishmentDelegate { @@ -289,12 +290,14 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta * the PIN code provied in the setupPayload). * @param[in] salt The PAKE Salt associated with the PAKE Passcode ID and ephemeral PAKE passcode * verifier to be used for this commissioning. + * @param[in] callback The function to be called on success or failure of opening of commissioning window. * @param[out] setupPayload The setup payload corresponding to the generated onboarding token. * * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error */ CHIP_ERROR OpenCommissioningWindow(uint16_t timeout, uint32_t iteration, CommissioningWindowOption option, - const ByteSpan & salt, SetupPayload & setupPayload); + const ByteSpan & salt, Callback::Callback * callback, + SetupPayload & setupPayload); /** * @brief @@ -579,6 +582,9 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta Callback::CallbackDeque mConnectionSuccess; Callback::CallbackDeque mConnectionFailure; + Callback::Callback * mCommissioningWindowCallback = nullptr; + SetupPayload mSetupPayload; + Callback::Callback mOpenPairingSuccessCallback; Callback::Callback mOpenPairingFailureCallback; }; diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index fc2c66fc71cd66..71e213aeeeee1d 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -875,8 +875,9 @@ CHIP_ERROR DeviceCommissioner::OperationalDiscoveryComplete(NodeId remoteDeviceI return GetConnectedDevice(remoteDeviceId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); } -CHIP_ERROR DeviceCommissioner::OpenCommissioningWindow(NodeId deviceId, uint16_t timeout, uint16_t iteration, - uint16_t discriminator, uint8_t option) +CHIP_ERROR DeviceCommissioner::OpenCommissioningWindowWithCallback(NodeId deviceId, uint16_t timeout, uint16_t iteration, + uint16_t discriminator, uint8_t option, + Callback::Callback * callback) { ChipLogProgress(Controller, "OpenCommissioningWindow for device ID %" PRIu64, deviceId); VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); @@ -908,7 +909,7 @@ CHIP_ERROR DeviceCommissioner::OpenCommissioningWindow(NodeId deviceId, uint16_t return CHIP_ERROR_INVALID_ARGUMENT; } - ReturnErrorOnFailure(device->OpenCommissioningWindow(timeout, iteration, commissioningWindowOption, salt, payload)); + ReturnErrorOnFailure(device->OpenCommissioningWindow(timeout, iteration, commissioningWindowOption, salt, callback, payload)); if (commissioningWindowOption != Device::CommissioningWindowOption::kOriginalSetupCode) { diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 416238f3a1417c..d5920d5cfaa9bd 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -460,7 +460,30 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error */ CHIP_ERROR OpenCommissioningWindow(NodeId deviceId, uint16_t timeout, uint16_t iteration, uint16_t discriminator, - uint8_t option); + uint8_t option) + { + return OpenCommissioningWindowWithCallback(deviceId, timeout, iteration, discriminator, option, nullptr); + } + + /** + * @brief + * Trigger a paired device to re-enter the commissioning mode. The device will exit the commissioning mode + * after a successful commissioning, or after the given `timeout` time. + * + * @param[in] deviceId The device Id. + * @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] option The commissioning window can be opened using the original setup code, or an + * onboarding token can be generated using a random setup PIN code (or with + * the PIN code provied in the setupPayload). + * @param[in] callback The function to be called on success or failure of opening of commissioning window. + * + * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error + */ + CHIP_ERROR OpenCommissioningWindowWithCallback(NodeId deviceId, uint16_t timeout, uint16_t iteration, uint16_t discriminator, + uint8_t option, Callback::Callback * callback); //////////// SessionEstablishmentDelegate Implementation /////////////// void OnSessionEstablishmentError(CHIP_ERROR error) override; diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h index 5c07ad56e18601..2003bb0418ce2d 100644 --- a/src/lib/core/CHIPError.h +++ b/src/lib/core/CHIPError.h @@ -2207,6 +2207,15 @@ using CHIP_ERROR = ::chip::ChipError; */ #define CHIP_ERROR_IM_STATUS_CODE_RECEIVED CHIP_CORE_ERROR(0xca) +/* + * @def CHIP_ERROR_ANOTHER_COMMISSIONING_IN_PROGRESS + * + * @brief + * Indicates that the commissioning window on the device is already open, and another + * commissioning is in progress + */ +#define CHIP_ERROR_ANOTHER_COMMISSIONING_IN_PROGRESS CHIP_CORE_ERROR(0xcb) + /** * @} */