Skip to content

Commit b085862

Browse files
Alami-AmineAlami-Amine
committed
various cleanup
1 parent 403d4ee commit b085862

File tree

1 file changed

+34
-30
lines changed

1 file changed

+34
-30
lines changed

‎src/protocols/secure_channel/CASESession.cpp

+34-30
Original file line numberDiff line numberDiff line change
@@ -1265,7 +1265,7 @@ CHIP_ERROR CASESession::PrepareSigma2(EncodeSigma2Inputs & outSigma2Data)
12651265
size_t msgR2SignedLen = EstimateStructOverhead(kMaxCHIPCertLength, // responderNoc
12661266
kMaxCHIPCertLength, // responderICAC
12671267
kP256_PublicKey_Length, // responderEphPubKey
1268-
kP256_PublicKey_Length // InitiatorEphPubKey
1268+
kP256_PublicKey_Length // initiatorEphPubKey
12691269
);
12701270

12711271
P256ECDSASignature tbsData2Signature;
@@ -1494,7 +1494,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
14941494

14951495
chip::Platform::ScopedMemoryBuffer<uint8_t> msg_R2_Encrypted;
14961496

1497-
chip::Platform::ScopedMemoryBuffer<uint8_t> msg_R2_Signed;
1497+
chip::Platform::ScopedMemoryBuffer<uint8_t> msgR2Signed;
14981498

14991499
AutoReleaseSessionKey sr2k(*mSessionManager->GetSessionKeystore());
15001500

@@ -1510,9 +1510,9 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
15101510
// uint16_t responderSessionId;
15111511

15121512
// TLVType containerType = kTLVType_Structure;
1513-
size_t msg_r2_signed_len;
1513+
size_t msgR2SignedLen;
15141514

1515-
size_t msg_r2_encrypted_len = 0;
1515+
size_t msgR2EncryptedLen = 0;
15161516

15171517
ChipLogProgress(SecureChannel, "Received Sigma2 msg");
15181518
CHIP_ERROR err = CHIP_NO_ERROR;
@@ -1534,7 +1534,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
15341534
VerifyOrExit(mEphemeralKey != nullptr, err = CHIP_ERROR_INTERNAL);
15351535
VerifyOrExit(buf != nullptr, err = CHIP_ERROR_MESSAGE_INCOMPLETE);
15361536

1537-
ParseSigma2(tlvReader, parsedSigma2);
1537+
SuccessOrExit(err = ParseSigma2(tlvReader, parsedSigma2));
15381538

15391539
ChipLogDetail(SecureChannel, "Peer assigned session key ID %d", parsedSigma2.responderSessionId);
15401540
SetPeerSessionId(parsedSigma2.responderSessionId);
@@ -1553,8 +1553,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
15531553
err = ConstructSaltSigma2(ByteSpan(parsedSigma2.responderRandom), mRemotePubKey, ByteSpan(mIPK), saltSpan));
15541554
ReturnErrorOnFailure(err = DeriveSigmaKey(saltSpan, ByteSpan(kKDFSR2Info), sr2k));
15551555
}
1556-
// TODO verify location of this
1557-
// TODO why does this trigger a fialure if i move it just after call to PArseSigma2
1556+
// Msg2 should only be added to MessageDigest after we construct SaltSigma2 that is used to derive S2K
15581557
ReturnErrorOnFailure(err = mCommissioningHash.AddData(ByteSpan{ buf, buflen }));
15591558

15601559
if (parsedSigma2.responderMrpParamsPresent)
@@ -1566,19 +1565,19 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
15661565
/**********************************************************************************************************************************
15671566
*/
15681567

1569-
msg_r2_encrypted_len = parsedSigma2.msgR2Encrypted.AllocatedSize() - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES;
1568+
msgR2EncryptedLen = parsedSigma2.msgR2Encrypted.AllocatedSize() - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES;
15701569

