diff --git a/examples/lit-icd-app/silabs/src/ShellCommands.cpp b/examples/lit-icd-app/silabs/src/ShellCommands.cpp index 0d898dc26b319f..52f305eb275333 100644 --- a/examples/lit-icd-app/silabs/src/ShellCommands.cpp +++ b/examples/lit-icd-app/silabs/src/ShellCommands.cpp @@ -18,9 +18,6 @@ #if defined(ENABLE_CHIP_SHELL) #include "ShellCommands.h" -#include "BindingHandler.h" - -#include #include #include #include diff --git a/src/app/icd/BUILD.gn b/src/app/icd/BUILD.gn index fcb396b3bcd41b..b19384c2e41047 100644 --- a/src/app/icd/BUILD.gn +++ b/src/app/icd/BUILD.gn @@ -81,6 +81,7 @@ source_set("sender") { ] public_deps = [ + ":configuration-data", ":monitoring-table", ":notifier", "${chip_root}/src/credentials:credentials", diff --git a/src/app/icd/ICDCheckInSender.cpp b/src/app/icd/ICDCheckInSender.cpp index ea827c2c7ac6c1..163072309518dc 100644 --- a/src/app/icd/ICDCheckInSender.cpp +++ b/src/app/icd/ICDCheckInSender.cpp @@ -15,15 +15,12 @@ * limitations under the License. */ -#include "ICDCheckInSender.h" - -#include "ICDNotifier.h" - -#include - -#include - +#include +#include +#include #include +#include +#include namespace chip { namespace app { @@ -56,12 +53,26 @@ void ICDCheckInSender::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP CHIP_ERROR ICDCheckInSender::SendCheckInMsg(const Transport::PeerAddress & addr) { - System::PacketBufferHandle buffer = MessagePacketBuffer::New(CheckinMessage::sMinPayloadSize); + System::PacketBufferHandle buffer = MessagePacketBuffer::New(CheckinMessage::kMinPayloadSize); VerifyOrReturnError(!buffer.IsNull(), CHIP_ERROR_NO_MEMORY); MutableByteSpan output{ buffer->Start(), buffer->MaxDataLength() }; - ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mAesKeyHandle, mICDCounter, ByteSpan(), output)); + // Encoded ActiveModeThreshold in littleEndian for Check-In message application data + { + uint8_t activeModeThresholdBuffer[kApplicationDataSize] = { 0 }; + size_t writtenBytes = 0; + Encoding::LittleEndian::BufferWriter writer(activeModeThresholdBuffer, sizeof(activeModeThresholdBuffer)); + + writer.Put16(ICDConfigurationData::GetInstance().GetActiveModeThresholdMs()); + VerifyOrReturnError(writer.Fit(writtenBytes), CHIP_ERROR_INTERNAL); + + ByteSpan activeModeThresholdByteSpan(writer.Buffer(), writtenBytes); + + ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mAes128KeyHandle, mHmac128KeyHandle, mICDCounter, + activeModeThresholdByteSpan, output)); + } + buffer->SetDataLength(static_cast(output.size())); VerifyOrReturnError(mExchangeManager->GetSessionManager() != nullptr, CHIP_ERROR_INTERNAL); @@ -89,9 +100,12 @@ CHIP_ERROR ICDCheckInSender::RequestResolve(ICDMonitoringEntry & entry, FabricTa AddressResolve::NodeLookupRequest request(peerId); - memcpy(mAesKeyHandle.AsMutable(), + memcpy(mAes128KeyHandle.AsMutable(), entry.aesKeyHandle.As(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); + memcpy(mHmac128KeyHandle.AsMutable(), + entry.hmacKeyHandle.As(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); + CHIP_ERROR err = AddressResolve::Resolver::Instance().LookupNode(request, mAddressLookupHandle); if (err == CHIP_NO_ERROR) diff --git a/src/app/icd/ICDCheckInSender.h b/src/app/icd/ICDCheckInSender.h index b6e2dfa2212f92..a9d7919aaaaf28 100644 --- a/src/app/icd/ICDCheckInSender.h +++ b/src/app/icd/ICDCheckInSender.h @@ -43,6 +43,8 @@ class ICDCheckInSender : public AddressResolve::NodeListener bool mResolveInProgress = false; private: + static constexpr uint8_t kApplicationDataSize = 2; // ActiveModeThreshold is 2 bytes + CHIP_ERROR SendCheckInMsg(const Transport::PeerAddress & addr); // This is used when a node address is required. @@ -50,7 +52,8 @@ class ICDCheckInSender : public AddressResolve::NodeListener Messaging::ExchangeManager * mExchangeManager = nullptr; - Crypto::Aes128KeyHandle mAesKeyHandle = Crypto::Aes128KeyHandle(); + Crypto::Aes128KeyHandle mAes128KeyHandle = Crypto::Aes128KeyHandle(); + Crypto::Hmac128KeyHandle mHmac128KeyHandle = Crypto::Hmac128KeyHandle(); uint32_t mICDCounter = 0; }; diff --git a/src/app/icd/ICDConfigurationData.cpp b/src/app/icd/ICDConfigurationData.cpp index 6abb9784fde1ce..4df2ca75f217b9 100644 --- a/src/app/icd/ICDConfigurationData.cpp +++ b/src/app/icd/ICDConfigurationData.cpp @@ -28,9 +28,9 @@ System::Clock::Milliseconds32 ICDConfigurationData::GetSlowPollingInterval() // When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec. // This is important for ICD device configured for LIT operation but currently operating as a SIT // due to a lack of client registration - if (mICDMode == ICDMode::SIT && GetSlowPollingInterval() > GetSITPollingThreshold()) + if (mICDMode == ICDMode::SIT && mSlowPollingInterval > kSITPollingThreshold) { - return GetSITPollingThreshold(); + return kSITPollingThreshold; } #endif return mSlowPollingInterval; diff --git a/src/app/icd/ICDManager.cpp b/src/app/icd/ICDManager.cpp index daf312058ee693..09190ae9dfb8e1 100644 --- a/src/app/icd/ICDManager.cpp +++ b/src/app/icd/ICDManager.cpp @@ -275,15 +275,6 @@ void ICDManager::UpdateOperationState(OperationalState state) System::Clock::Milliseconds32 slowPollInterval = ICDConfigurationData::GetInstance().GetSlowPollingInterval(); -#if ICD_ENFORCE_SIT_SLOW_POLL_LIMIT - // When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec. - if (ICDConfigurationData::GetInstance().GetICDMode() == ICDConfigurationData::ICDMode::SIT && - GetSlowPollingInterval() > GetSITPollingThreshold()) - { - slowPollInterval = GetSITPollingThreshold(); - } -#endif - // Going back to Idle, all Check-In messages are sent mICDSenderPool.ReleaseAll(); diff --git a/src/protocols/secure_channel/CheckinMessage.cpp b/src/protocols/secure_channel/CheckinMessage.cpp index 5e63feb3428aa0..f8545b913687f7 100644 --- a/src/protocols/secure_channel/CheckinMessage.cpp +++ b/src/protocols/secure_channel/CheckinMessage.cpp @@ -30,71 +30,118 @@ namespace chip { namespace Protocols { namespace SecureChannel { -CHIP_ERROR CheckinMessage::GenerateCheckinMessagePayload(Crypto::Aes128KeyHandle & key, CounterType counter, - const ByteSpan & appData, MutableByteSpan & output) +CHIP_ERROR CheckinMessage::GenerateCheckinMessagePayload(const Crypto::Aes128KeyHandle & aes128KeyHandle, + const Crypto::Hmac128KeyHandle & hmacKeyHandle, + const CounterType & counter, const ByteSpan & appData, + MutableByteSpan & output) { - VerifyOrReturnError(appData.size() <= sMaxAppDataSize, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(output.size() >= (appData.size() + sMinPayloadSize), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(output.size() >= (appData.size() + kMinPayloadSize), CHIP_ERROR_BUFFER_TOO_SMALL); + size_t cursorIndex = 0; - CHIP_ERROR err = CHIP_NO_ERROR; - uint8_t * appDataStartPtr = output.data() + CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES; - Encoding::LittleEndian::Put32(appDataStartPtr, counter); + // Generate Nonce from Key and counter value + { + MutableByteSpan nonce = output.SubSpan(0, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES); + cursorIndex += nonce.size(); - chip::Crypto::HMAC_sha shaHandler; - uint8_t nonceWorkBuffer[CHIP_CRYPTO_HASH_LEN_BYTES] = { 0 }; + Encoding::LittleEndian::BufferWriter writer(nonce); + ReturnErrorOnFailure(GenerateCheckInMessageNonce(hmacKeyHandle, counter, writer)); + } + + // Encrypt Counter and Application Data + { + MutableByteSpan payloadByteSpan = output.SubSpan(cursorIndex, sizeof(CounterType) + appData.size()); + cursorIndex += payloadByteSpan.size(); - ReturnErrorOnFailure(shaHandler.HMAC_SHA256(key.As(), sizeof(Symmetric128BitsKeyByteArray), - appDataStartPtr, sizeof(CounterType), nonceWorkBuffer, CHIP_CRYPTO_HASH_LEN_BYTES)); + Encoding::LittleEndian::BufferWriter payloadWriter(payloadByteSpan); - static_assert(sizeof(nonceWorkBuffer) >= CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, "We're reading off the end of our buffer."); - memcpy(output.data(), nonceWorkBuffer, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES); + payloadWriter.EndianPut(counter, sizeof(counter)); + payloadWriter.Put(appData.data(), appData.size()); + VerifyOrReturnError(payloadWriter.Fit(), CHIP_ERROR_INTERNAL); - // In place encryption to save some RAM - memcpy(appDataStartPtr + sizeof(CounterType), appData.data(), appData.size()); + MutableByteSpan mic = output.SubSpan(cursorIndex, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES); + cursorIndex += mic.size(); - uint8_t * micPtr = appDataStartPtr + sizeof(CounterType) + appData.size(); - ReturnErrorOnFailure(Crypto::AES_CCM_encrypt(appDataStartPtr, sizeof(CounterType) + appData.size(), nullptr, 0, key, - output.data(), CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, appDataStartPtr, micPtr, - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES)); + // Validate that the cursorIndex is within the available output space + VerifyOrReturnError(cursorIndex <= output.size(), CHIP_ERROR_BUFFER_TOO_SMALL); + // Validate that the cursorIndex matchs the message length + VerifyOrReturnError(cursorIndex == appData.size() + kMinPayloadSize, CHIP_ERROR_INTERNAL); - output.reduce_size(appData.size() + sMinPayloadSize); + ReturnErrorOnFailure(Crypto::AES_CCM_encrypt(payloadByteSpan.data(), payloadByteSpan.size(), nullptr, 0, aes128KeyHandle, + output.data(), CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, payloadByteSpan.data(), + mic.data(), mic.size())); + } - return err; + output.reduce_size(appData.size() + kMinPayloadSize); + return CHIP_NO_ERROR; } -CHIP_ERROR CheckinMessage::ParseCheckinMessagePayload(Crypto::Aes128KeyHandle & key, ByteSpan & payload, CounterType & counter, - MutableByteSpan & appData) +CHIP_ERROR CheckinMessage::ParseCheckinMessagePayload(const Crypto::Aes128KeyHandle & aes128KeyHandle, + const Crypto::Hmac128KeyHandle & hmacKeyHandle, ByteSpan & payload, + CounterType & counter, MutableByteSpan & appData) { - VerifyOrReturnError(payload.size() >= sMinPayloadSize, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(payload.size() <= (sMinPayloadSize + sMaxAppDataSize), CHIP_ERROR_INVALID_ARGUMENT); - - CHIP_ERROR err = CHIP_NO_ERROR; size_t appDataSize = GetAppDataSize(payload); + VerifyOrReturnError(payload.size() >= kMinPayloadSize, CHIP_ERROR_INVALID_MESSAGE_LENGTH); // To prevent workbuffer usage, appData size needs to be large enough to hold both the appData and the counter - VerifyOrReturnError(appData.size() >= sizeof(CounterType) + appDataSize, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(appData.size() >= sizeof(CounterType) + appDataSize, CHIP_ERROR_BUFFER_TOO_SMALL); + + // Decrypt received data + { + size_t cursorIndex = 0; + + ByteSpan nonce = payload.SubSpan(cursorIndex, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES); + cursorIndex += nonce.size(); - ByteSpan nonce = payload.SubSpan(0, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES); - ByteSpan encryptedData = payload.SubSpan(CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, sizeof(CounterType) + appDataSize); - ByteSpan mic = - payload.SubSpan(CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES + sizeof(CounterType) + appDataSize, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES); + ByteSpan encryptedData = payload.SubSpan(cursorIndex, sizeof(CounterType) + appDataSize); + cursorIndex += encryptedData.size(); - err = Crypto::AES_CCM_decrypt(encryptedData.data(), encryptedData.size(), nullptr, 0, mic.data(), mic.size(), key, nonce.data(), - nonce.size(), appData.data()); + ByteSpan mic = payload.SubSpan(cursorIndex, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES); + cursorIndex += mic.size(); - ReturnErrorOnFailure(err); + // Return Invalid message length since the payload isn't the right size + VerifyOrReturnError(cursorIndex == payload.size(), CHIP_ERROR_INVALID_MESSAGE_LENGTH); + ReturnErrorOnFailure(Crypto::AES_CCM_decrypt(encryptedData.data(), encryptedData.size(), nullptr, 0, mic.data(), mic.size(), + aes128KeyHandle, nonce.data(), nonce.size(), appData.data())); + } + + // Read decrypted counter and application data counter = Encoding::LittleEndian::Get32(appData.data()); + + // TODO : Validate received nonce by calculating it with the hmacKeyHandle and received Counter value + // Shift to remove the counter from the appData memmove(appData.data(), sizeof(CounterType) + appData.data(), appDataSize); - appData.reduce_size(appDataSize); - return err; + + return CHIP_NO_ERROR; +} + +CHIP_ERROR CheckinMessage::GenerateCheckInMessageNonce(const Crypto::Hmac128KeyHandle & hmacKeyHandle, CounterType counter, + Encoding::LittleEndian::BufferWriter & writer) +{ + VerifyOrReturnError(writer.Available() >= CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, CHIP_ERROR_BUFFER_TOO_SMALL); + + uint8_t hashWorkBuffer[CHIP_CRYPTO_HASH_LEN_BYTES] = { 0 }; + uint8_t counterBuffer[sizeof(CounterType)]; + + // validate that Check-In counter is a uint32_t + static_assert(sizeof(CounterType) == sizeof(uint32_t), "Expect counter to be 32 bits for correct encoding"); + Encoding::LittleEndian::Put32(counterBuffer, counter); + + chip::Crypto::HMAC_sha shaHandler; + ReturnErrorOnFailure( + shaHandler.HMAC_SHA256(hmacKeyHandle, counterBuffer, sizeof(CounterType), hashWorkBuffer, CHIP_CRYPTO_HASH_LEN_BYTES)); + + writer.Put(hashWorkBuffer, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES); + VerifyOrReturnError(writer.Fit(), CHIP_ERROR_BUFFER_TOO_SMALL); + + return CHIP_NO_ERROR; } size_t CheckinMessage::GetAppDataSize(ByteSpan & payload) { - return (payload.size() <= sMinPayloadSize) ? 0 : payload.size() - sMinPayloadSize; + return (payload.size() <= kMinPayloadSize) ? 0 : payload.size() - kMinPayloadSize; } } // namespace SecureChannel diff --git a/src/protocols/secure_channel/CheckinMessage.h b/src/protocols/secure_channel/CheckinMessage.h index aa494c3689b5c8..530c601a2432fe 100644 --- a/src/protocols/secure_channel/CheckinMessage.h +++ b/src/protocols/secure_channel/CheckinMessage.h @@ -23,6 +23,7 @@ #pragma once #include +#include #include #include @@ -45,30 +46,47 @@ class DLL_EXPORT CheckinMessage /** * @brief Generate Check-in Message payload * - * @param key Key with which to encrypt the check-in payload - * @param counter Check-in counter - * @param appData Application Data to incorporate within the Check-in message. Allowed to be empty. - * @param output Buffer in Which to store the generated payload. SUFFICIENT SPACE MUST BE ALLOCATED by the caller - * Required Buffer Size is : GetCheckinPayloadSize(appData.size()) - * @return CHIP_ERROR + * @note Function requires two key handles to generate the Check-In message. + * Due to the way some key stores work, the same key handle cannot be used for AES-CCM and HMAC-SHA-256 operations. + * + * @param[in] aes128KeyHandle Key handle with which to encrypt the check-in payload (using AEAD). + * @param[in] hmac128KeyHandle Key handle with which to generate the nonce for the check-in payload (using HMAC). + * @param[in] counter Check-in counter + * @param[in] appData Application Data to incorporate within the Check-in message. Allowed to be empty. + * @param[out] output Buffer in Which to store the generated payload. SUFFICIENT SPACE MUST BE ALLOCATED by the + * caller Required Buffer Size is : GetCheckinPayloadSize(appData.size()) + * + * @return CHIP_ERROR_BUFFER_TOO_SMALL if output buffer is too small + * CHIP_ERROR_INVALID_ARGUMENT if the provided arguments cannot be used to generate the Check-In message + * CHIP_ERROR_INTERNAL if an error occurs during the generation of the Check-In message */ - static CHIP_ERROR GenerateCheckinMessagePayload(Crypto::Aes128KeyHandle & key, CounterType counter, const ByteSpan & appData, - MutableByteSpan & output); + static CHIP_ERROR GenerateCheckinMessagePayload(const Crypto::Aes128KeyHandle & aes128KeyHandle, + const Crypto::Hmac128KeyHandle & hmacKeyHandle, const CounterType & counter, + const ByteSpan & appData, MutableByteSpan & output); /** * @brief Parse Check-in Message payload * - * @param key Key with which to decrypt the check-in payload - * @param payload The received payload to decrypt and parse - * @param counter The counter value retrieved from the payload - * @param appData The optional application data decrypted. The size of appData must be at least the size of - * GetAppDataSize(payload) + sizeof(CounterType) - * @return CHIP_ERROR + * @note Function requires two key handles to parse the Check-In message. + * Due to the way some key stores work, the same key handle cannot be used for AES-CCM and HMAC-SHA-256 operations. + * + * @param[in] aes128KeyHandle Key handle with which to decrypt the received check-in payload (using AEAD). + * @param[in] hmac128KeyHandle Key handle with which to verify the received nonce in the check-in payload (using HMAC). + * @param[in] payload The received payload to decrypt and parse + * @param[out] counter The counter value retrieved from the payload + * @param[in,out] appData The optional application data decrypted. The input size of appData must be at least the + * size of GetAppDataSize(payload) + sizeof(CounterType), because appData is used as a work buffer for the decryption process. + * The output size on success will be GetAppDataSize(payload). + * + * @return CHIP_ERROR_INVALID_MESSAGE_LENGTH if the payload is shorter than the minimum payload size + * CHIP_ERROR_BUFFER_TOO_SMALL if appData buffer is too small + * CHIP_ERROR_INVALID_ARGUMENT if the provided arguments cannot be used to parse the Check-In message */ - static CHIP_ERROR ParseCheckinMessagePayload(Crypto::Aes128KeyHandle & key, ByteSpan & payload, CounterType & counter, - MutableByteSpan & appData); + static CHIP_ERROR ParseCheckinMessagePayload(const Crypto::Aes128KeyHandle & aes128KeyHandle, + const Crypto::Hmac128KeyHandle & hmacKeyHandle, ByteSpan & payload, + CounterType & counter, MutableByteSpan & appData); - static inline size_t GetCheckinPayloadSize(size_t appDataSize) { return appDataSize + sMinPayloadSize; } + static inline size_t GetCheckinPayloadSize(size_t appDataSize) { return appDataSize + kMinPayloadSize; } /** * @brief Get the App Data Size @@ -78,11 +96,24 @@ class DLL_EXPORT CheckinMessage */ static size_t GetAppDataSize(ByteSpan & payload); - static constexpr uint16_t sMinPayloadSize = + static constexpr uint16_t kMinPayloadSize = CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES + sizeof(CounterType) + CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES; - // Issue #28603 - static constexpr uint16_t sMaxAppDataSize = 1024; +private: + /** + * @brief Generate the Nonce for the Check-In message + * + * @param[in] hmacKeyHandle Key handle to use with the HMAC algorithm + * @param[in] counter Check-In Counter value to use as message of the HMAC algorithm + * @param[out] output output buffer for the generated Nonce. + * SUFFICIENT SPACE MUST BE ALLOCATED by the caller + * Size must be at least CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES + * + * @return CHIP_ERROR_BUFFER_TOO_SMALL if output buffer is too small + * CHIP_ERROR_INVALID_ARGUMENT if the provided arguments cannot be used to generate the Check-In message Nonce + */ + static CHIP_ERROR GenerateCheckInMessageNonce(const Crypto::Hmac128KeyHandle & hmacKeyHandle, CounterType counter, + Encoding::LittleEndian::BufferWriter & writer); }; } // namespace SecureChannel diff --git a/src/protocols/secure_channel/tests/TestCheckinMsg.cpp b/src/protocols/secure_channel/tests/TestCheckinMsg.cpp index 6310cf38c05494..07134235464de2 100644 --- a/src/protocols/secure_channel/tests/TestCheckinMsg.cpp +++ b/src/protocols/secure_channel/tests/TestCheckinMsg.cpp @@ -56,17 +56,24 @@ void TestCheckin_Generate(nlTestSuite * inSuite, void * inContext) { const ccm_128_test_vector & test = *testPtr; - Symmetric128BitsKeyByteArray keyMaterial; - memcpy(keyMaterial, test.key, test.key_len); + // Two distinct key material buffers to ensure crypto-hardware-assist with single-usage keys create two different handles. + Symmetric128BitsKeyByteArray aesKeyMaterial; + memcpy(aesKeyMaterial, test.key, test.key_len); - Aes128KeyHandle keyHandle; - NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(keyMaterial, keyHandle)); + Symmetric128BitsKeyByteArray hmacKeyMaterial; + memcpy(hmacKeyMaterial, test.key, test.key_len); + + Aes128KeyHandle aes128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(aesKeyMaterial, aes128KeyHandle)); + + Hmac128KeyHandle hmac128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(hmacKeyMaterial, hmac128KeyHandle)); // Validate that counter change, indeed changes the output buffer content counter = 0; for (uint8_t j = 0; j < 5; j++) { - err = CheckinMessage::GenerateCheckinMessagePayload(keyHandle, counter, userData, outputBuffer); + err = CheckinMessage::GenerateCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, counter, userData, outputBuffer); NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR == err)); // Verifiy that the output buffer changed @@ -77,21 +84,30 @@ void TestCheckin_Generate(nlTestSuite * inSuite, void * inContext) counter += chip::Crypto::GetRandU32() + 1; outputBuffer = MutableByteSpan(a); } - keystore.DestroyKey(keyHandle); + + keystore.DestroyKey(aes128KeyHandle); + keystore.DestroyKey(hmac128KeyHandle); } // Parameter check { - uint8_t data[] = { "This is some user Data. It should be encrypted" }; - userData = chip::ByteSpan(data); - const ccm_128_test_vector & test = *ccm_128_test_vectors[0]; - uint8_t gargantuaBuffer[2 * CheckinMessage::sMaxAppDataSize] = { 0 }; + uint8_t data[] = { "This is some user Data. It should be encrypted" }; + userData = chip::ByteSpan(data); + const ccm_128_test_vector & test = *ccm_128_test_vectors[0]; + uint8_t veryLargeBuffer[2048] = { 0 }; - Symmetric128BitsKeyByteArray keyMaterial; - memcpy(keyMaterial, test.key, test.key_len); + // Two distinct key material buffers to ensure crypto-hardware-assist with single-usage keys create two different handles. + Symmetric128BitsKeyByteArray aesKeyMaterial; + memcpy(aesKeyMaterial, test.key, test.key_len); - Aes128KeyHandle keyHandle; - NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(keyMaterial, keyHandle)); + Symmetric128BitsKeyByteArray hmacKeyMaterial; + memcpy(hmacKeyMaterial, test.key, test.key_len); + + Aes128KeyHandle aes128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(aesKeyMaterial, aes128KeyHandle)); + + Hmac128KeyHandle hmac128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(hmacKeyMaterial, hmac128KeyHandle)); // As of now passing an empty key handle while using PSA crypto will result in a failure. // However when using OpenSSL this same test result in a success. @@ -102,20 +118,24 @@ void TestCheckin_Generate(nlTestSuite * inSuite, void * inContext) // ChipLogError(Inet, "%s", err.AsString()); // NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR == err)); + // Testing empty application data ByteSpan emptyData; - err = CheckinMessage::GenerateCheckinMessagePayload(keyHandle, counter, emptyData, outputBuffer); + err = CheckinMessage::GenerateCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, counter, emptyData, outputBuffer); NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR == err)); + // Testing empty output buffer MutableByteSpan empty; - err = CheckinMessage::GenerateCheckinMessagePayload(keyHandle, counter, emptyData, empty); - NL_TEST_ASSERT(inSuite, (CHIP_ERROR_INVALID_ARGUMENT == err)); + err = CheckinMessage::GenerateCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, counter, emptyData, empty); + NL_TEST_ASSERT(inSuite, (CHIP_ERROR_BUFFER_TOO_SMALL == err)); - userData = chip::ByteSpan(gargantuaBuffer, sizeof(gargantuaBuffer)); - err = CheckinMessage::GenerateCheckinMessagePayload(keyHandle, counter, userData, outputBuffer); - NL_TEST_ASSERT(inSuite, (CHIP_ERROR_INVALID_ARGUMENT == err)); + // Test output buffer smaller than the ApplicationData + userData = chip::ByteSpan(veryLargeBuffer, sizeof(veryLargeBuffer)); + err = CheckinMessage::GenerateCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, counter, userData, outputBuffer); + NL_TEST_ASSERT(inSuite, (CHIP_ERROR_BUFFER_TOO_SMALL == err)); // Cleanup - keystore.DestroyKey(keyHandle); + keystore.DestroyKey(aes128KeyHandle); + keystore.DestroyKey(hmac128KeyHandle); } } @@ -137,27 +157,38 @@ void TestCheckin_Parse(nlTestSuite * inSuite, void * inContext) userData = chip::ByteSpan(data); const ccm_128_test_vector & test = *ccm_128_test_vectors[0]; - Symmetric128BitsKeyByteArray keyMaterial; - memcpy(keyMaterial, test.key, test.key_len); + // Two distinct key material buffers to ensure crypto-hardware-assist with single-usage keys create two different handles. + Symmetric128BitsKeyByteArray aesKeyMaterial; + memcpy(aesKeyMaterial, test.key, test.key_len); + + Symmetric128BitsKeyByteArray hmacKeyMaterial; + memcpy(hmacKeyMaterial, test.key, test.key_len); - Aes128KeyHandle keyHandle; - NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(keyMaterial, keyHandle)); + Aes128KeyHandle aes128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(aesKeyMaterial, aes128KeyHandle)); + + Hmac128KeyHandle hmac128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(hmacKeyMaterial, hmac128KeyHandle)); //=================Encrypt======================= - err = CheckinMessage::GenerateCheckinMessagePayload(keyHandle, counter, userData, outputBuffer); + err = CheckinMessage::GenerateCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, counter, userData, outputBuffer); ByteSpan payload = chip::ByteSpan(outputBuffer.data(), outputBuffer.size()); NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR == err)); //=================Decrypt======================= MutableByteSpan empty; - err = CheckinMessage::ParseCheckinMessagePayload(keyHandle, payload, decryptedCounter, empty); + err = CheckinMessage::ParseCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, payload, decryptedCounter, empty); NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR != err)); ByteSpan emptyPayload; - err = CheckinMessage::ParseCheckinMessagePayload(keyHandle, emptyPayload, decryptedCounter, buffer); + err = CheckinMessage::ParseCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, emptyPayload, decryptedCounter, buffer); NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR != err)); + + // Cleanup + keystore.DestroyKey(aes128KeyHandle); + keystore.DestroyKey(hmac128KeyHandle); } void TestCheckin_GenerateParse(nlTestSuite * inSuite, void * inContext) @@ -180,22 +211,29 @@ void TestCheckin_GenerateParse(nlTestSuite * inSuite, void * inContext) { const ccm_128_test_vector & test = *testPtr; - Symmetric128BitsKeyByteArray keyMaterial; - memcpy(keyMaterial, test.key, test.key_len); + // Two disctint key material to force the PSA unit tests to create two different Key IDs + Symmetric128BitsKeyByteArray aesKeyMaterial; + memcpy(aesKeyMaterial, test.key, test.key_len); + + Symmetric128BitsKeyByteArray hmacKeyMaterial; + memcpy(hmacKeyMaterial, test.key, test.key_len); - Aes128KeyHandle keyHandle; - NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(keyMaterial, keyHandle)); + Aes128KeyHandle aes128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(aesKeyMaterial, aes128KeyHandle)); + + Hmac128KeyHandle hmac128KeyHandle; + NL_TEST_ASSERT_SUCCESS(inSuite, keystore.CreateKey(hmacKeyMaterial, hmac128KeyHandle)); //=================Encrypt======================= - err = CheckinMessage::GenerateCheckinMessagePayload(keyHandle, counter, userData, outputBuffer); + err = CheckinMessage::GenerateCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, counter, userData, outputBuffer); NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR == err)); //=================Decrypt======================= uint32_t decryptedCounter = 0; ByteSpan payload = chip::ByteSpan(outputBuffer.data(), outputBuffer.size()); - err = CheckinMessage::ParseCheckinMessagePayload(keyHandle, payload, decryptedCounter, buffer); + err = CheckinMessage::ParseCheckinMessagePayload(aes128KeyHandle, hmac128KeyHandle, payload, decryptedCounter, buffer); NL_TEST_ASSERT(inSuite, (CHIP_NO_ERROR == err)); NL_TEST_ASSERT(inSuite, (memcmp(data, buffer.data(), sizeof(data)) == 0)); @@ -208,7 +246,10 @@ void TestCheckin_GenerateParse(nlTestSuite * inSuite, void * inContext) buffer = MutableByteSpan(b); counter += chip::Crypto::GetRandU32() + 1; - keystore.DestroyKey(keyHandle); + + // Cleanup + keystore.DestroyKey(aes128KeyHandle); + keystore.DestroyKey(hmac128KeyHandle); } }