address review comments

project-chip · Jul 30, 2021 · e7cdccd · e7cdccd
1 parent 9ffa15e
commit e7cdccd
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 3 deletions.
diff --git a/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp b/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp
@@ -23,11 +23,15 @@
 #include <app/CommandHandler.h>
 #include <app/server/Server.h>
 #include <app/util/af.h>
+#include <setup_payload/SetupPayload.h>
 #include <support/CodeUtils.h>
 #include <support/logging/CHIPLogging.h>
 
 using namespace chip;
 
+// Specifications section 5.4.2.3. Announcement Duration
+constexpr uint32_t kMaxCommissionioningTimeoutSeconds = 15 * 60;
+
 bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(chip::EndpointId endpoint,
                                                                              app::CommandHandler * commandObj,
                                                                              uint16_t commissioningTimeout, ByteSpan pakeVerifier,
@@ -36,10 +40,22 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(chi
 {
     EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
     PASEVerifier verifier;
+    const uint8_t * verifierData = pakeVerifier.data();
+
     ChipLogProgress(Zcl, "Received command to open commissioning window");
+
     VerifyOrExit(!IsPairingWindowOpen(), status = EMBER_ZCL_STATUS_FAILURE);
     VerifyOrExit(sizeof(verifier) == pakeVerifier.size(), status = EMBER_ZCL_STATUS_FAILURE);
-    memcpy(&verifier[0][0], pakeVerifier.data(), pakeVerifier.size());
+    VerifyOrExit(iterations >= kPBKDFMinimumIterations, status = EMBER_ZCL_STATUS_FAILURE);
+    VerifyOrExit(iterations <= kPBKDFMaximumIterations, status = EMBER_ZCL_STATUS_FAILURE);
+    VerifyOrExit(salt.size() >= kPBKDFMinimumSaltLen, status = EMBER_ZCL_STATUS_FAILURE);
+    VerifyOrExit(salt.size() <= kPBKDFMaximumSaltLen, status = EMBER_ZCL_STATUS_FAILURE);
+    VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds, status = EMBER_ZCL_STATUS_FAILURE);
+    VerifyOrExit(discriminator <= kMaxDiscriminatorValue, status = EMBER_ZCL_STATUS_FAILURE);
+
+    memcpy(&verifier[0][0], &verifierData[0], kSpake2p_WS_Length);
+    memcpy(&verifier[1][0], &verifierData[kSpake2p_WS_Length], kSpake2p_WS_Length);
+
     VerifyOrExit(OpenPairingWindowUsingVerifier(commissioningTimeout, discriminator, verifier, iterations, salt, passcodeID) ==
                      CHIP_NO_ERROR,
                  status = EMBER_ZCL_STATUS_FAILURE);
@@ -61,6 +77,7 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
     EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
     ChipLogProgress(Zcl, "Received command to open basic commissioning window");
     VerifyOrExit(!IsPairingWindowOpen(), status = EMBER_ZCL_STATUS_FAILURE);
+    VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds, status = EMBER_ZCL_STATUS_FAILURE);
     VerifyOrExit(OpenDefaultPairingWindow(ResetFabrics::kNo, commissioningTimeout) == CHIP_NO_ERROR,
                  status = EMBER_ZCL_STATUS_FAILURE);
     ChipLogProgress(Zcl, "Commissioning window is now open");

diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp
@@ -370,11 +370,11 @@ CHIP_ERROR Device::OpenPairingWindow(uint16_t timeout, PairingWindowOption optio
         PASEVerifier verifier;
         ByteSpan salt(reinterpret_cast<const uint8_t *>(kSpake2pKeyExchangeSalt), strlen(kSpake2pKeyExchangeSalt));
         ReturnErrorOnFailure(
-            PASESession::GeneratePASEVerifier(verifier, kSpake2p_Iteration_Count, salt, randomSetupPIN, setupPayload.setUpPINCode));
+            PASESession::GeneratePASEVerifier(verifier, kPBKDFMinimumIterations, salt, randomSetupPIN, setupPayload.setUpPINCode));
 
         ReturnErrorOnFailure(
             cluster.OpenCommissioningWindow(successCallback, failureCallback, timeout, ByteSpan(&verifier[0][0], sizeof(verifier)),
-                                            setupPayload.discriminator, kSpake2p_Iteration_Count, salt, mPAKEVerifierID++));
+                                            setupPayload.discriminator, kPBKDFMinimumIterations, salt, mPAKEVerifierID++));
     }
     else
     {

diff --git a/src/protocols/secure_channel/PASESession.h b/src/protocols/secure_channel/PASESession.h
@@ -53,6 +53,12 @@ extern const char * kSpake2pKeyExchangeSalt;
 constexpr uint16_t kPBKDFParamRandomNumberSize = 32;
 constexpr uint32_t kSpake2p_Iteration_Count    = 100;
 
+// Specifications section 3.9. Password-Based Key Derivation Function
+constexpr uint32_t kPBKDFMinimumIterations = 1000;
+constexpr uint32_t kPBKDFMaximumIterations = 100000;
+constexpr uint32_t kPBKDFMinimumSaltLen    = 16;
+constexpr uint32_t kPBKDFMaximumSaltLen    = 32;
+
 using namespace Crypto;
 
 constexpr size_t kSpake2p_WS_Length = kP256_FE_Length + 8;