Skip to content

Commit

Permalink
Detect OpenCommissioningWindow errors in controller apps (#10918)
Browse files Browse the repository at this point in the history
* Detect OpenCommissioningWindow errors in controller apps

* fix build

* address review comments
  • Loading branch information
pan-apple authored and pull[bot] committed Dec 1, 2021
1 parent ff9b4c2 commit 5270877
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 11 deletions.
17 changes: 14 additions & 3 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<PairingCommand *>(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)
Expand Down
6 changes: 5 additions & 1 deletion examples/chip-tool/commands/pairing/PairingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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<chip::Controller::OnDeviceConnected> mOnDeviceConnectedCallback;
chip::Callback::Callback<chip::Controller::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
chip::Callback::Callback<chip::Controller::OnOpenCommissioningWindow> mOnOpenCommissioningWindowCallback;
};
26 changes: 24 additions & 2 deletions src/controller/CHIPDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Device *>(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<Device *>(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,
Expand All @@ -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<OnOpenCommissioningWindow> * callback,
SetupPayload & setupPayload)
{
constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0;

Expand All @@ -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);
Expand All @@ -361,14 +381,16 @@ CHIP_ERROR Device::OpenCommissioningWindow(uint16_t timeout, uint32_t iteration,
setupPayload.version = 0;
setupPayload.rendezvousInformation = RendezvousInformationFlags(RendezvousInformationFlag::kOnNetwork);

mSetupPayload = setupPayload;

return CHIP_NO_ERROR;
}

CHIP_ERROR Device::OpenPairingWindow(uint16_t timeout, CommissioningWindowOption option, SetupPayload & setupPayload)
{
ByteSpan salt(reinterpret_cast<const uint8_t *>(kSpake2pKeyExchangeSalt), strlen(kSpake2pKeyExchangeSalt));

return OpenCommissioningWindow(timeout, kPBKDFMinimumIterations, option, salt, setupPayload);
return OpenCommissioningWindow(timeout, kPBKDFMinimumIterations, option, salt, nullptr, setupPayload);
}

void Device::UpdateSession(bool connected)
Expand Down
8 changes: 7 additions & 1 deletion src/controller/CHIPDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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<OnOpenCommissioningWindow> * callback,
SetupPayload & setupPayload);

/**
* @brief
Expand Down Expand Up @@ -579,6 +582,9 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
Callback::CallbackDeque mConnectionSuccess;
Callback::CallbackDeque mConnectionFailure;

Callback::Callback<OnOpenCommissioningWindow> * mCommissioningWindowCallback = nullptr;
SetupPayload mSetupPayload;

Callback::Callback<DefaultSuccessCallback> mOpenPairingSuccessCallback;
Callback::Callback<DefaultFailureCallback> mOpenPairingFailureCallback;
};
Expand Down
7 changes: 4 additions & 3 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<OnOpenCommissioningWindow> * callback)
{
ChipLogProgress(Controller, "OpenCommissioningWindow for device ID %" PRIu64, deviceId);
VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -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)
{
Expand Down
25 changes: 24 additions & 1 deletion src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<OnOpenCommissioningWindow> * callback);

//////////// SessionEstablishmentDelegate Implementation ///////////////
void OnSessionEstablishmentError(CHIP_ERROR error) override;
Expand Down
9 changes: 9 additions & 0 deletions src/lib/core/CHIPError.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)

/**
* @}
*/
Expand Down

0 comments on commit 5270877

Please sign in to comment.