From ea053d86b09b3a3d846a3e9336beb3140f2ad641 Mon Sep 17 00:00:00 2001 From: Alami-Amine Date: Sun, 29 Dec 2024 21:04:39 +0100 Subject: [PATCH] various cleanup --- src/protocols/secure_channel/CASESession.cpp | 46 +++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 561ff476d05f89..3eca90983d19ac 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -1265,7 +1265,7 @@ CHIP_ERROR CASESession::PrepareSigma2(EncodeSigma2Inputs & outSigma2Data) size_t msgR2SignedLen = EstimateStructOverhead(kMaxCHIPCertLength, // responderNoc kMaxCHIPCertLength, // responderICAC kP256_PublicKey_Length, // responderEphPubKey - kP256_PublicKey_Length // InitiatorEphPubKey + kP256_PublicKey_Length // initiatorEphPubKey ); P256ECDSASignature tbsData2Signature; @@ -1494,7 +1494,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) chip::Platform::ScopedMemoryBuffer msg_R2_Encrypted; - chip::Platform::ScopedMemoryBuffer msg_R2_Signed; + chip::Platform::ScopedMemoryBuffer msgR2Signed; AutoReleaseSessionKey sr2k(*mSessionManager->GetSessionKeystore()); @@ -1510,9 +1510,9 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) // uint16_t responderSessionId; // TLVType containerType = kTLVType_Structure; - size_t msg_r2_signed_len; + size_t msgR2SignedLen; - size_t msg_r2_encrypted_len = 0; + size_t msgR2EncryptedLen = 0; ChipLogProgress(SecureChannel, "Received Sigma2 msg"); CHIP_ERROR err = CHIP_NO_ERROR; @@ -1534,7 +1534,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) VerifyOrExit(mEphemeralKey != nullptr, err = CHIP_ERROR_INTERNAL); VerifyOrExit(buf != nullptr, err = CHIP_ERROR_MESSAGE_INCOMPLETE); - ParseSigma2(tlvReader, parsedSigma2); + SuccessOrExit(err = ParseSigma2(tlvReader, parsedSigma2)); ChipLogDetail(SecureChannel, "Peer assigned session key ID %d", parsedSigma2.responderSessionId); SetPeerSessionId(parsedSigma2.responderSessionId); @@ -1553,8 +1553,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) err = ConstructSaltSigma2(ByteSpan(parsedSigma2.responderRandom), mRemotePubKey, ByteSpan(mIPK), saltSpan)); ReturnErrorOnFailure(err = DeriveSigmaKey(saltSpan, ByteSpan(kKDFSR2Info), sr2k)); } - // TODO verify location of this - // TODO why does this trigger a fialure if i move it just after call to PArseSigma2 + // Msg2 should only be added to MessageDigest after we construct SaltSigma2 used to derive S2K ReturnErrorOnFailure(err = mCommissioningHash.AddData(ByteSpan{ buf, buflen })); if (parsedSigma2.responderMrpParamsPresent) @@ -1566,19 +1565,19 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) /********************************************************************************************************************************** */ - msg_r2_encrypted_len = parsedSigma2.msgR2Encrypted.AllocatedSize() - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES; + msgR2EncryptedLen = parsedSigma2.msgR2Encrypted.AllocatedSize() - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES; - SuccessOrExit(err = AES_CCM_decrypt(parsedSigma2.msgR2Encrypted.Get(), msg_r2_encrypted_len, nullptr, 0, - parsedSigma2.msgR2Encrypted.Get() + msg_r2_encrypted_len, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, + SuccessOrExit(err = AES_CCM_decrypt(parsedSigma2.msgR2Encrypted.Get(), msgR2EncryptedLen, nullptr, 0, + parsedSigma2.msgR2Encrypted.Get() + msgR2EncryptedLen, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, sr2k.KeyHandle(), kTBEData2_Nonce, kTBEDataNonceLength, parsedSigma2.msgR2Encrypted.Get())); - decryptedDataTlvReader.Init(parsedSigma2.msgR2Encrypted.Get(), msg_r2_encrypted_len); + decryptedDataTlvReader.Init(parsedSigma2.msgR2Encrypted.Get(), msgR2EncryptedLen); - ParseSigma2TBEData(decryptedDataTlvReader, parsedSigma2TBEData); + SuccessOrExit(err = ParseSigma2TBEData(decryptedDataTlvReader, parsedSigma2TBEData)); std::copy(parsedSigma2TBEData.resumptionId.begin(), parsedSigma2TBEData.resumptionId.end(), mNewResumptionId.begin()); - // Validate responder identity located in msg_r2_encrypted + // Validate responder identity located in msgR2Encrypted // Constructing responder identity { CompressedFabricId unused; @@ -1593,20 +1592,20 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) VerifyOrExit(mPeerNodeId == responderNodeId, err = CHIP_ERROR_INVALID_CASE_PARAMETER); } - // Construct msg_R2_Signed and validate the signature in msg_r2_encrypted - msg_r2_signed_len = + // Construct msgR2Signed and validate the signature in msgR2Encrypted. + msgR2SignedLen = EstimateStructOverhead(sizeof(uint16_t), parsedSigma2TBEData.responderNOC.size(), parsedSigma2TBEData.responderICAC.size(), kP256_PublicKey_Length, kP256_PublicKey_Length); - VerifyOrExit(msg_R2_Signed.Alloc(msg_r2_signed_len), err = CHIP_ERROR_NO_MEMORY); + VerifyOrExit(msgR2Signed.Alloc(msgR2SignedLen), err = CHIP_ERROR_NO_MEMORY); SuccessOrExit(err = ConstructTBSData(parsedSigma2TBEData.responderNOC, parsedSigma2TBEData.responderICAC, ByteSpan(mRemotePubKey, mRemotePubKey.Length()), - ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), msg_R2_Signed.Get(), - msg_r2_signed_len)); + ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), msgR2Signed.Get(), + msgR2SignedLen)); // Validate signature - SuccessOrExit(err = responderPublicKey.ECDSA_validate_msg_signature(msg_R2_Signed.Get(), msg_r2_signed_len, + SuccessOrExit(err = responderPublicKey.ECDSA_validate_msg_signature(msgR2Signed.Get(), msgR2SignedLen, parsedSigma2TBEData.tbsData2Signature)); // Retrieve peer CASE Authenticated Tags (CATs) from peer's NOC. @@ -1631,12 +1630,15 @@ CHIP_ERROR CASESession::ParseSigma2TBEData(ContiguousBufferTLVReader & decrypted ReturnErrorOnFailure(err = decryptedDataTlvReader.Next(kTLVType_ByteString, AsTlvContextTag(TBEDataTags::kSenderNOC))); ReturnErrorOnFailure(err = decryptedDataTlvReader.GetByteView(outParsedSigma2TBE.responderNOC)); + VerifyOrReturnError(outParsedSigma2TBE.responderNOC.size() <= kMaxCHIPCertLength, CHIP_ERROR_INVALID_CASE_PARAMETER); ReturnErrorOnFailure(err = decryptedDataTlvReader.Next()); if (decryptedDataTlvReader.GetTag() == AsTlvContextTag(TBEDataTags::kSenderICAC)) { VerifyOrReturnError(decryptedDataTlvReader.GetType() == kTLVType_ByteString, err = CHIP_ERROR_WRONG_TLV_TYPE); ReturnErrorOnFailure(err = decryptedDataTlvReader.GetByteView(outParsedSigma2TBE.responderICAC)); + VerifyOrReturnError(outParsedSigma2TBE.responderICAC.size() <= kMaxCHIPCertLength, CHIP_ERROR_INVALID_CASE_PARAMETER); + ReturnErrorOnFailure(err = decryptedDataTlvReader.Next(kTLVType_ByteString, AsTlvContextTag(TBEDataTags::kSignature))); } @@ -1651,6 +1653,8 @@ CHIP_ERROR CASESession::ParseSigma2TBEData(ContiguousBufferTLVReader & decrypted // Retrieve session resumption ID ReturnErrorOnFailure(err = decryptedDataTlvReader.Next(kTLVType_ByteString, AsTlvContextTag(TBEDataTags::kResumptionID))); ReturnErrorOnFailure(err = decryptedDataTlvReader.GetByteView(outParsedSigma2TBE.resumptionId)); + VerifyOrReturnError(outParsedSigma2TBE.resumptionId.size() == SessionResumptionStorage::kResumptionIdSize, + CHIP_ERROR_INVALID_CASE_PARAMETER); return CHIP_NO_ERROR; } @@ -2412,7 +2416,7 @@ CHIP_ERROR CASESession::ParseSigma2(ContiguousBufferTLVReader & tlvReader, Parse ReturnErrorOnFailure(err = tlvReader.Next(kTLVType_ByteString, AsTlvContextTag(Sigma2Tags::kEncrypted2))); // TODO find a solution to this - // size_t msg_r2_encrypted_len = 0; + // size_t msgR2EncryptedLen = 0; size_t msg_r2_encrypted_len_with_tag = 0; @@ -2435,7 +2439,7 @@ CHIP_ERROR CASESession::ParseSigma2(ContiguousBufferTLVReader & tlvReader, Parse // TODO, should I keep this as GetBytes? or should I use GetByteView for consistency and do something else? ReturnErrorOnFailure( err = tlvReader.GetBytes(outParsedSigma2.msgR2Encrypted.Get(), outParsedSigma2.msgR2Encrypted.AllocatedSize())); - // msg_r2_encrypted_len = msg_r2_encrypted_len_with_tag - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES; + // msgR2EncryptedLen = msg_r2_encrypted_len_with_tag - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES; // Retrieve responderMRPParams if present if (tlvReader.Next() != CHIP_END_OF_TLV)