Skip to content

Commit

Permalink
[Fabric-Admin] Fix 'fabricsync sync-device' command always fails (#34938
Browse files Browse the repository at this point in the history
)

* [Fabric-Admin] Fix 'fabricsync sync-device' command always fails

* Use default random salt

* Update examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h

Co-authored-by: Abdul Samad <abdul.samad@smartthings.com>

* Address review comments

---------

Co-authored-by: Abdul Samad <abdul.samad@smartthings.com>
  • Loading branch information
yufengwangca and samadDotDev authored Aug 13, 2024
1 parent 74b8ce1 commit 128c37a
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 6 deletions.
2 changes: 1 addition & 1 deletion examples/fabric-admin/commands/common/CHIPCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ inline constexpr char kIdentityGamma[] = "gamma";
// (CASE) communcation.
inline constexpr char kIdentityNull[] = "null-fabric-commissioner";

constexpr uint16_t kMaxCommandSize = 128;
constexpr uint16_t kMaxCommandSize = 384;

class CHIPCommand : public Command
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ CHIP_ERROR FabricSyncRemoveBridgeCommand::RunCommand()

void FabricSyncDeviceCommand::OnCommissioningWindowOpened(NodeId deviceId, CHIP_ERROR err, chip::SetupPayload payload)
{
ChipLogProgress(NotSpecified, "FabricSyncDeviceCommand::OnCommissioningWindowOpened");

if (err == CHIP_NO_ERROR)
{
char payloadBuffer[kMaxManaulCodeLength + 1];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand()
.SetTimeout(mCommissioningWindowTimeout)
.SetIteration(mIteration)
.SetDiscriminator(mDiscriminator)
.SetSetupPIN(mSetupPIN)
.SetSalt(mSalt)
.SetReadVIDPIDAttributes(true)
.SetCallback(&mOnOpenCommissioningWindowCallback),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ class OpenCommissioningWindowCommand : public CHIPCommand
"Time, in seconds, before the commissioning window closes.");
AddArgument("iteration", chip::Crypto::kSpake2p_Min_PBKDF_Iterations, chip::Crypto::kSpake2p_Max_PBKDF_Iterations,
&mIteration, "Number of PBKDF iterations to use to derive the verifier. Ignored if 'option' is 0.");
AddArgument("discriminator", 0, 4096, &mDiscriminator, "Discriminator to use for advertising. Ignored if 'option' is 0.");
AddArgument("discriminator", 0, 4095, &mDiscriminator, "Discriminator to use for advertising. Ignored if 'option' is 0.");
AddArgument("timeout", 0, UINT16_MAX, &mTimeout, "Time, in seconds, before this command is considered to have timed out.");
AddArgument("setup-pin", 1, chip::kSetupPINCodeMaximumValue, &mSetupPIN, "The setup PIN (Passcode) to use.");
AddArgument("salt", &mSalt,
"Salt payload encoded in hexadecimal. Random salt will be generated if absent. "
"This needs to be present if verifier is provided, corresponding to salt used for generating verifier");
Expand Down Expand Up @@ -76,6 +77,7 @@ class OpenCommissioningWindowCommand : public CHIPCommand
uint16_t mDiscriminator;

chip::Optional<uint16_t> mTimeout;
chip::Optional<uint32_t> mSetupPIN;
chip::Optional<chip::ByteSpan> mSalt;
chip::Optional<chip::ByteSpan> mVerifier;

Expand Down
13 changes: 9 additions & 4 deletions examples/fabric-admin/device_manager/DeviceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ namespace {
// Constants
constexpr uint32_t kSetupPinCode = 20202021;
constexpr uint16_t kRemoteBridgePort = 5540;
constexpr uint16_t kDiscriminator = 3840;
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;

} // namespace
Expand Down Expand Up @@ -118,7 +118,7 @@ void DeviceManager::OpenDeviceCommissioningWindow(NodeId nodeId, uint32_t commis
uint32_t discriminator, const char * saltHex, const char * verifierHex)
{
// Open the commissioning window of a device within its own fabric.
StringBuilder<512> commandBuilder;
StringBuilder<kMaxCommandSize> commandBuilder;

commandBuilder.Add("pairing open-commissioning-window ");
commandBuilder.AddFormat("%lu %d %d %d %d %d --salt hex:%s --verifier hex:%s", nodeId, kRootEndpointId,
Expand All @@ -132,11 +132,16 @@ 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<kMaxCommandSize> commandBuilder;
StringBuilder<512> commandBuilder;

// Use random discriminator to have less chance of collission.
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, kDiscriminator);
kWindowTimeout, kIteration, discriminator);
commandBuilder.Add(" --setup-pin 20202021");

PushCommand(commandBuilder.c_str());
}
Expand Down

0 comments on commit 128c37a

Please sign in to comment.