Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@yufengwangca
yufengwangca committed Jun 11, 2024
1 parent 8b4756e commit 5d735ec
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 68 deletions.
72 changes: 36 additions & 36 deletions src/controller/CommissioningWindowOpener.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -60,58 +60,58 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, S
Callback::Callback<OnOpenCommissioningWindow> * 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<uint32_t> setupPIN, Optional<ByteSpan> salt,
Callback::Callback<OnOpenCommissioningWindow> * 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<uint32_t> setupPIN, Optional<ByteSpan> salt,
Callback::Callback<OnOpenCommissioningWindow> * 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
{
Expand All @@ -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));

Expand All @@ -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)
{
Expand Down
56 changes: 24 additions & 32 deletions src/controller/CommissioningWindowOpener.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint32_t> setupPIN; // The setup PIN to use, or NullOptional to use a randomly-generated one.
Optional<ByteSpan> 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<OnOpenCommissioningWindow> *
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)
Expand Down Expand Up @@ -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<uint32_t> setupPIN, Optional<ByteSpan> salt,
Callback::Callback<OnOpenCommissioningWindow> * callback, SetupPayload & payload);
CHIP_ERROR OpenCommissioningWindow(const CommissioningWindowParams & params, SetupPayload & payload);

private:
enum class Step : uint8_t
Expand All @@ -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<uint32_t> setupPIN,
Optional<ByteSpan> salt, Callback::Callback<OnOpenCommissioningWindow> * 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);
Expand All @@ -174,7 +166,7 @@ class CommissioningWindowOpener
Callback::Callback<OnOpenBasicCommissioningWindow> * 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.
Expand Down

0 comments on commit 5d735ec

Please sign in to comment.