From 5ec5bb14ef4a977ef9ddd432a8f80b696eef5443 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Wed, 30 Mar 2022 21:37:53 -0400 Subject: [PATCH] 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 --- src/app/OperationalDeviceProxy.h | 1 - src/controller/ExampleOperationalCredentialsIssuer.cpp | 6 ++---- .../java/AndroidOperationalCredentialsIssuer.cpp | 5 ++--- src/credentials/GroupDataProviderImpl.cpp | 4 ++-- src/protocols/secure_channel/CASEDestinationId.cpp | 6 ++++++ src/protocols/secure_channel/CASESession.cpp | 10 +++++----- src/protocols/secure_channel/CASESession.h | 4 ++-- 7 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index ce5275d6d6e088..463f05061716ad 100644 --- 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 index d273ca169194d8..5271c71cb58882 100644 --- 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 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()); + onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, icacSpan, rcacSpan, MakeOptional(ipkSpan), + Optional()); return CHIP_NO_ERROR; } diff --git a/src/controller/java/AndroidOperationalCredentialsIssuer.cpp b/src/controller/java/AndroidOperationalCredentialsIssuer.cpp index bedc27a4bec7d5..191dea78a50181 100644 --- 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 ipkSpanValue; - ipkSpanValue.SetValue(ipkSpan); // Call-back into commissioner with the generated data. - onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, ByteSpan(), rcacSpan, ipkSpanValue, Optional()); + onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, ByteSpan(), rcacSpan, MakeOptional(ipkSpan), + Optional()); jbyteArray javaCsr; JniReferences::GetInstance().GetEnvForCurrentThread()->ExceptionClear(); diff --git a/src/credentials/GroupDataProviderImpl.cpp b/src/credentials/GroupDataProviderImpl.cpp index c3e2069e0e674d..0925b667bc5d8f 100644 --- 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 index 396177da9901be..17eac22b1409a5 100644 --- 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 index a68e524ac2ded2..0ef0f76c0f6efc 100644 --- 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(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 index b945affa127087..63e51073b64606 100644 --- 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 mrpConfig = Optional::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();