From 969053227bebd72e6e0184fea96ad63fe5310b90 Mon Sep 17 00:00:00 2001 From: Amine Alami <43780877+Alami-Amine@users.noreply.github.com> Date: Thu, 10 Oct 2024 21:59:20 +0200 Subject: [PATCH] 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 * Integrating comments --------- Co-authored-by: Andrei Litvin --- src/protocols/secure_channel/PASESession.cpp | 5 +--- src/setup_payload/SetupPayload.cpp | 29 ++++++++++++++++++++ src/setup_payload/SetupPayload.h | 11 ++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index 6178348ad5d5d5..345b307f26c24d 100644 --- 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(&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 index 9683da21f8a7f9..915f20c4a2c863 100644 --- a/src/setup_payload/SetupPayload.cpp +++ b/src/setup_payload/SetupPayload.cpp @@ -23,6 +23,7 @@ #include "SetupPayload.h" +#include #include #include #include @@ -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(&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 index 3d5d101fbf4c6e..829af128bf7627 100644 --- 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 optionalVendorData; std::map optionalExtensionData;