[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 <[email protected]> * Update src/app/icd/client/DefaultICDClientStorage.cpp Co-authored-by: mkardous-silabs <[email protected]> * Restyled by clang-format --------- Co-authored-by: mkardous-silabs <[email protected]> Co-authored-by: Restyled.io <[email protected]>
project-chip · Feb 2, 2024 · e3d84f8 · e3d84f8
1 parent dae1270
commit e3d84f8
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 23 deletions.
diff --git 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<decltype(CHIPCommand::sSessionKeystore), Crypto::RawKeySessionKeystore>::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::Symmetric128BitsKeyByteArray>(), 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::Symmetric128BitsKeyByteArray>(), 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::Symmetric128BitsKeyByteArray>(), 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
@@ -258,13 +258,22 @@ CHIP_ERROR DefaultICDClientStorage::Load(FabricIndex fabricIndex, std::vector<IC
         ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kMonitoredSubject)));
         ReturnErrorOnFailure(reader.Get(clientInfo.monitored_subject));
 
-        // Shared key
-        ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kSharedKey)));
-        ByteSpan buf;
-        ReturnErrorOnFailure(reader.Get(buf));
-        VerifyOrReturnError(buf.size() == sizeof(Crypto::Symmetric128BitsKeyByteArray), CHIP_ERROR_INTERNAL);
-        memcpy(clientInfo.shared_key.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(), 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<Crypto::Symmetric128BitsKeyByteArray>(), 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<Crypto::Symmetric128BitsKeyByteArray>(), 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<ICDClientInfo> & 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<Crypto::Symmetric128BitsKeyByteArray>());
-        ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kSharedKey), buf));
+        ByteSpan aesBuf(clientInfo.aes_key_handle.As<Crypto::Symmetric128BitsKeyByteArray>());
+        ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kAesKeyHandle), aesBuf));
+        ByteSpan hmacBuf(clientInfo.hmac_key_handle.As<Crypto::Symmetric128BitsKeyByteArray>());
+        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
@@ -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
@@ -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<uint64_t>(0);
-    Crypto::Aes128KeyHandle shared_key = Crypto::Aes128KeyHandle();
+    uint32_t start_icd_counter               = 0;
+    uint32_t offset                          = 0;
+    uint64_t monitored_subject               = static_cast<uint64_t>(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<Crypto::Symmetric128BitsKeyByteArray>());
-        memcpy(shared_key.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(), buf.data(),
+        ByteSpan aes_buf(other.aes_key_handle.As<Crypto::Symmetric128BitsKeyByteArray>());
+        memcpy(aes_key_handle.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(), aes_buf.data(),
+               sizeof(Crypto::Symmetric128BitsKeyByteArray));
+        ByteSpan hmac_buf(other.hmac_key_handle.As<Crypto::Symmetric128BitsKeyByteArray>());
+        memcpy(hmac_key_handle.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(), hmac_buf.data(),
                sizeof(Crypto::Symmetric128BitsKeyByteArray));
         return *this;
     }

diff --git 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
@@ -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;