Skip to content

Commit a52bdf7

Browse files
mergify[bot]Alami-Aminepre-commit-ci[bot]
authored
apply ClearSecretData and SensitiveDataFixedBuffer consistently (#71994) (#72358)
* testing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * integrating comments --------- (cherry picked from commit fa18120) Co-authored-by: Amine Alami <43780877+Alami-Amine@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent bec9757 commit a52bdf7

12 files changed

Lines changed: 73 additions & 53 deletions

‎src/credentials/GroupDataProviderImpl.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ struct KeySetData : PersistableData<kPersistentBufferMax>
659659
{
660660
policy = GroupDataProvider::SecurityPolicy::kCacheAndSync;
661661
keys_count = 0;
662-
memset(operational_keys, 0x00, sizeof(operational_keys));
662+
Crypto::ClearSecretData(reinterpret_cast<uint8_t *>(operational_keys), sizeof(operational_keys));
663663
next = kInvalidKeysetId;
664664
}
665665

‎src/crypto/CHIPCryptoPAL.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -574,8 +574,8 @@ CHIP_ERROR Spake2pVerifier::Deserialize(const ByteSpan & inSerialized)
574574

575575
CHIP_ERROR Spake2pVerifier::Generate(uint32_t pbkdf2IterCount, const ByteSpan & salt, uint32_t setupPin)
576576
{
577-
uint8_t serializedWS[kSpake2p_WS_Length * 2] = { 0 };
578-
ReturnErrorOnFailure(ComputeWS(pbkdf2IterCount, salt, setupPin, serializedWS, sizeof(serializedWS)));
577+
SensitiveDataFixedBuffer<kSpake2p_WS_Length * 2> serializedWS;
578+
ReturnErrorOnFailure(ComputeWS(pbkdf2IterCount, salt, setupPin, serializedWS.Bytes(), serializedWS.Capacity()));
579579

580580
CHIP_ERROR err = CHIP_NO_ERROR;
581581
size_t len;
@@ -587,12 +587,12 @@ CHIP_ERROR Spake2pVerifier::Generate(uint32_t pbkdf2IterCount, const ByteSpan &
587587

588588
// Compute w0
589589
len = sizeof(mW0);
590-
SuccessOrExit(err = spake2p.ComputeW0(mW0, &len, &serializedWS[0], kSpake2p_WS_Length));
590+
SuccessOrExit(err = spake2p.ComputeW0(mW0, &len, serializedWS.Bytes(), kSpake2p_WS_Length));
591591
VerifyOrExit(len == sizeof(mW0), err = CHIP_ERROR_INTERNAL);
592592

593593
// Compute L
594594
len = sizeof(mL);
595-
SuccessOrExit(err = spake2p.ComputeL(mL, &len, &serializedWS[kSpake2p_WS_Length], kSpake2p_WS_Length));
595+
SuccessOrExit(err = spake2p.ComputeL(mL, &len, serializedWS.Bytes() + kSpake2p_WS_Length, kSpake2p_WS_Length));
596596
VerifyOrExit(len == sizeof(mL), err = CHIP_ERROR_INTERNAL);
597597

598598
exit:
@@ -604,15 +604,15 @@ CHIP_ERROR Spake2pVerifier::ComputeWS(uint32_t pbkdf2IterCount, const ByteSpan &
604604
uint32_t ws_len)
605605
{
606606
PBKDF2_sha256 pbkdf2;
607-
uint8_t littleEndianSetupPINCode[sizeof(uint32_t)];
608-
Encoding::LittleEndian::Put32(littleEndianSetupPINCode, setupPin);
607+
SensitiveDataFixedBuffer<sizeof(uint32_t)> littleEndianSetupPINCode;
608+
Encoding::LittleEndian::Put32(littleEndianSetupPINCode.Bytes(), setupPin);
609609

610610
VerifyOrReturnError(salt.size() >= kSpake2p_Min_PBKDF_Salt_Length && salt.size() <= kSpake2p_Max_PBKDF_Salt_Length,
611611
CHIP_ERROR_INVALID_ARGUMENT);
612612
VerifyOrReturnError(pbkdf2IterCount >= kSpake2p_Min_PBKDF_Iterations && pbkdf2IterCount <= kSpake2p_Max_PBKDF_Iterations,
613613
CHIP_ERROR_INVALID_ARGUMENT);
614614

615-
return pbkdf2.pbkdf2_sha256(littleEndianSetupPINCode, sizeof(littleEndianSetupPINCode), salt.data(), salt.size(),
615+
return pbkdf2.pbkdf2_sha256(littleEndianSetupPINCode.Bytes(), littleEndianSetupPINCode.Capacity(), salt.data(), salt.size(),
616616
pbkdf2IterCount, ws_len, ws);
617617
}
618618

‎src/crypto/CHIPCryptoPALOpenSSL.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,9 @@ void Spake2p_P256_SHA256_HKDF_HMAC::Clear()
10331033
free_bn(tempbn);
10341034
free_bn(order);
10351035

1036+
ClearSecretData(Kcab);
1037+
ClearSecretData(Kae);
1038+
10361039
state = CHIP_SPAKE2P_STATE::PREINIT;
10371040
}
10381041

‎src/crypto/CHIPCryptoPALPSA.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ void P256Keypair::Clear()
909909
{
910910
PsaP256KeypairContext & context = ToPsaContext(mKeypair);
911911
psa_destroy_key(context.key_id);
912-
memset(&context, 0, sizeof(context));
912+
ClearSecretData(reinterpret_cast<uint8_t *>(&context), sizeof(context));
913913
mInitialized = false;
914914
}
915915
}
@@ -1025,6 +1025,10 @@ void Spake2p_P256_SHA256_HKDF_HMAC::Clear()
10251025
mbedtls_mpi_free(&context->tempbn);
10261026

10271027
mbedtls_ecp_group_free(&context->curve);
1028+
1029+
ClearSecretData(Kcab);
1030+
ClearSecretData(Kae);
1031+
10281032
state = CHIP_SPAKE2P_STATE::PREINIT;
10291033
}
10301034

‎src/crypto/CHIPCryptoPALmbedTLS.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,10 @@ void Spake2p_P256_SHA256_HKDF_HMAC::Clear()
929929
mbedtls_mpi_free(&context->tempbn);
930930

931931
mbedtls_ecp_group_free(&context->curve);
932+
933+
ClearSecretData(Kcab);
934+
ClearSecretData(Kae);
935+
932936
state = CHIP_SPAKE2P_STATE::PREINIT;
933937
}
934938

