From e16029027dfdf506566994697138f14cd6dcebd5 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Wed, 13 Oct 2021 12:17:36 -0400 Subject: [PATCH 1/3] Introduce a RNG determinism audit - Add a temporary audit function in ExchangeMgr.cpp. - ExchangeMgr.cpp is used because it is used by both `DeviceController` and `Server` (i.e. by all modalities) and it already relies on random numbers. - Audit must be run manually since it needs to be done on targets and comparisons across reboot. - Audit validates `GetRandU*` and `Crypto::DRBG_get_bytes()`, which may both use different underlying impl. Testing done: - Unit tests pass - Manually tested on Linux by commenting-out `srand(seed);` in Entropy.cpp, which forces `GetRandU*` to give the same values across calls. - If you are testing this, search for lines starting with `AUDIT:` in the log. Issue #10454 --- src/messaging/ExchangeMgr.cpp | 55 +++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 6cd46ac7baef51..749a4bdb9616e1 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -33,6 +33,13 @@ #include #include +// Temporary includes for TemporaryAuditRandomPerformance() +// TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. +#if 1 +#include +#include +#endif + #include #include #include @@ -50,6 +57,49 @@ using namespace chip::System; namespace chip { namespace Messaging { +namespace { + +// Audit random number generator proper initialization with prints. +// TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. +void TemporaryAuditRandomNumberGenerator() +{ + uint8_t buf1[16]; + uint8_t buf2[16]; + + memset(&buf1[0], 0, sizeof(buf1)); + memset(&buf2[0], 0, sizeof(buf2)); + + VerifyOrDie(chip::Crypto::DRBG_get_bytes(&buf1[0], sizeof(buf1)) == CHIP_NO_ERROR); + VerifyOrDie(chip::Crypto::DRBG_get_bytes(&buf2[0], sizeof(buf2)) == CHIP_NO_ERROR); + + char hex_buf[sizeof(buf1) * 2 + 1]; + + ChipLogProgress(ExchangeManager, "AUDIT: ===== RANDOM NUMBER GENERATOR AUDIT START ===="); + ChipLogProgress(ExchangeManager, "AUDIT: * Validate buf1 and buf2 are <<>>"); + ChipLogProgress(ExchangeManager, "AUDIT: * Validate r1 and r2 are <<>>"); + + memset(&hex_buf[0], 0, sizeof(hex_buf)); + VerifyOrDie(Encoding::BytesToUppercaseHexString(&buf1[0], sizeof(buf1), &hex_buf[0], sizeof(hex_buf)) == CHIP_NO_ERROR); + ChipLogProgress(ExchangeManager, "AUDIT: * buf1: %s", &hex_buf[0]); + + memset(&hex_buf[0], 0, sizeof(hex_buf)); + VerifyOrDie(Encoding::BytesToUppercaseHexString(&buf2[0], sizeof(buf2), &hex_buf[0], sizeof(hex_buf)) == CHIP_NO_ERROR); + ChipLogProgress(ExchangeManager, "AUDIT: * buf2: %s", &hex_buf[0]); + + VerifyOrDieWithMsg(memcmp(&buf1[0], &buf2[0], sizeof(buf1)) != 0, ExchangeManager, + "AUDIT: FAILED: buf1, buf2 are equal: DRBG_get_bytes() does not function!"); + + uint32_t r1 = GetRandU32(); + uint32_t r2 = GetRandU32(); + + ChipLogProgress(ExchangeManager, "AUDIT: * r1: 0x%08" PRIX32 " r2: 0x%08" PRIX32, r1, r2); + VerifyOrDieWithMsg(r1 != r2, ExchangeManager, + "AUDIT: FAILED: buf1, buf2 are equal: random number generator does not function!"); + ChipLogProgress(ExchangeManager, "AUDIT: ===== RANDOM NUMBER GENERATOR AUDIT END ===="); +} + +} // namespace + /** * Constructor for the ExchangeManager class. * It sets the state to kState_NotInitialized. @@ -72,6 +122,11 @@ CHIP_ERROR ExchangeManager::Init(SessionManager * sessionManager) mSessionManager = sessionManager; + { + // TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. + TemporaryAuditRandomNumberGenerator(); + } + mNextExchangeId = GetRandU16(); mNextKeyId = 0; From 33a1302ba3f1c19c3932778a99ee7fa9e6406979 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Wed, 13 Oct 2021 15:25:10 -0400 Subject: [PATCH 2/3] Move audit to InitEntropy() - InitEntropy is in `GenericPlatformManagerImpl` and should always be called. Lack of the logs will help catch bad init. --- src/messaging/ExchangeMgr.cpp | 55 --------------------------------- src/platform/Entropy.cpp | 57 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 55 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 749a4bdb9616e1..6cd46ac7baef51 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -33,13 +33,6 @@ #include #include -// Temporary includes for TemporaryAuditRandomPerformance() -// TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. -#if 1 -#include -#include -#endif - #include #include #include @@ -57,49 +50,6 @@ using namespace chip::System; namespace chip { namespace Messaging { -namespace { - -// Audit random number generator proper initialization with prints. -// TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. -void TemporaryAuditRandomNumberGenerator() -{ - uint8_t buf1[16]; - uint8_t buf2[16]; - - memset(&buf1[0], 0, sizeof(buf1)); - memset(&buf2[0], 0, sizeof(buf2)); - - VerifyOrDie(chip::Crypto::DRBG_get_bytes(&buf1[0], sizeof(buf1)) == CHIP_NO_ERROR); - VerifyOrDie(chip::Crypto::DRBG_get_bytes(&buf2[0], sizeof(buf2)) == CHIP_NO_ERROR); - - char hex_buf[sizeof(buf1) * 2 + 1]; - - ChipLogProgress(ExchangeManager, "AUDIT: ===== RANDOM NUMBER GENERATOR AUDIT START ===="); - ChipLogProgress(ExchangeManager, "AUDIT: * Validate buf1 and buf2 are <<>>"); - ChipLogProgress(ExchangeManager, "AUDIT: * Validate r1 and r2 are <<>>"); - - memset(&hex_buf[0], 0, sizeof(hex_buf)); - VerifyOrDie(Encoding::BytesToUppercaseHexString(&buf1[0], sizeof(buf1), &hex_buf[0], sizeof(hex_buf)) == CHIP_NO_ERROR); - ChipLogProgress(ExchangeManager, "AUDIT: * buf1: %s", &hex_buf[0]); - - memset(&hex_buf[0], 0, sizeof(hex_buf)); - VerifyOrDie(Encoding::BytesToUppercaseHexString(&buf2[0], sizeof(buf2), &hex_buf[0], sizeof(hex_buf)) == CHIP_NO_ERROR); - ChipLogProgress(ExchangeManager, "AUDIT: * buf2: %s", &hex_buf[0]); - - VerifyOrDieWithMsg(memcmp(&buf1[0], &buf2[0], sizeof(buf1)) != 0, ExchangeManager, - "AUDIT: FAILED: buf1, buf2 are equal: DRBG_get_bytes() does not function!"); - - uint32_t r1 = GetRandU32(); - uint32_t r2 = GetRandU32(); - - ChipLogProgress(ExchangeManager, "AUDIT: * r1: 0x%08" PRIX32 " r2: 0x%08" PRIX32, r1, r2); - VerifyOrDieWithMsg(r1 != r2, ExchangeManager, - "AUDIT: FAILED: buf1, buf2 are equal: random number generator does not function!"); - ChipLogProgress(ExchangeManager, "AUDIT: ===== RANDOM NUMBER GENERATOR AUDIT END ===="); -} - -} // namespace - /** * Constructor for the ExchangeManager class. * It sets the state to kState_NotInitialized. @@ -122,11 +72,6 @@ CHIP_ERROR ExchangeManager::Init(SessionManager * sessionManager) mSessionManager = sessionManager; - { - // TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. - TemporaryAuditRandomNumberGenerator(); - } - mNextExchangeId = GetRandU16(); mNextKeyId = 0; diff --git a/src/platform/Entropy.cpp b/src/platform/Entropy.cpp index 08225c4440c71f..88d6bf2c80e61f 100644 --- a/src/platform/Entropy.cpp +++ b/src/platform/Entropy.cpp @@ -23,8 +23,60 @@ */ #include +#include +#include + +// Temporary includes for TemporaryAuditRandomPerformance() +// TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. +#if 1 +#include +#endif namespace chip { + +namespace { + +// Audit random number generator proper initialization with prints. +// TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. +void TemporaryAuditRandomNumberGenerator() +{ + uint8_t buf1[16]; + uint8_t buf2[16]; + + memset(&buf1[0], 0, sizeof(buf1)); + memset(&buf2[0], 0, sizeof(buf2)); + + VerifyOrDie(chip::Crypto::DRBG_get_bytes(&buf1[0], sizeof(buf1)) == CHIP_NO_ERROR); + VerifyOrDie(chip::Crypto::DRBG_get_bytes(&buf2[0], sizeof(buf2)) == CHIP_NO_ERROR); + + char hex_buf[sizeof(buf1) * 2 + 1]; + + ChipLogProgress(DeviceLayer, "AUDIT: ===== RANDOM NUMBER GENERATOR AUDIT START ===="); + ChipLogProgress(DeviceLayer, "AUDIT: * Validate buf1 and buf2 are <<>>"); + ChipLogProgress(DeviceLayer, "AUDIT: * Validate r1 and r2 are <<>>"); + + memset(&hex_buf[0], 0, sizeof(hex_buf)); + VerifyOrDie(Encoding::BytesToUppercaseHexString(&buf1[0], sizeof(buf1), &hex_buf[0], sizeof(hex_buf)) == CHIP_NO_ERROR); + ChipLogProgress(DeviceLayer, "AUDIT: * buf1: %s", &hex_buf[0]); + + memset(&hex_buf[0], 0, sizeof(hex_buf)); + VerifyOrDie(Encoding::BytesToUppercaseHexString(&buf2[0], sizeof(buf2), &hex_buf[0], sizeof(hex_buf)) == CHIP_NO_ERROR); + ChipLogProgress(DeviceLayer, "AUDIT: * buf2: %s", &hex_buf[0]); + + VerifyOrDieWithMsg(memcmp(&buf1[0], &buf2[0], sizeof(buf1)) != 0, DeviceLayer, + "AUDIT: FAILED: buf1, buf2 are equal: DRBG_get_bytes() does not function!"); + + uint32_t r1 = GetRandU32(); + uint32_t r2 = GetRandU32(); + + ChipLogProgress(DeviceLayer, "AUDIT: * r1: 0x%08" PRIX32 " r2: 0x%08" PRIX32, r1, r2); + VerifyOrDieWithMsg(r1 != r2, DeviceLayer, + "AUDIT: FAILED: buf1, buf2 are equal: random number generator does not function!"); + ChipLogProgress(DeviceLayer, "AUDIT: ===== RANDOM NUMBER GENERATOR AUDIT END ===="); +} + +} // namespace + namespace DeviceLayer { namespace Internal { @@ -33,6 +85,11 @@ CHIP_ERROR InitEntropy() unsigned int seed; ReturnErrorOnFailure(chip::Crypto::DRBG_get_bytes((uint8_t *) &seed, sizeof(seed))); srand(seed); + + { + // TODO: remove once https://github.com/project-chip/connectedhomeip/issues/10454 is done. + TemporaryAuditRandomNumberGenerator(); + } return CHIP_NO_ERROR; } From b5cd7734c0a9b685305c851bc441ada3bb96fb6c Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 13 Oct 2021 19:26:34 +0000 Subject: [PATCH 3/3] Restyled by clang-format --- src/platform/Entropy.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/platform/Entropy.cpp b/src/platform/Entropy.cpp index 88d6bf2c80e61f..b443b644dce672 100644 --- a/src/platform/Entropy.cpp +++ b/src/platform/Entropy.cpp @@ -70,8 +70,7 @@ void TemporaryAuditRandomNumberGenerator() uint32_t r2 = GetRandU32(); ChipLogProgress(DeviceLayer, "AUDIT: * r1: 0x%08" PRIX32 " r2: 0x%08" PRIX32, r1, r2); - VerifyOrDieWithMsg(r1 != r2, DeviceLayer, - "AUDIT: FAILED: buf1, buf2 are equal: random number generator does not function!"); + VerifyOrDieWithMsg(r1 != r2, DeviceLayer, "AUDIT: FAILED: buf1, buf2 are equal: random number generator does not function!"); ChipLogProgress(DeviceLayer, "AUDIT: ===== RANDOM NUMBER GENERATOR AUDIT END ===="); }