Refactoring random SetupPinCode generation into a method. (#36009)

* Creating a Function to generate random Setup Pin codes and making sure that the generated pin code is valid * Using generateRandomSetupPin function to generate random setup pin codes * Integrating comments * Handling return value of generateRandomSetupPin invocation Co-authored-by: Andrei Litvin <[email protected]> * Integrating comments --------- Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Oct 11, 2024 · 9690532 · 9690532
1 parent d49654b
commit 9690532
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 4 deletions.
diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp
@@ -139,10 +139,7 @@ CHIP_ERROR PASESession::GeneratePASEVerifier(Spake2pVerifier & verifier, uint32_
 
     if (useRandomPIN)
     {
-        ReturnErrorOnFailure(DRBG_get_bytes(reinterpret_cast<uint8_t *>(&setupPINCode), sizeof(setupPINCode)));
-
-        // Passcodes shall be restricted to the values 00000001 to 99999998 in decimal, see 5.1.1.6
-        setupPINCode = (setupPINCode % kSetupPINCodeMaximumValue) + 1;
+        ReturnErrorOnFailure(SetupPayload::generateRandomSetupPin(setupPINCode));
     }
 
     return verifier.Generate(pbkdf2IterCount, salt, setupPINCode);

diff --git a/src/setup_payload/SetupPayload.cpp b/src/setup_payload/SetupPayload.cpp
@@ -23,6 +23,7 @@
 
 #include "SetupPayload.h"
 
+#include <crypto/CHIPCryptoPAL.h>
 #include <lib/core/CHIPCore.h>
 #include <lib/core/CHIPVendorIdentifiers.hpp>
 #include <lib/core/TLV.h>
@@ -207,6 +208,34 @@ CHIP_ERROR SetupPayload::removeSerialNumber()
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR SetupPayload::generateRandomSetupPin(uint32_t & setupPINCode)
+{
+    uint8_t retries          = 0;
+    const uint8_t maxRetries = 10;
+
+    do
+    {
+        ReturnErrorOnFailure(Crypto::DRBG_get_bytes(reinterpret_cast<uint8_t *>(&setupPINCode), sizeof(setupPINCode)));
+
+        // Passcodes shall be restricted to the values 00000001 to 99999998 in decimal, see 5.1.1.6
+        // TODO: Consider revising this method to ensure uniform distribution of setup PIN codes
+        setupPINCode = (setupPINCode % kSetupPINCodeMaximumValue) + 1;
+
+        // Make sure that the Generated Setup Pin code is not one of the invalid passcodes/pin codes defined in the
+        // specification.
+        if (IsValidSetupPIN(setupPINCode))
+        {
+            return CHIP_NO_ERROR;
+        }
+
+        retries++;
+        // We got pretty unlucky with the random number generator, Just try again.
+        // This shouldn't take many retries assuming DRBG_get_bytes is not broken.
+    } while (retries < maxRetries);
+
+    return CHIP_ERROR_INTERNAL;
+}
+
 CHIP_ERROR SetupPayload::addOptionalVendorData(const OptionalQRCodeInfo & info)
 {
     VerifyOrReturnError(IsVendorTag(info.tag), CHIP_ERROR_INVALID_ARGUMENT);

diff --git a/src/setup_payload/SetupPayload.h b/src/setup_payload/SetupPayload.h
@@ -259,6 +259,17 @@ class SetupPayload : public PayloadContents
      **/
     static bool IsVendorTag(uint8_t tag) { return !IsCommonTag(tag); }
 
+    /** @brief Generate a Random Setup Pin Code (Passcode)
+     *
+     * This function generates a random passcode within the defined limits (00000001 to 99999998)
+     * It also checks that the generated passcode is not equal to any invalid passcode values as defined in 5.1.7.1.
+     *
+     * @param[out] setupPINCode The generated random setup PIN code.
+     * @return Returns a CHIP_ERROR_INTERNAL if unable to generate a valid passcode within a reasonable number of attempts,
+     * CHIP_NO_ERROR otherwise
+     **/
+    static CHIP_ERROR generateRandomSetupPin(uint32_t & setupPINCode);
+
 private:
     std::map<uint8_t, OptionalQRCodeInfo> optionalVendorData;
     std::map<uint8_t, OptionalQRCodeInfoExtension> optionalExtensionData;