1571-
SuccessOrExit(err = AES_CCM_decrypt(parsedSigma2.msgR2Encrypted.Get(), msg_r2_encrypted_len, nullptr, 0,
1572-
parsedSigma2.msgR2Encrypted.Get() + msg_r2_encrypted_len, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES,
1570+
SuccessOrExit(err = AES_CCM_decrypt(parsedSigma2.msgR2Encrypted.Get(), msgR2EncryptedLen, nullptr, 0,
1571+
parsedSigma2.msgR2Encrypted.Get() + msgR2EncryptedLen, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES,
15731572
sr2k.KeyHandle(), kTBEData2_Nonce, kTBEDataNonceLength, parsedSigma2.msgR2Encrypted.Get()));
15741573

1575-
decryptedDataTlvReader.Init(parsedSigma2.msgR2Encrypted.Get(), msg_r2_encrypted_len);
1574+
decryptedDataTlvReader.Init(parsedSigma2.msgR2Encrypted.Get(), msgR2EncryptedLen);
15761575

1577-
ParseSigma2TBEData(decryptedDataTlvReader, parsedSigma2TBEData);
1576+
SuccessOrExit(err = ParseSigma2TBEData(decryptedDataTlvReader, parsedSigma2TBEData));
15781577

15791578
std::copy(parsedSigma2TBEData.resumptionId.begin(), parsedSigma2TBEData.resumptionId.end(), mNewResumptionId.begin());
15801579

1581-
// Validate responder identity located in msg_r2_encrypted
1580+
// Validate responder identity located in msgR2Encrypted
15821581
// Constructing responder identity
15831582
{
15841583
CompressedFabricId unused;
@@ -1593,20 +1592,20 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
15931592
VerifyOrExit(mPeerNodeId == responderNodeId, err = CHIP_ERROR_INVALID_CASE_PARAMETER);
15941593
}
15951594

1596-
// Construct msg_R2_Signed and validate the signature in msg_r2_encrypted
1597-
msg_r2_signed_len =
1595+
// Construct msgR2Signed and validate the signature in msgR2Encrypted.
1596+
msgR2SignedLen =
15981597
EstimateStructOverhead(sizeof(uint16_t), parsedSigma2TBEData.responderNOC.size(), parsedSigma2TBEData.responderICAC.size(),
15991598
kP256_PublicKey_Length, kP256_PublicKey_Length);
16001599

1601-
VerifyOrExit(msg_R2_Signed.Alloc(msg_r2_signed_len), err = CHIP_ERROR_NO_MEMORY);
1600+
VerifyOrExit(msgR2Signed.Alloc(msgR2SignedLen), err = CHIP_ERROR_NO_MEMORY);
16021601

16031602
SuccessOrExit(err = ConstructTBSData(parsedSigma2TBEData.responderNOC, parsedSigma2TBEData.responderICAC,
16041603
ByteSpan(mRemotePubKey, mRemotePubKey.Length()),
1605-
ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), msg_R2_Signed.Get(),
1606-
msg_r2_signed_len));
1604+
ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), msgR2Signed.Get(),
1605+
msgR2SignedLen));
16071606

