Address review comments on PR 22205. (#22433)

Stops unnecessarily using CHIPDeviceController instances. Stops manually serializing verifiers when we have utilities for that. Fixes #22205
project-chip · Jul 14, 2023 · 1239054 · 1239054
1 parent 6b037f7
commit 1239054
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 22 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.h b/src/darwin/Framework/CHIP/MTRDeviceController.h
@@ -148,8 +148,11 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS
  * @param[in] setupPincode    The desired PIN code to use
  * @param[in] iterations      The number of iterations to use when generating the verifier
  * @param[in] salt            The 16-byte salt for verifier computation
+ *
+ * Returns nil on errors (e.g. salt has the wrong size), otherwise the computed
+ * verifier bytes.
  */
-- (nullable NSData *)computePaseVerifier:(uint32_t)setupPincode iterations:(uint32_t)iterations salt:(NSData *)salt;
++ (nullable NSData *)computePaseVerifier:(uint32_t)setupPincode iterations:(uint32_t)iterations salt:(NSData *)salt;
 
 /**
  * Shutdown the controller. Calls to shutdown after the first one are NO-OPs.

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -696,32 +696,24 @@ - (void)setNocChainIssuer:(id<MTRNOCChainIssuer>)nocChainIssuer queue:(dispatch_
     });
 }
 
-- (nullable NSData *)computePaseVerifier:(uint32_t)setupPincode iterations:(uint32_t)iterations salt:(NSData *)salt
++ (nullable NSData *)computePaseVerifier:(uint32_t)setupPincode iterations:(uint32_t)iterations salt:(NSData *)salt
 {
-    __block CHIP_ERROR errorCode = CHIP_ERROR_INCORRECT_STATE;
-    if (![self isRunning]) {
-        [self checkForError:errorCode logMsg:kErrorNotRunning error:nil];
+    chip::Spake2pVerifier verifier;
+    CHIP_ERROR err = verifier.Generate(iterations, AsByteSpan(salt), setupPincode);
+    if (err != CHIP_NO_ERROR) {
+        MTR_LOG_ERROR("computePaseVerifier generation failed: %s", chip::ErrorStr(err));
         return nil;
     }
 
-    __block NSData * result;
-    __block chip::Spake2pVerifier paseVerifier;
-    __block chip::ByteSpan saltByteSpan = chip::ByteSpan(static_cast<const uint8_t *>(salt.bytes), salt.length);
-
-    dispatch_sync(_chipWorkQueue, ^{
-        if ([self isRunning]) {
-            errorCode = self.cppCommissioner->ComputePASEVerifier(iterations, setupPincode, saltByteSpan, paseVerifier);
-            MTR_LOG_ERROR("ComputePaseVerifier: %s", chip::ErrorStr(errorCode));
-
-            uint8_t serializedVerifier[sizeof(paseVerifier.mW0) + sizeof(paseVerifier.mL)];
-            memcpy(serializedVerifier, paseVerifier.mW0, chip::kSpake2p_WS_Length);
-            memcpy(&serializedVerifier[sizeof(paseVerifier.mW0)], paseVerifier.mL, sizeof(paseVerifier.mL));
-
-            result = [NSData dataWithBytes:serializedVerifier length:sizeof(serializedVerifier)];
-        }
-    });
+    uint8_t serializedBuffer[chip::Crypto::kSpake2p_VerifierSerialized_Length];
+    chip::MutableByteSpan serializedBytes(serializedBuffer);
+    err = verifier.Serialize(serializedBytes);
+    if (err != CHIP_NO_ERROR) {
+        MTR_LOG_ERROR("computePaseVerifier serialization failed: %s", chip::ErrorStr(err));
+        return nil;
+    }
 
-    return result;
+    return AsData(serializedBytes);
 }
 
 - (nullable NSData *)fetchAttestationChallengeForDeviceId:(uint64_t)deviceId