address review comments

project-chip · Aug 3, 2021 · 2618857 · 2618857
1 parent 1c79ba1
commit 2618857
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 15 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
@@ -53,8 +53,8 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(chi
     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);
+    memcpy(verifier.mW0, &verifierData[0], kSpake2p_WS_Length);
+    memcpy(verifier.mL, &verifierData[kSpake2p_WS_Length], kSpake2p_WS_Length);
 
     VerifyOrExit(OpenPairingWindowUsingVerifier(commissioningTimeout, discriminator, verifier, iterations, salt, passcodeID) ==
                      CHIP_NO_ERROR,

diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp
@@ -358,8 +358,10 @@ void Device::OnOpenPairingWindowFailureResponse(void * context, uint8_t status)
 
 CHIP_ERROR Device::OpenPairingWindow(uint16_t timeout, PairingWindowOption option, SetupPayload & setupPayload)
 {
+    constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0;
+
     chip::Controller::AdministratorCommissioningCluster cluster;
-    cluster.Associate(this, 0);
+    cluster.Associate(this, kAdministratorCommissioningClusterEndpoint);
 
     Callback::Cancelable * successCallback = mOpenPairingSuccessCallback.Cancel();
     Callback::Cancelable * failureCallback = mOpenPairingFailureCallback.Cancel();
@@ -372,9 +374,9 @@ CHIP_ERROR Device::OpenPairingWindow(uint16_t timeout, PairingWindowOption optio
         ReturnErrorOnFailure(
             PASESession::GeneratePASEVerifier(verifier, kPBKDFMinimumIterations, salt, randomSetupPIN, setupPayload.setUpPINCode));
 
-        ReturnErrorOnFailure(
-            cluster.OpenCommissioningWindow(successCallback, failureCallback, timeout, ByteSpan(&verifier[0][0], sizeof(verifier)),
-                                            setupPayload.discriminator, kPBKDFMinimumIterations, salt, mPAKEVerifierID++));
+        ReturnErrorOnFailure(cluster.OpenCommissioningWindow(
+            successCallback, failureCallback, timeout, ByteSpan(reinterpret_cast<uint8_t *>(&verifier), sizeof(verifier)),
+            setupPayload.discriminator, kPBKDFMinimumIterations, salt, mPAKEVerifierID++));
     }
     else
     {

diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp
@@ -80,7 +80,7 @@ void PASESession::Clear()
     // This function zeroes out and resets the memory used by the object.
     // It's done so that no security related information will be leaked.
     memset(&mPoint[0], 0, sizeof(mPoint));
-    memset(&mPASEVerifier[0][0], 0, sizeof(mPASEVerifier));
+    memset(&mPASEVerifier, 0, sizeof(mPASEVerifier));
     memset(&mKe[0], 0, sizeof(mKe));
     mNextExpectedMsg = Protocols::SecureChannel::MsgType::PASE_Spake2pError;
 
@@ -209,7 +209,7 @@ CHIP_ERROR PASESession::ComputePASEVerifier(uint32_t setUpPINCode, uint32_t pbkd
     Encoding::LittleEndian::Put32(littleEndianSetupPINCode, setUpPINCode);
 
     return mPBKDF.pbkdf2_sha256(littleEndianSetupPINCode, sizeof(littleEndianSetupPINCode), salt.data(), salt.size(),
-                                pbkdf2IterCount, sizeof(PASEVerifier), &verifier[0][0]);
+                                pbkdf2IterCount, sizeof(PASEVerifier), reinterpret_cast<uint8_t *>(&verifier));
 }
 
 CHIP_ERROR PASESession::GeneratePASEVerifier(PASEVerifier & verifier, uint32_t pbkdf2IterCount, const ByteSpan & salt,
@@ -292,7 +292,7 @@ CHIP_ERROR PASESession::WaitForPairing(const PASEVerifier & verifier, uint32_t p
 {
     ReturnErrorOnFailure(WaitForPairing(0, pbkdf2IterCount, salt, myKeyId, delegate));
 
-    memmove(&mPASEVerifier, verifier, sizeof(verifier));
+    memmove(&mPASEVerifier, &verifier, sizeof(verifier));
     mComputeVerifier = false;
     mPasscodeID      = passcodeID;
 
@@ -427,7 +427,7 @@ CHIP_ERROR PASESession::SendPBKDFParamResponse()
     // Update commissioning hash with the pbkdf2 param response that's being sent.
     ReturnErrorOnFailure(mCommissioningHash.AddData(ByteSpan{ resp->Start(), resp->DataLength() }));
     ReturnErrorOnFailure(SetupSpake2p(mIterationCount, ByteSpan(mSalt, mSaltLength)));
-    ReturnErrorOnFailure(mSpake2p.ComputeL(mPoint, &sizeof_point, &mPASEVerifier[1][0], kSpake2p_WS_Length));
+    ReturnErrorOnFailure(mSpake2p.ComputeL(mPoint, &sizeof_point, mPASEVerifier.mL, kSpake2p_WS_Length));
 
     mNextExpectedMsg = Protocols::SecureChannel::MsgType::PASE_Spake2p1;
 
@@ -492,8 +492,8 @@ CHIP_ERROR PASESession::SendMsg1()
     uint8_t X[kMAX_Point_Length];
     size_t X_len = sizeof(X);
 
-    ReturnErrorOnFailure(mSpake2p.BeginProver(nullptr, 0, nullptr, 0, &mPASEVerifier[0][0], kSpake2p_WS_Length,
-                                              &mPASEVerifier[1][0], kSpake2p_WS_Length));
+    ReturnErrorOnFailure(
+        mSpake2p.BeginProver(nullptr, 0, nullptr, 0, mPASEVerifier.mW0, kSpake2p_WS_Length, mPASEVerifier.mL, kSpake2p_WS_Length));
 
     ReturnErrorOnFailure(mSpake2p.ComputeRoundOne(NULL, 0, X, &X_len));
 
@@ -535,7 +535,7 @@ CHIP_ERROR PASESession::HandleMsg1_and_SendMsg2(const System::PacketBufferHandle
     VerifyOrExit(buf != nullptr, err = CHIP_ERROR_MESSAGE_INCOMPLETE);
     VerifyOrExit(buf_len == sizeof(encryptionKeyId) + kMAX_Point_Length, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH);
 
-    err = mSpake2p.BeginVerifier(nullptr, 0, nullptr, 0, &mPASEVerifier[0][0], kSpake2p_WS_Length, mPoint, sizeof(mPoint));
+    err = mSpake2p.BeginVerifier(nullptr, 0, nullptr, 0, mPASEVerifier.mW0, kSpake2p_WS_Length, mPoint, sizeof(mPoint));
     SuccessOrExit(err);
 
     encryptionKeyId = chip::Encoding::LittleEndian::Read16(buf);

diff --git a/src/protocols/secure_channel/PASESession.h b/src/protocols/secure_channel/PASESession.h
@@ -74,7 +74,11 @@ struct PASESessionSerializable
     uint16_t mPeerKeyId;
 };
 
-typedef uint8_t PASEVerifier[2][kSpake2p_WS_Length];
+struct PASEVerifier
+{
+    uint8_t mW0[kSpake2p_WS_Length];
+    uint8_t mL[kSpake2p_WS_Length];
+};
 
 class DLL_EXPORT PASESession : public Messaging::ExchangeDelegate, public PairingSession
 {

diff --git a/src/protocols/secure_channel/RendezvousParameters.h b/src/protocols/secure_channel/RendezvousParameters.h
@@ -106,7 +106,7 @@ class RendezvousParameters
     const PASEVerifier & GetPASEVerifier() const { return mPASEVerifier; }
     RendezvousParameters & SetPASEVerifier(PASEVerifier & verifier)
     {
-        memmove(mPASEVerifier, verifier, sizeof(verifier));
+        memmove(&mPASEVerifier, &verifier, sizeof(verifier));
         mHasPASEVerifier = true;
         return *this;
     }