From a3aa768dcb3809e5e543064ef2d22238c61d0761 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Wed, 26 Jul 2023 12:01:39 +0000 Subject: [PATCH 1/7] Fix uninitialised value in KeySet --- src/credentials/GroupDataProvider.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h index 7d0665ce2fd6ce..72584088a08f5c 100644 --- a/src/credentials/GroupDataProvider.h +++ b/src/credentials/GroupDataProvider.h @@ -143,7 +143,7 @@ class GroupDataProvider {} // The actual keys for the group key set - EpochKey epoch_keys[kEpochKeysMax]; + EpochKey epoch_keys[kEpochKeysMax] = {}; // Logical id provided by the Administrator that configured the entry uint16_t keyset_id = 0; // Security policy to use for groups that use this keyset @@ -202,7 +202,7 @@ class GroupDataProvider virtual ~GroupDataProvider() = default; // Not copyable - GroupDataProvider(const GroupDataProvider &) = delete; + GroupDataProvider(const GroupDataProvider &) = delete; GroupDataProvider & operator=(const GroupDataProvider &) = delete; uint16_t GetMaxGroupsPerFabric() const { return mMaxGroupsPerFabric; } From cef1b7505994d2da0b084000f8a00f5764d9d0c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Thu, 27 Jul 2023 11:22:56 +0000 Subject: [PATCH 2/7] Fix uninitilised mGlobalAttributeEndIndex --- src/app/AttributePathExpandIterator.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/AttributePathExpandIterator.cpp b/src/app/AttributePathExpandIterator.cpp index 6c45055c3933b0..b98cd63606eb09 100644 --- a/src/app/AttributePathExpandIterator.cpp +++ b/src/app/AttributePathExpandIterator.cpp @@ -65,7 +65,8 @@ AttributePathExpandIterator::AttributePathExpandIterator(ObjectList::value, "If this changes audit all uses where we set to UINT8_MAX"); - mGlobalAttributeIndex = UINT8_MAX; + mGlobalAttributeIndex = UINT8_MAX; + mGlobalAttributeEndIndex = 0; // Make the iterator ready to emit the first valid path in the list. Next(); From be22ef1bcfef986c50aff92097b1315d11a9b32e Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 28 Jul 2023 09:24:38 +0000 Subject: [PATCH 3/7] Restyled by clang-format --- src/credentials/GroupDataProvider.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h index 72584088a08f5c..6beac89aaa8bef 100644 --- a/src/credentials/GroupDataProvider.h +++ b/src/credentials/GroupDataProvider.h @@ -202,7 +202,7 @@ class GroupDataProvider virtual ~GroupDataProvider() = default; // Not copyable - GroupDataProvider(const GroupDataProvider &) = delete; + GroupDataProvider(const GroupDataProvider &) = delete; GroupDataProvider & operator=(const GroupDataProvider &) = delete; uint16_t GetMaxGroupsPerFabric() const { return mMaxGroupsPerFabric; } From 2ad9f75b7d8788bdc0abc0bdc62667e2ef839767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Mon, 31 Jul 2023 11:32:04 +0000 Subject: [PATCH 4/7] Review proposed change initilize mGlobalAttributeEndIndex --- src/app/AttributePathExpandIterator.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/app/AttributePathExpandIterator.cpp b/src/app/AttributePathExpandIterator.cpp index b98cd63606eb09..5c1bb50c179f47 100644 --- a/src/app/AttributePathExpandIterator.cpp +++ b/src/app/AttributePathExpandIterator.cpp @@ -65,8 +65,7 @@ AttributePathExpandIterator::AttributePathExpandIterator(ObjectList::value, "If this changes audit all uses where we set to UINT8_MAX"); - mGlobalAttributeIndex = UINT8_MAX; - mGlobalAttributeEndIndex = 0; + mGlobalAttributeIndex = UINT8_MAX; // Make the iterator ready to emit the first valid path in the list. Next(); @@ -142,6 +141,11 @@ void AttributePathExpandIterator::PrepareAttributeIndexRange(const AttributePath } mGlobalAttributeEndIndex = static_cast(mGlobalAttributeIndex + 1); } + else + { + mGlobalAttributeIndex = UINT8_MAX; + mGlobalAttributeEndIndex = 0; + } } } From 94157b6c5f144505789189853eff0d375c0b34e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Mon, 31 Jul 2023 11:39:36 +0000 Subject: [PATCH 5/7] Change initialize values in KeySet to initilize only start_time in EpochKey --- src/credentials/GroupDataProvider.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h index 6beac89aaa8bef..b7484d778ad318 100644 --- a/src/credentials/GroupDataProvider.h +++ b/src/credentials/GroupDataProvider.h @@ -121,7 +121,7 @@ class GroupDataProvider { static constexpr size_t kLengthBytes = Crypto::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES; // Validity start time in microseconds since 2000-01-01T00:00:00 UTC ("the Epoch") - uint64_t start_time; + uint64_t start_time = 0; // Actual key bits. Depending on context, it may be a raw epoch key (as seen within `SetKeySet` calls) // or it may be the derived operational group key (as seen in any other usage). uint8_t key[kLengthBytes]; @@ -143,7 +143,7 @@ class GroupDataProvider {} // The actual keys for the group key set - EpochKey epoch_keys[kEpochKeysMax] = {}; + EpochKey epoch_keys[kEpochKeysMax]; // Logical id provided by the Administrator that configured the entry uint16_t keyset_id = 0; // Security policy to use for groups that use this keyset @@ -202,7 +202,7 @@ class GroupDataProvider virtual ~GroupDataProvider() = default; // Not copyable - GroupDataProvider(const GroupDataProvider &) = delete; + GroupDataProvider(const GroupDataProvider &) = delete; GroupDataProvider & operator=(const GroupDataProvider &) = delete; uint16_t GetMaxGroupsPerFabric() const { return mMaxGroupsPerFabric; } From cf7e1e4f3d89bcf2d743f5c86a42e16ce445ea89 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 31 Jul 2023 12:29:02 +0000 Subject: [PATCH 6/7] Restyled by clang-format --- src/credentials/GroupDataProvider.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h index b7484d778ad318..6684d4bea9cfd0 100644 --- a/src/credentials/GroupDataProvider.h +++ b/src/credentials/GroupDataProvider.h @@ -202,7 +202,7 @@ class GroupDataProvider virtual ~GroupDataProvider() = default; // Not copyable - GroupDataProvider(const GroupDataProvider &) = delete; + GroupDataProvider(const GroupDataProvider &) = delete; GroupDataProvider & operator=(const GroupDataProvider &) = delete; uint16_t GetMaxGroupsPerFabric() const { return mMaxGroupsPerFabric; } From 5cdefd8680378e7acdd2588d08fdec4020c685fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Tue, 1 Aug 2023 07:46:57 +0000 Subject: [PATCH 7/7] Review proposed change initilize IPK keyset start_time --- .../operational-credentials-server.cpp | 7 ++++--- src/credentials/GroupDataProvider.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index 5a6c645a510670..4340f683b0d666 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -654,9 +654,10 @@ bool emberAfOperationalCredentialsClusterAddNOCCallback(app::CommandHandler * co // Set the Identity Protection Key (IPK) // The IPK SHALL be the operational group key under GroupKeySetID of 0 - keyset.keyset_id = Credentials::GroupDataProvider::kIdentityProtectionKeySetId; - keyset.policy = GroupKeyManagement::GroupKeySecurityPolicyEnum::kTrustFirst; - keyset.num_keys_used = 1; + keyset.keyset_id = Credentials::GroupDataProvider::kIdentityProtectionKeySetId; + keyset.policy = GroupKeyManagement::GroupKeySecurityPolicyEnum::kTrustFirst; + keyset.num_keys_used = 1; + keyset.epoch_keys[0].start_time = 0; memcpy(keyset.epoch_keys[0].key, ipkValue.data(), Crypto::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES); err = newFabricInfo->GetCompressedFabricIdBytes(compressed_fabric_id); diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h index 6684d4bea9cfd0..7d0665ce2fd6ce 100644 --- a/src/credentials/GroupDataProvider.h +++ b/src/credentials/GroupDataProvider.h @@ -121,7 +121,7 @@ class GroupDataProvider { static constexpr size_t kLengthBytes = Crypto::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES; // Validity start time in microseconds since 2000-01-01T00:00:00 UTC ("the Epoch") - uint64_t start_time = 0; + uint64_t start_time; // Actual key bits. Depending on context, it may be a raw epoch key (as seen within `SetKeySet` calls) // or it may be the derived operational group key (as seen in any other usage). uint8_t key[kLengthBytes];