16081607
// Validate signature
1609-
SuccessOrExit(err = responderPublicKey.ECDSA_validate_msg_signature(msg_R2_Signed.Get(), msg_r2_signed_len,
1608+
SuccessOrExit(err = responderPublicKey.ECDSA_validate_msg_signature(msgR2Signed.Get(), msgR2SignedLen,
16101609
parsedSigma2TBEData.tbsData2Signature));
16111610

16121611
// Retrieve peer CASE Authenticated Tags (CATs) from peer's NOC.
@@ -1631,12 +1630,15 @@ CHIP_ERROR CASESession::ParseSigma2TBEData(ContiguousBufferTLVReader & decrypted
16311630

16321631
ReturnErrorOnFailure(err = decryptedDataTlvReader.Next(kTLVType_ByteString, AsTlvContextTag(TBEDataTags::kSenderNOC)));
16331632
ReturnErrorOnFailure(err = decryptedDataTlvReader.GetByteView(outParsedSigma2TBE.responderNOC));
1633+
VerifyOrReturnError(outParsedSigma2TBE.responderNOC.size() <= kMaxCHIPCertLength, CHIP_ERROR_INVALID_CASE_PARAMETER);
16341634

16351635
ReturnErrorOnFailure(err = decryptedDataTlvReader.Next());
16361636
if (decryptedDataTlvReader.GetTag() == AsTlvContextTag(TBEDataTags::kSenderICAC))
16371637
{
16381638
VerifyOrReturnError(decryptedDataTlvReader.GetType() == kTLVType_ByteString, err = CHIP_ERROR_WRONG_TLV_TYPE);
16391639
ReturnErrorOnFailure(err = decryptedDataTlvReader.GetByteView(outParsedSigma2TBE.responderICAC));
1640+
VerifyOrReturnError(outParsedSigma2TBE.responderICAC.size() <= kMaxCHIPCertLength, CHIP_ERROR_INVALID_CASE_PARAMETER);
1641+
16401642
ReturnErrorOnFailure(err = decryptedDataTlvReader.Next(kTLVType_ByteString, AsTlvContextTag(TBEDataTags::kSignature)));
16411643
}
16421644

@@ -1651,6 +1653,8 @@ CHIP_ERROR CASESession::ParseSigma2TBEData(ContiguousBufferTLVReader & decrypted
16511653
// Retrieve session resumption ID
16521654
ReturnErrorOnFailure(err = decryptedDataTlvReader.Next(kTLVType_ByteString, AsTlvContextTag(TBEDataTags::kResumptionID)));
16531655
ReturnErrorOnFailure(err = decryptedDataTlvReader.GetByteView(outParsedSigma2TBE.resumptionId));
1656+
VerifyOrReturnError(outParsedSigma2TBE.resumptionId.size() == SessionResumptionStorage::kResumptionIdSize,
1657+
CHIP_ERROR_INVALID_CASE_PARAMETER);
16541658

16551659
return CHIP_NO_ERROR;
16561660
}
@@ -2412,30 +2416,30 @@ CHIP_ERROR CASESession::ParseSigma2(ContiguousBufferTLVReader & tlvReader, Parse
24122416
ReturnErrorOnFailure(err = tlvReader.Next(kTLVType_ByteString, AsTlvContextTag(Sigma2Tags::kEncrypted2)));
24132417

24142418
// TODO find a solution to this
2415-
// size_t msg_r2_encrypted_len = 0;
2419+
// size_t msgR2EncryptedLen = 0;
24162420

2417-
size_t msg_r2_encrypted_len_with_tag = 0;
2421+
size_t msgR2EncryptedLenWithTag = 0;
24182422

24192423
constexpr size_t kCaseOverheadForFutureTbeData = 128;
24202424

2421-
size_t max_msg_r2_signed_enc_len;
2425+
size_t maxMsgR2SignedEncLen;
24222426

2423-
max_msg_r2_signed_enc_len =
2424-
EstimateStructOverhead(Credentials::kMaxCHIPCertLength, Credentials::kMaxCHIPCertLength, tbsData2Signature.Length(),
2425-
SessionResumptionStorage::kResumptionIdSize, kCaseOverheadForFutureTbeData);
2426-
msg_r2_encrypted_len_with_tag = tlvReader.GetLength();
2427+
maxMsgR2SignedEncLen = EstimateStructOverhead(kMaxCHIPCertLength, kMaxCHIPCertLength, tbsData2Signature.Length(),
2428+
SessionResumptionStorage::kResumptionIdSize, kCaseOverheadForFutureTbeData);
2429+
msgR2EncryptedLenWithTag = tlvReader.GetLength();
24272430

24282431
// Validate we did not receive a buffer larger than legal
24292432
// TODO why are comparing this to the "signed one"
24302433
// Maybe it is signed encrypted, but is the estimation calculated well? document it
2431-
VerifyOrReturnError(msg_r2_encrypted_len_with_tag <= max_msg_r2_signed_enc_len, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
2432-
VerifyOrReturnError(msg_r2_encrypted_len_with_tag > CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
2433-
VerifyOrReturnError(outParsedSigma2.msgR2Encrypted.Alloc(msg_r2_encrypted_len_with_tag), err = CHIP_ERROR_NO_MEMORY);
2434+
VerifyOrReturnError(msgR2EncryptedLenWithTag <= maxMsgR2SignedEncLen, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
2435+
VerifyOrReturnError(msgR2EncryptedLenWithTag > CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
2436+
// Allocate after making sure that msgR2EncryptedLenWithTag is within bounds
2437+
VerifyOrReturnError(outParsedSigma2.msgR2Encrypted.Alloc(msgR2EncryptedLenWithTag), err = CHIP_ERROR_NO_MEMORY);
24342438

24352439
// TODO, should I keep this as GetBytes? or should I use GetByteView for consistency and do something else?
24362440
ReturnErrorOnFailure(
24372441
err = tlvReader.GetBytes(outParsedSigma2.msgR2Encrypted.Get(), outParsedSigma2.msgR2Encrypted.AllocatedSize()));
2438-
// msg_r2_encrypted_len = msg_r2_encrypted_len_with_tag - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES;
2442+
// msgR2EncryptedLen = msgR2EncryptedLenWithTag - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES;
24392443

24402444
// Retrieve responderMRPParams if present
24412445
if (tlvReader.Next() != CHIP_END_OF_TLV)

0 commit comments

Comments
 (0)