Quick editorial follow-ups on IPK for CASE (#16856)

* Quick editorial follow-ups on IPK for CASE - Fixes the follow-up editorial comments from @bzbarsky-apple on #16737 - Unit tests still passa - Cert tests pass * Restyled by clang-format Co-authored-by: Restyled.io <[email protected]>
project-chip · Mar 31, 2022 · 5ec5bb1 · 5ec5bb1
1 parent 6c692ab
commit 5ec5bb1
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 17 deletions.
diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h
@@ -95,7 +95,6 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,
     OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId) : mSecureSession(*this)
     {
         mInitParams = params;
-        // Do not do worse
         if (params.Validate() != CHIP_NO_ERROR)
         {
             mState = State::Uninitialized;

diff --git a/src/controller/ExampleOperationalCredentialsIssuer.cpp b/src/controller/ExampleOperationalCredentialsIssuer.cpp
@@ -243,14 +243,12 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan
     Crypto::AesCcm128KeySpan ipkSpan(ipkValue);
 
     ReturnErrorCodeIf(defaultIpkSpan.size() != sizeof(ipkValue), CHIP_ERROR_INTERNAL);
-
     memcpy(&ipkValue[0], defaultIpkSpan.data(), defaultIpkSpan.size());
-    Optional<Crypto::AesCcm128KeySpan> ipkSpanValue;
-    ipkSpanValue.SetValue(ipkSpan);
 
     // Callback onto commissioner.
     ChipLogProgress(Controller, "Providing certificate chain to the commissioner");
-    onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, icacSpan, rcacSpan, ipkSpanValue, Optional<NodeId>());
+    onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, icacSpan, rcacSpan, MakeOptional(ipkSpan),
+                        Optional<NodeId>());
     return CHIP_NO_ERROR;
 }
 

diff --git a/src/controller/java/AndroidOperationalCredentialsIssuer.cpp b/src/controller/java/AndroidOperationalCredentialsIssuer.cpp
@@ -196,11 +196,10 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan
     ReturnErrorCodeIf(defaultIpkSpan.size() != sizeof(ipkValue), CHIP_ERROR_INTERNAL);
 
     memcpy(&ipkValue[0], defaultIpkSpan.data(), defaultIpkSpan.size());
-    Optional<Crypto::AesCcm128KeySpan> ipkSpanValue;
-    ipkSpanValue.SetValue(ipkSpan);
 
     // Call-back into commissioner with the generated data.
-    onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, ByteSpan(), rcacSpan, ipkSpanValue, Optional<NodeId>());
+    onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, ByteSpan(), rcacSpan, MakeOptional(ipkSpan),
+                        Optional<NodeId>());
 
     jbyteArray javaCsr;
     JniReferences::GetInstance().GetEnvForCurrentThread()->ExceptionClear();

