Fix discriminator handling in chip-tool pairing commands.

Not all commands initialize the discriminator value, and the ones that don't should not use it. Fixes project-chip#34096
bzbarsky-apple · Jun 27, 2024 · 0e8a13f · 0e8a13f
1 parent dd0c49b
commit 0e8a13f
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 5 deletions.
diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp
@@ -212,7 +212,12 @@ CHIP_ERROR PairingCommand::PairWithCode(NodeId remoteId)
 
 CHIP_ERROR PairingCommand::Pair(NodeId remoteId, PeerAddress address)
 {
-    auto params = RendezvousParameters().SetSetupPINCode(mSetupPINCode).SetDiscriminator(mDiscriminator).SetPeerAddress(address);
+    VerifyOrDieWithMsg(mSetupPINCodeUsable, chipTool, "Using mSetupPINCode in a mode when we have not gotten one");
+    auto params = RendezvousParameters().SetSetupPINCode(mSetupPINCode).SetPeerAddress(address);
+    if (mDiscriminatorForBLEAndSoftAPUsable)
+    {
+        params.SetDiscriminator(mDiscriminatorForBLEAndSoftAP);
+    }
 
     CHIP_ERROR err = CHIP_NO_ERROR;
     if (mPaseOnly.ValueOr(false))
@@ -232,6 +237,8 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndex(NodeId remoteId, uint16_t in
 #if CHIP_DEVICE_LAYER_TARGET_DARWIN
     VerifyOrReturnError(IsInteractive(), CHIP_ERROR_INCORRECT_STATE);
 
+    VerifyOrDieWithMsg(mSetupPINCodeUsable, chipTool, "Using mSetupPINCode in a mode when we have not gotten one");
+
     RendezvousParameters params;
     ReturnErrorOnFailure(GetDeviceScanner().Get(index, params));
     params.SetSetupPINCode(mSetupPINCode);
@@ -254,6 +261,10 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndex(NodeId remoteId, uint16_t in
 
 CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndexWithCode(NodeId remoteId, uint16_t index)
 {
+    // We might or might not have a setup code.  We don't know yet, but if we
+    // do, we will set this boolean true at that point.
+    mSetupPINCodeUsable = false;
+
 #if CHIP_DEVICE_LAYER_TARGET_DARWIN
     VerifyOrReturnError(IsInteractive(), CHIP_ERROR_INCORRECT_STATE);
 
@@ -277,7 +288,8 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndexWithCode(NodeId remoteId, uin
             VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT);
         }
 
-        mSetupPINCode = payload.setUpPINCode;
+        mSetupPINCode       = payload.setUpPINCode;
+        mSetupPINCodeUsable = true;
         return PairWithMdnsOrBleByIndex(remoteId, index);
     }
 

diff --git a/examples/chip-tool/commands/pairing/PairingCommand.h b/examples/chip-tool/commands/pairing/PairingCommand.h
@@ -102,33 +102,40 @@ class PairingCommand : public CHIPCommand,
         case PairingMode::Ble:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
             AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
-            AddArgument("discriminator", 0, 4096, &mDiscriminator);
+            AddArgument("discriminator", 0, 4096, &mDiscriminatorForBLEAndSoftAP);
+            mSetupPINCodeUsable                 = true;
+            mDiscriminatorForBLEAndSoftAPUsable = true;
             break;
         case PairingMode::OnNetwork:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
             AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
             AddArgument("pase-only", 0, 1, &mPaseOnly);
+            mSetupPINCodeUsable = true;
             break;
         case PairingMode::SoftAP:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
             AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
-            AddArgument("discriminator", 0, 4096, &mDiscriminator);
+            AddArgument("discriminator", 0, 4096, &mDiscriminatorForBLEAndSoftAP);
             AddArgument("device-remote-ip", &mRemoteAddr);
             AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
             AddArgument("pase-only", 0, 1, &mPaseOnly);
+            mSetupPINCodeUsable                 = true;
+            mDiscriminatorForBLEAndSoftAPUsable = true;
             break;
         case PairingMode::AlreadyDiscovered:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
             AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
             AddArgument("device-remote-ip", &mRemoteAddr);
             AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
             AddArgument("pase-only", 0, 1, &mPaseOnly);
+            mSetupPINCodeUsable = true;
             break;
         case PairingMode::AlreadyDiscoveredByIndex:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
             AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
             AddArgument("index", 0, UINT16_MAX, &mIndex);
             AddArgument("pase-only", 0, 1, &mPaseOnly);
+            mSetupPINCodeUsable = true;
             break;
         case PairingMode::AlreadyDiscoveredByIndexWithCode:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
@@ -246,7 +253,7 @@ class PairingCommand : public CHIPCommand,
         mComplex_DSTOffsets;
 
     uint16_t mRemotePort;
-    uint16_t mDiscriminator;
+    uint16_t mDiscriminatorForBLEAndSoftAP;
     uint32_t mSetupPINCode;
     uint16_t mIndex;
     chip::ByteSpan mOperationalDataset;
@@ -257,6 +264,14 @@ class PairingCommand : public CHIPCommand,
     char * mDiscoveryFilterInstanceName;
 
     bool mDeviceIsICD;
+    // We only use mDiscriminatorForBLEAndSoftAP in some modes, but in those
+    // modes it's required (so we can't use an Optional for it).  Use an
+    // out-of-band boolean to keep track of whether that field is valid at all.
+    bool mDiscriminatorForBLEAndSoftAPUsable = false;
+    // We only use mSetupPINCode in some modes,  but in those
+    // modes it's required (so we can't use an Optional for it).  Use an
+    // out-of-band boolean to keep track of whether that field is valid at all.
+    bool mSetupPINCodeUsable = false;
     uint8_t mRandomGeneratedICDSymmetricKey[chip::Crypto::kAES_CCM128_Key_Length];
 
     // For unpair