‎src/crypto/RawKeySessionKeystore.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,12 @@ CHIP_ERROR RawKeySessionKeystore::DeriveSessionKeys(const ByteSpan & secret, con
7373
AttestationChallenge & attestationChallenge)
7474
{
7575
HKDF_sha hkdf;
76-
uint8_t keyMaterial[2 * sizeof(Symmetric128BitsKeyByteArray) + AttestationChallenge::Capacity()];
76+
SensitiveDataFixedBuffer<2 * sizeof(Symmetric128BitsKeyByteArray) + AttestationChallenge::Capacity()> keyMaterial;
7777

7878
ReturnErrorOnFailure(hkdf.HKDF_SHA256(secret.data(), secret.size(), salt.data(), salt.size(), info.data(), info.size(),
79-
keyMaterial, sizeof(keyMaterial)));
79+
keyMaterial.Bytes(), keyMaterial.Capacity()));
8080

81-
Encoding::LittleEndian::Reader reader(keyMaterial, sizeof(keyMaterial));
81+
Encoding::LittleEndian::Reader reader(keyMaterial.Bytes(), keyMaterial.Capacity());
8282

8383
return reader.ReadBytes(i2rKey.AsMutable<Symmetric128BitsKeyByteArray>(), sizeof(Symmetric128BitsKeyByteArray))
8484
.ReadBytes(r2iKey.AsMutable<Symmetric128BitsKeyByteArray>(), sizeof(Symmetric128BitsKeyByteArray))