diff --git a/src/credentials/GroupDataProviderImpl.cpp b/src/credentials/GroupDataProviderImpl.cpp
@@ -1830,7 +1830,7 @@ CHIP_ERROR GroupDataProviderImpl::GetIpkKeySet(FabricIndex fabric_index, KeySet
 
     KeyMapData mapping(fabric.fabric_index, fabric.first_map);
 
-    // Group found, get the keyset
+    // Fabric found, get the keyset
     KeySetData keyset;
     VerifyOrReturnError(keyset.Find(mStorage, fabric, kIdentityProtectionKeySetId), CHIP_ERROR_NOT_FOUND);
 
@@ -1841,7 +1841,7 @@ CHIP_ERROR GroupDataProviderImpl::GetIpkKeySet(FabricIndex fabric_index, KeySet
     out_keyset.num_keys_used = keyset.keys_count;
     out_keyset.policy        = keyset.policy;
 
-    for (size_t key_idx = 0; key_idx < KeySet::kEpochKeysMax; ++key_idx)
+    for (size_t key_idx = 0; key_idx < ArraySize(out_keyset.epoch_keys); ++key_idx)
     {
         out_keyset.epoch_keys[key_idx].Clear();
         if (key_idx < keyset.keys_count)

diff --git a/src/protocols/secure_channel/CASEDestinationId.cpp b/src/protocols/secure_channel/CASEDestinationId.cpp
@@ -52,6 +52,12 @@ CHIP_ERROR GenerateCaseDestinationId(const ByteSpan & ipk, const ByteSpan & init
     HMAC_sha hmac;
     CHIP_ERROR err =
         hmac.HMAC_SHA256(ipk.data(), ipk.size(), bbuf.Buffer(), written, outDestinationId.data(), outDestinationId.size());
+
+    if (err == CHIP_NO_ERROR)
+    {
+        outDestinationId.reduce_size(kSHA256_Hash_Length);
+    }
+
     return err;
 }
 

diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
@@ -335,9 +335,9 @@ CHIP_ERROR CASESession::RecoverInitiatorIpk()
     size_t ipkIndex = (ipkKeySet.num_keys_used > 1) ? ((ipkKeySet.num_keys_used - 1) - 1) : 0;
     memcpy(&mIPK[0], ipkKeySet.epoch_keys[ipkIndex].key, sizeof(mIPK));
 
-    ChipLogProgress(Support, "RecoverInitiatorIpk: GroupDataProvider %p, Got IPK for FabricIndex %u", mGroupDataProvider,
-                    (unsigned) mFabricInfo->GetFabricIndex());
-    ChipLogByteSpan(Support, ByteSpan(mIPK));
+    ChipLogProgress(SecureChannel, "RecoverInitiatorIpk: GroupDataProvider %p, Got IPK for FabricIndex %u", mGroupDataProvider,
+                    static_cast<unsigned>(mFabricInfo->GetFabricIndex()));
+    ChipLogByteSpan(SecureChannel, ByteSpan(mIPK));
 
     return CHIP_NO_ERROR;
 }
@@ -383,7 +383,7 @@ CHIP_ERROR CASESession::SendSigma1()
 
         FabricId fabricId = mFabricInfo->GetFabricId();
         uint8_t rootPubKeyBuf[Crypto::kP256_Point_Length];
-        Credentials::P256PublicKeySpan rootPubKeySpan(&rootPubKeyBuf[0]);
+        Credentials::P256PublicKeySpan rootPubKeySpan(rootPubKeyBuf);
         ReturnErrorOnFailure(mFabricInfo->GetRootPubkey(rootPubKeySpan));
 
         MutableByteSpan destinationIdSpan(destinationIdentifier);
@@ -453,7 +453,7 @@ CHIP_ERROR CASESession::FindLocalNodeFromDestionationId(const ByteSpan & destina
         FabricId fabricId = fabricInfo.GetFabricId();
         NodeId nodeId     = fabricInfo.GetNodeId();
         uint8_t rootPubKeyBuf[Crypto::kP256_Point_Length];
-        Credentials::P256PublicKeySpan rootPubKeySpan(&rootPubKeyBuf[0]);
+        Credentials::P256PublicKeySpan rootPubKeySpan(rootPubKeyBuf);
         ReturnErrorOnFailure(fabricInfo.GetRootPubkey(rootPubKeySpan));
 
         // Get IPK operational group key set for current candidate fabric

diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
@@ -108,7 +108,7 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
                      Optional<ReliableMessageProtocolConfig> mrpConfig = Optional<ReliableMessageProtocolConfig>::Missing());
 
     /**
-     * @brief Set the Group Data Provider which will be used to look-up IPKs
+     * @brief Set the Group Data Provider which will be used to look up IPKs
      *
      * The GroupDataProvider set MUST have key sets available through `GetIpkKeySet` method
      * for the FabricIndex that is associated with the CASESession's FabricInfo.
@@ -195,7 +195,7 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
     // On success, sets mIpk to the correct value for outgoing Sigma1 based on internal state
     CHIP_ERROR RecoverInitiatorIpk();
     // On success, sets locally maching mFabricInfo in internal state to the entry matched by
-    // destinationId/initiatorRandom from processing of Sigma1
+    // destinationId/initiatorRandom from processing of Sigma1, and sets mIpk to the right IPK.
     CHIP_ERROR FindLocalNodeFromDestionationId(const ByteSpan & destinationId, const ByteSpan & initiatorRandom);
 
     CHIP_ERROR SendSigma1();