From e3d84f89023c7a225d10f945e36d640116989da4 Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Thu, 14 Dec 2023 20:05:23 -0800 Subject: [PATCH] [ICD]add hmac support in icd client storage (#30939) * add hmac support in icd client storage * address comment * address comments * add missing change * Update examples/chip-tool/commands/icd/ICDCommand.cpp Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/icd/client/DefaultICDClientStorage.cpp Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Restyled by clang-format --------- Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> Co-authored-by: Restyled.io --- .../chip-tool/commands/icd/ICDCommand.cpp | 13 ++++--- .../icd/client/DefaultICDClientStorage.cpp | 39 ++++++++++++++----- src/app/icd/client/DefaultICDClientStorage.h | 3 +- src/app/icd/client/ICDClientInfo.h | 16 +++++--- src/app/icd/client/ICDClientStorage.h | 2 +- src/crypto/CHIPCryptoPAL.h | 1 + 6 files changed, 51 insertions(+), 23 deletions(-) diff --git a/examples/chip-tool/commands/icd/ICDCommand.cpp b/examples/chip-tool/commands/icd/ICDCommand.cpp index f3b43cfce6497f..4d8ab7e93452d1 100644 --- a/examples/chip-tool/commands/icd/ICDCommand.cpp +++ b/examples/chip-tool/commands/icd/ICDCommand.cpp @@ -27,8 +27,8 @@ CHIP_ERROR ICDListCommand::RunCommand() { app::ICDClientInfo info; auto iter = CHIPCommand::sICDClientStorage.IterateICDClientInfo(); - char icdSymmetricKeyHex[Crypto::kAES_CCM128_Key_Length * 2 + 1]; - + char icdAesKeyHex[Crypto::kAES_CCM128_Key_Length * 2 + 1]; + char icdHmacKeyHex[Crypto::kHMAC_CCM128_Key_Length * 2 + 1]; fprintf(stderr, " +-----------------------------------------------------------------------------+\n"); fprintf(stderr, " | %-75s |\n", "Known ICDs:"); fprintf(stderr, " +-----------------------------------------------------------------------------+\n"); @@ -45,9 +45,12 @@ CHIP_ERROR ICDListCommand::RunCommand() static_assert(std::is_same::value, "The following BytesToHex can copy/encode the key bytes from sharedKey to hexadecimal format, which only " "works for RawKeySessionKeystore"); - Encoding::BytesToHex(info.shared_key.As(), Crypto::kAES_CCM128_Key_Length, - icdSymmetricKeyHex, sizeof(icdSymmetricKeyHex), chip::Encoding::HexFlags::kNullTerminate); - fprintf(stderr, " | Symmetric Key: %60s |\n", icdSymmetricKeyHex); + Encoding::BytesToHex(info.aes_key_handle.As(), Crypto::kAES_CCM128_Key_Length, + icdAesKeyHex, sizeof(icdAesKeyHex), chip::Encoding::HexFlags::kNullTerminate); + fprintf(stderr, " | aes key: %60s |\n", icdAesKeyHex); + Encoding::BytesToHex(info.hmac_key_handle.As(), Crypto::kHMAC_CCM128_Key_Length, + icdHmacKeyHex, sizeof(icdHmacKeyHex), chip::Encoding::HexFlags::kNullTerminate); + fprintf(stderr, " | hmac key: %60s |\n", icdHmacKeyHex); } fprintf(stderr, " +-----------------------------------------------------------------------------+\n"); diff --git a/src/app/icd/client/DefaultICDClientStorage.cpp b/src/app/icd/client/DefaultICDClientStorage.cpp index 70b2aa035af5a3..79713fc6be2c15 100644 --- a/src/app/icd/client/DefaultICDClientStorage.cpp +++ b/src/app/icd/client/DefaultICDClientStorage.cpp @@ -258,13 +258,22 @@ CHIP_ERROR DefaultICDClientStorage::Load(FabricIndex fabricIndex, std::vector(), buf.data(), + // Aes key handle + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kAesKeyHandle))); + ByteSpan aesBuf; + ReturnErrorOnFailure(reader.Get(aesBuf)); + VerifyOrReturnError(aesBuf.size() == sizeof(Crypto::Symmetric128BitsKeyByteArray), CHIP_ERROR_INTERNAL); + memcpy(clientInfo.aes_key_handle.AsMutable(), aesBuf.data(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); + + // Hmac key handle + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kHmacKeyHandle))); + ByteSpan hmacBuf; + ReturnErrorOnFailure(reader.Get(hmacBuf)); + VerifyOrReturnError(hmacBuf.size() == sizeof(Crypto::Symmetric128BitsKeyByteArray), CHIP_ERROR_INTERNAL); + memcpy(clientInfo.hmac_key_handle.AsMutable(), hmacBuf.data(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)); + ReturnErrorOnFailure(reader.ExitContainer(ICDClientInfoType)); clientInfoVector.push_back(clientInfo); } @@ -285,12 +294,20 @@ CHIP_ERROR DefaultICDClientStorage::SetKey(ICDClientInfo & clientInfo, const Byt Crypto::Symmetric128BitsKeyByteArray keyMaterial; memcpy(keyMaterial, keyData.data(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); - return mpKeyStore->CreateKey(keyMaterial, clientInfo.shared_key); + // TODO : Update key lifetime once creaKey method supports it. + ReturnErrorOnFailure(mpKeyStore->CreateKey(keyMaterial, clientInfo.aes_key_handle)); + CHIP_ERROR err = mpKeyStore->CreateKey(keyMaterial, clientInfo.hmac_key_handle); + if (err != CHIP_NO_ERROR) + { + mpKeyStore->DestroyKey(clientInfo.aes_key_handle); + } + return err; } void DefaultICDClientStorage::RemoveKey(ICDClientInfo & clientInfo) { - mpKeyStore->DestroyKey(clientInfo.shared_key); + mpKeyStore->DestroyKey(clientInfo.aes_key_handle); + mpKeyStore->DestroyKey(clientInfo.hmac_key_handle); } CHIP_ERROR DefaultICDClientStorage::SerializeToTlv(TLV::TLVWriter & writer, const std::vector & clientInfoVector) @@ -306,8 +323,10 @@ CHIP_ERROR DefaultICDClientStorage::SerializeToTlv(TLV::TLVWriter & writer, cons ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kStartICDCounter), clientInfo.start_icd_counter)); ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kOffset), clientInfo.offset)); ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kMonitoredSubject), clientInfo.monitored_subject)); - ByteSpan buf(clientInfo.shared_key.As()); - ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kSharedKey), buf)); + ByteSpan aesBuf(clientInfo.aes_key_handle.As()); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kAesKeyHandle), aesBuf)); + ByteSpan hmacBuf(clientInfo.hmac_key_handle.As()); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kHmacKeyHandle), hmacBuf)); ReturnErrorOnFailure(writer.EndContainer(ICDClientInfoContainerType)); } return writer.EndContainer(arrayType); diff --git a/src/app/icd/client/DefaultICDClientStorage.h b/src/app/icd/client/DefaultICDClientStorage.h index be1c8a2f4056ae..c98d058fbba3e8 100644 --- a/src/app/icd/client/DefaultICDClientStorage.h +++ b/src/app/icd/client/DefaultICDClientStorage.h @@ -91,7 +91,8 @@ class DefaultICDClientStorage : public ICDClientStorage kStartICDCounter = 3, kOffset = 4, kMonitoredSubject = 5, - kSharedKey = 6 + kAesKeyHandle = 6, + kHmacKeyHandle = 7, }; enum class CounterTag : uint8_t diff --git a/src/app/icd/client/ICDClientInfo.h b/src/app/icd/client/ICDClientInfo.h index f7863b61798148..5a20b1990744bd 100644 --- a/src/app/icd/client/ICDClientInfo.h +++ b/src/app/icd/client/ICDClientInfo.h @@ -30,10 +30,11 @@ namespace app { struct ICDClientInfo { ScopedNodeId peer_node; - uint32_t start_icd_counter = 0; - uint32_t offset = 0; - uint64_t monitored_subject = static_cast(0); - Crypto::Aes128KeyHandle shared_key = Crypto::Aes128KeyHandle(); + uint32_t start_icd_counter = 0; + uint32_t offset = 0; + uint64_t monitored_subject = static_cast(0); + Crypto::Aes128KeyHandle aes_key_handle = Crypto::Aes128KeyHandle(); + Crypto::Hmac128KeyHandle hmac_key_handle = Crypto::Hmac128KeyHandle(); ICDClientInfo() {} ICDClientInfo(const ICDClientInfo & other) { *this = other; } @@ -44,8 +45,11 @@ struct ICDClientInfo start_icd_counter = other.start_icd_counter; offset = other.offset; monitored_subject = other.monitored_subject; - ByteSpan buf(other.shared_key.As()); - memcpy(shared_key.AsMutable(), buf.data(), + ByteSpan aes_buf(other.aes_key_handle.As()); + memcpy(aes_key_handle.AsMutable(), aes_buf.data(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)); + ByteSpan hmac_buf(other.hmac_key_handle.As()); + memcpy(hmac_key_handle.AsMutable(), hmac_buf.data(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); return *this; } diff --git a/src/app/icd/client/ICDClientStorage.h b/src/app/icd/client/ICDClientStorage.h index 1d3e8edceec9f1..369926c3394853 100644 --- a/src/app/icd/client/ICDClientStorage.h +++ b/src/app/icd/client/ICDClientStorage.h @@ -50,7 +50,7 @@ class ICDClientStorage /** * Called during ICD device registration in commissioning, commissioner/controller - * provides raw key data, the shared key handle in clientInfo is updated based upon raw key data + * provides raw key data, the shared aes key handle and hmac key handle in clientInfo are updated based upon raw key data * * @param[inout] clientInfo the ICD Client information to be updated with keyData and be saved * @param[in] aKeyData raw key data provided by application diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 836b807dbbb44b..578111d35e7a16 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -92,6 +92,7 @@ inline constexpr size_t kAES_CCM128_Key_Length = 128u / 8u; inline constexpr size_t kAES_CCM128_Block_Length = kAES_CCM128_Key_Length; inline constexpr size_t kAES_CCM128_Nonce_Length = 13; inline constexpr size_t kAES_CCM128_Tag_Length = 16; +inline constexpr size_t kHMAC_CCM128_Key_Length = 128u / 8u; inline constexpr size_t CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES = kAES_CCM128_Nonce_Length;