‎src/lib/support/ThreadOperationalDataset.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,5 +497,11 @@ CHIP_ERROR OperationalDataset::SetDelayTimer(uint32_t aDelayMillis)
497497
return CHIP_NO_ERROR;
498498
}
499499

500+
void OperationalDataset::Clear()
501+
{
502+
memset(mBuffer, 0, sizeof(mBuffer));
503+
mData = ByteSpan(mBuffer, 0);
504+
}
505+
500506
} // namespace Thread
501507
} // namespace chip

‎src/lib/support/ThreadOperationalDataset.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ class OperationalDataset : public OperationalDatasetView
394394
/**
395395
* This method clears all data stored in the dataset.
396396
*/
397-
void Clear() { mData = ByteSpan(mBuffer, 0); }
397+
void Clear();
398398

399399
/**
400400
* Returns a ByteSpan view of the current state of the dataset.

‎src/protocols/secure_channel/CASESession.cpp‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -602,35 +602,36 @@ CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session)
602602
switch (mState)
603603
{
604604
case State::kFinished: {
605-
std::array<uint8_t, sizeof(mIPK) + kSHA256_Hash_Length> msg_salt;
605+
SensitiveDataFixedBuffer<sizeof(mIPK) + kSHA256_Hash_Length> msg_salt;
606606

607607
{
608-
Encoding::LittleEndian::BufferWriter bbuf(msg_salt);
608+
Encoding::LittleEndian::BufferWriter bbuf(msg_salt.Bytes(), msg_salt.Capacity());
609609
bbuf.Put(mIPK, sizeof(mIPK));
610610
bbuf.Put(mMessageDigest, sizeof(mMessageDigest));
611611

612612
VerifyOrReturnError(bbuf.Fit(), CHIP_ERROR_BUFFER_TOO_SMALL);
613613
}
614614

615615
ReturnErrorOnFailure(session.InitFromSecret(*mSessionManager->GetSessionKeystore(), mSharedSecret.Span(),
616-
ByteSpan(msg_salt), CryptoContext::SessionInfoType::kSessionEstablishment,
617-
mRole));
616+
ByteSpan(msg_salt.ConstBytes(), msg_salt.Capacity()),
617+
CryptoContext::SessionInfoType::kSessionEstablishment, mRole));
618618

619619
return CHIP_NO_ERROR;
620620
}
621621
case State::kFinishedViaResume: {
622-
std::array<uint8_t, sizeof(mInitiatorRandom) + decltype(mResumeResumptionId)().size()> msg_salt;
622+
SensitiveDataFixedBuffer<sizeof(mInitiatorRandom) + decltype(mResumeResumptionId)().size()> msg_salt;
623623

624624
{
625-
Encoding::LittleEndian::BufferWriter bbuf(msg_salt);
625+
Encoding::LittleEndian::BufferWriter bbuf(msg_salt.Bytes(), msg_salt.Capacity());
626626
bbuf.Put(mInitiatorRandom, sizeof(mInitiatorRandom));
627627
bbuf.Put(mResumeResumptionId.data(), mResumeResumptionId.size());
628628

629629
VerifyOrReturnError(bbuf.Fit(), CHIP_ERROR_BUFFER_TOO_SMALL);
630630
}
631631

632632
ReturnErrorOnFailure(session.InitFromSecret(*mSessionManager->GetSessionKeystore(), mSharedSecret.Span(),
633-
ByteSpan(msg_salt), CryptoContext::SessionInfoType::kSessionResumption, mRole));
633+
ByteSpan(msg_salt.ConstBytes(), msg_salt.Capacity()),
634+
CryptoContext::SessionInfoType::kSessionResumption, mRole));
634635

635636
return CHIP_NO_ERROR;
636637
}
@@ -642,6 +643,7 @@ CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session)
642643
CHIP_ERROR CASESession::RecoverInitiatorIpk()
643644
{
644645
Credentials::GroupDataProvider::KeySet ipkKeySet;
646+
auto ipkKeySetWiperOnScopeExit = ScopeExit([&] { ipkKeySet.ClearKeys(); });
645647

646648
CHIP_ERROR err = mGroupDataProvider->GetIpkKeySet(mFabricIndex, ipkKeySet);
647649

@@ -960,7 +962,8 @@ CHIP_ERROR CASESession::FindLocalNodeFromDestinationId(const ByteSpan & destinat
960962

961963
// Get IPK operational group key set for current candidate fabric
962964
GroupDataProvider::KeySet ipkKeySet;
963-
CHIP_ERROR err = mGroupDataProvider->GetIpkKeySet(fabricInfo.GetFabricIndex(), ipkKeySet);
965+
auto ipkKeySetWiperOnScopeExit = ScopeExit([&] { ipkKeySet.ClearKeys(); });
966+
CHIP_ERROR err = mGroupDataProvider->GetIpkKeySet(fabricInfo.GetFabricIndex(), ipkKeySet);
964967
if ((err != CHIP_NO_ERROR) ||
965968
((ipkKeySet.num_keys_used == 0) || (ipkKeySet.num_keys_used > Credentials::GroupDataProvider::KeySet::kEpochKeysMax)))
966969
{
@@ -1197,9 +1200,9 @@ CHIP_ERROR CASESession::PrepareSigma2(EncodeSigma2Inputs & outSigma2Data)
11971200
// Generate a Shared Secret
11981201
ReturnErrorOnFailure(mEphemeralKey->ECDH_derive_secret(mRemotePubKey, mSharedSecret));
11991202

1200-
uint8_t msgSalt[kIPKSize + kSigmaParamRandomNumberSize + kP256_PublicKey_Length + kSHA256_Hash_Length];
1203+
SensitiveDataFixedBuffer<kIPKSize + kSigmaParamRandomNumberSize + kP256_PublicKey_Length + kSHA256_Hash_Length> msgSalt;
12011204

1202-
MutableByteSpan saltSpan(msgSalt);
1205+
MutableByteSpan saltSpan(msgSalt.Bytes(), msgSalt.Capacity());
12031206
ReturnErrorOnFailure(
12041207
ConstructSaltSigma2(ByteSpan(outSigma2Data.responderRandom), mEphemeralKey->Pubkey(), ByteSpan(mIPK), saltSpan));
12051208

@@ -1498,8 +1501,8 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
14981501
// Generate the S2K key
14991502
AutoReleaseSessionKey sr2k(*mSessionManager->GetSessionKeystore());
15001503
{
1501-
uint8_t msg_salt[kIPKSize + kSigmaParamRandomNumberSize + kP256_PublicKey_Length + kSHA256_Hash_Length];
1502-
MutableByteSpan saltSpan(msg_salt);
1504+
SensitiveDataFixedBuffer<kIPKSize + kSigmaParamRandomNumberSize + kP256_PublicKey_Length + kSHA256_Hash_Length> msg_salt;
1505+
MutableByteSpan saltSpan(msg_salt.Bytes(), msg_salt.Capacity());
15031506
ReturnErrorOnFailure(ConstructSaltSigma2(parsedSigma2.responderRandom, mRemotePubKey, ByteSpan(mIPK), saltSpan));
15041507
ReturnErrorOnFailure(DeriveSigmaKey(saltSpan, ByteSpan(kKDFSR2Info), sr2k));
15051508
}
@@ -1825,7 +1828,7 @@ CHIP_ERROR CASESession::SendSigma3c(SendSigma3Data & data, CHIP_ERROR status)
18251828
System::PacketBufferHandle msg_R3;
18261829
size_t data_len;
18271830

1828-
uint8_t msg_salt[kIPKSize + kSHA256_Hash_Length];
1831+
SensitiveDataFixedBuffer<kIPKSize + kSHA256_Hash_Length> msg_salt;
18291832

18301833
AutoReleaseSessionKey sr3k(*mSessionManager->GetSessionKeystore());
18311834

@@ -1835,7 +1838,7 @@ CHIP_ERROR CASESession::SendSigma3c(SendSigma3Data & data, CHIP_ERROR status)
18351838

18361839
// Generate S3K key
18371840
{
1838-
MutableByteSpan saltSpan(msg_salt);
1841+
MutableByteSpan saltSpan(msg_salt.Bytes(), msg_salt.Capacity());
18391842
SuccessOrExit(err = ConstructSaltSigma3(ByteSpan(mIPK), saltSpan));
18401843
SuccessOrExit(err = DeriveSigmaKey(saltSpan, ByteSpan(kKDFSR3Info), sr3k));
18411844
}
@@ -1914,7 +1917,7 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg)
19141917

19151918
AutoReleaseSessionKey sr3k(*mSessionManager->GetSessionKeystore());
19161919

1917-
uint8_t msg_salt[kIPKSize + kSHA256_Hash_Length];
1920+
SensitiveDataFixedBuffer<kIPKSize + kSHA256_Hash_Length> msg_salt;
19181921

19191922
ChipLogProgress(SecureChannel, "Received Sigma3 msg");
19201923
MATTER_TRACE_COUNTER("Sigma3");
@@ -1946,7 +1949,7 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg)
19461949
SuccessOrExit(err = ParseSigma3(tlvReader, msgR3Encrypted, msgR3EncryptedPayload, msgR3MIC));
19471950

19481951
// Generate the S3K key
1949-
MutableByteSpan saltSpan(msg_salt);
1952+
MutableByteSpan saltSpan(msg_salt.Bytes(), msg_salt.Capacity());
19501953
SuccessOrExit(err = ConstructSaltSigma3(ByteSpan(mIPK), saltSpan));
19511954
SuccessOrExit(err = DeriveSigmaKey(saltSpan, ByteSpan(kKDFSR3Info), sr3k));
19521955

‎src/protocols/secure_channel/CheckinMessage.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,18 @@ CHIP_ERROR CheckinMessage::GenerateCheckInMessageNonce(const Crypto::Hmac128KeyH
138138
{
139139
VerifyOrReturnError(writer.Available() >= CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, CHIP_ERROR_BUFFER_TOO_SMALL);
140140

141-
uint8_t hashWorkBuffer[CHIP_CRYPTO_HASH_LEN_BYTES] = { 0 };
141+
Crypto::SensitiveDataFixedBuffer<CHIP_CRYPTO_HASH_LEN_BYTES> hashWorkBuffer;
142142
uint8_t counterBuffer[sizeof(CounterType)];
143143

144144
// validate that Check-In counter is a uint32_t
145145
static_assert(sizeof(CounterType) == sizeof(uint32_t), "Expect counter to be 32 bits for correct encoding");
146146
Encoding::LittleEndian::Put32(counterBuffer, counter);
147147

148148
chip::Crypto::HMAC_sha shaHandler;
149-
ReturnErrorOnFailure(
150-
shaHandler.HMAC_SHA256(hmacKeyHandle, counterBuffer, sizeof(CounterType), hashWorkBuffer, CHIP_CRYPTO_HASH_LEN_BYTES));
149+
ReturnErrorOnFailure(shaHandler.HMAC_SHA256(hmacKeyHandle, counterBuffer, sizeof(CounterType), hashWorkBuffer.Bytes(),
150+
hashWorkBuffer.Capacity()));
151151

152-
writer.Put(hashWorkBuffer, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES);
152+
writer.Put(hashWorkBuffer.Bytes(), CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES);
153153
VerifyOrReturnError(writer.Fit(), CHIP_ERROR_BUFFER_TOO_SMALL);
154154

155155
return CHIP_NO_ERROR;

0 commit comments

Comments
 (0)