Skip to content

Commit

Permalink
Stop allocating CASE buffer based on wire sizes (#17296)
Browse files Browse the repository at this point in the history 
* Stop allocating CASE buffer based on wire sizes

- CASE was using direct lengths of TLV values to allocate heap values,
  without validations.
- Added validations on lengths of Sigma2/3 TBE, with some room to grow
- Updated tag order validation logic to be more explicit and easier to
  read.

Fixes #8924

Test done:
- Cert tests still pass
- Unit tests still pass

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
@tcarmelveilleux @restyled-commits @pull
2 people authored and pull[bot] committed Feb 20, 2024
1 parent 5e22fc3 commit 1784464
Showing 1 changed file with 66 additions and 25 deletions.
91 changes: 66 additions & 25 deletions src/protocols/secure_channel/CASESession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,43 @@
#include <transport/PairingSession.h>
#include <transport/SessionManager.h>

namespace {

enum
{
kTag_TBEData_SenderNOC = 1,
kTag_TBEData_SenderICAC = 2,
kTag_TBEData_Signature = 3,
kTag_TBEData_ResumptionID = 4,
};

enum
{
kTag_Sigma1_InitiatorRandom = 1,
kTag_Sigma1_InitiatorSessionId = 2,
kTag_Sigma1_DestinationId = 3,
kTag_Sigma1_InitiatorEphPubKey = 4,
kTag_Sigma1_InitiatorMRPParams = 5,
kTag_Sigma1_ResumptionID = 6,
kTag_Sigma1_InitiatorResumeMIC = 7,
};

enum
{
kTag_Sigma2_ResponderRandom = 1,
kTag_Sigma2_ResponderSessionId = 2,
kTag_Sigma2_ResponderEphPubKey = 3,
kTag_Sigma2_Encrypted2 = 4,
kTag_Sigma2_ResponderMRPParams = 5,
};

enum
{
kTag_Sigma3_Encrypted3 = 1,
};

} // namespace

namespace chip {

using namespace Crypto;
Expand Down Expand Up @@ -72,14 +109,6 @@ constexpr uint8_t kTBEData3_Nonce[] =
constexpr size_t kTBEDataNonceLength = sizeof(kTBEData2_Nonce);
static_assert(sizeof(kTBEData2_Nonce) == sizeof(kTBEData3_Nonce), "TBEData2_Nonce and TBEData3_Nonce must be same size");

enum
{
kTag_TBEData_SenderNOC = 1,
kTag_TBEData_SenderICAC = 2,
kTag_TBEData_Signature = 3,
kTag_TBEData_ResumptionID = 4,
};

#ifdef ENABLE_HSM_HKDF
using HKDF_sha_crypto = HKDF_shaHSM;
#else
Expand Down Expand Up @@ -846,6 +875,8 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)

chip::Platform::ScopedMemoryBuffer<uint8_t> msg_R2_Signed;
size_t msg_r2_signed_len;
size_t max_msg_r2_signed_enc_len;
constexpr size_t kCaseOverheadForFutureTbeData = 128;

uint8_t sr2k[CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES];

Expand All @@ -860,8 +891,6 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)

uint16_t responderSessionId;

uint32_t decodeTagIdSeq = 0;

VerifyOrExit(buf != nullptr, err = CHIP_ERROR_MESSAGE_INCOMPLETE);

ChipLogProgress(SecureChannel, "Received Sigma2 msg");
Expand All @@ -871,21 +900,18 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
SuccessOrExit(err = tlvReader.EnterContainer(containerType));

// Retrieve Responder's Random value
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Next(TLV::kTLVType_ByteString, TLV::ContextTag(kTag_Sigma2_ResponderRandom)));
SuccessOrExit(err = tlvReader.GetBytes(responderRandom, sizeof(responderRandom)));

// Assign Session ID
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Next(TLV::kTLVType_UnsignedInteger, TLV::ContextTag(kTag_Sigma2_ResponderSessionId)));
SuccessOrExit(err = tlvReader.Get(responderSessionId));

ChipLogDetail(SecureChannel, "Peer assigned session session ID %d", responderSessionId);
SetPeerSessionId(responderSessionId);

// Retrieve Responder's Ephemeral Pubkey
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Next(TLV::kTLVType_ByteString, TLV::ContextTag(kTag_Sigma2_ResponderEphPubKey)));
SuccessOrExit(err = tlvReader.GetBytes(mRemotePubKey, static_cast<uint32_t>(mRemotePubKey.Length())));

// Generate a Shared Secret
Expand All @@ -906,11 +932,18 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
SuccessOrExit(err = mCommissioningHash.AddData(ByteSpan{ buf, buflen }));

// Generate decrypted data
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
VerifyOrExit(msg_R2_Encrypted.Alloc(tlvReader.GetLength()), err = CHIP_ERROR_NO_MEMORY);
SuccessOrExit(err = tlvReader.Next(TLV::kTLVType_ByteString, TLV::ContextTag(kTag_Sigma2_Encrypted2)));

max_msg_r2_signed_enc_len =
TLV::EstimateStructOverhead(Credentials::kMaxCHIPCertLength, Credentials::kMaxCHIPCertLength, tbsData2Signature.Length(),
SessionResumptionStorage::kResumptionIdSize, kCaseOverheadForFutureTbeData);
msg_r2_encrypted_len_with_tag = tlvReader.GetLength();

// Validate we did not receive a buffer larger than legal
VerifyOrExit(msg_r2_encrypted_len_with_tag <= max_msg_r2_signed_enc_len, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
VerifyOrExit(msg_r2_encrypted_len_with_tag > CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
VerifyOrExit(msg_R2_Encrypted.Alloc(msg_r2_encrypted_len_with_tag), err = CHIP_ERROR_NO_MEMORY);

SuccessOrExit(err = tlvReader.GetBytes(msg_R2_Encrypted.Get(), static_cast<uint32_t>(msg_r2_encrypted_len_with_tag)));
msg_r2_encrypted_len = msg_r2_encrypted_len_with_tag - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES;

Expand Down Expand Up @@ -971,7 +1004,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg)
// Retrieve responderMRPParams if present
if (tlvReader.Next() != CHIP_END_OF_TLV)
{
SuccessOrExit(err = DecodeMRPParametersIfPresent(TLV::ContextTag(5), tlvReader));
SuccessOrExit(err = DecodeMRPParametersIfPresent(TLV::ContextTag(kTag_Sigma2_ResponderMRPParams), tlvReader));
}

exit:
Expand Down Expand Up @@ -1126,9 +1159,12 @@ CHIP_ERROR CASESession::HandleSigma3(System::PacketBufferHandle && msg)
const uint8_t * buf = msg->Start();
const uint16_t bufLen = msg->DataLength();

constexpr size_t kCaseOverheadForFutureTbeData = 128;

chip::Platform::ScopedMemoryBuffer<uint8_t> msg_R3_Encrypted;
size_t msg_r3_encrypted_len = 0;
size_t msg_r3_encrypted_len_with_tag = 0;
size_t max_msg_r3_signed_enc_len;
chip::Platform::ScopedMemoryBuffer<uint8_t> msg_R3_Signed;
size_t msg_r3_signed_len;

Expand All @@ -1144,20 +1180,25 @@ CHIP_ERROR CASESession::HandleSigma3(System::PacketBufferHandle && msg)

uint8_t msg_salt[kIPKSize + kSHA256_Hash_Length];

uint32_t decodeTagIdSeq = 0;

ChipLogProgress(SecureChannel, "Received Sigma3 msg");

tlvReader.Init(std::move(msg));
SuccessOrExit(err = tlvReader.Next(containerType, TLV::AnonymousTag()));
SuccessOrExit(err = tlvReader.EnterContainer(containerType));

// Fetch encrypted data
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
VerifyOrExit(msg_R3_Encrypted.Alloc(tlvReader.GetLength()), err = CHIP_ERROR_NO_MEMORY);
max_msg_r3_signed_enc_len = TLV::EstimateStructOverhead(Credentials::kMaxCHIPCertLength, Credentials::kMaxCHIPCertLength,
tbsData3Signature.Length(), kCaseOverheadForFutureTbeData);

SuccessOrExit(err = tlvReader.Next(TLV::kTLVType_ByteString, TLV::ContextTag(kTag_Sigma3_Encrypted3)));

msg_r3_encrypted_len_with_tag = tlvReader.GetLength();

// Validate we did not receive a buffer larger than legal
VerifyOrExit(msg_r3_encrypted_len_with_tag <= max_msg_r3_signed_enc_len, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
VerifyOrExit(msg_r3_encrypted_len_with_tag > CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, err = CHIP_ERROR_INVALID_TLV_ELEMENT);

VerifyOrExit(msg_R3_Encrypted.Alloc(msg_r3_encrypted_len_with_tag), err = CHIP_ERROR_NO_MEMORY);
SuccessOrExit(err = tlvReader.GetBytes(msg_R3_Encrypted.Get(), static_cast<uint32_t>(msg_r3_encrypted_len_with_tag)));
msg_r3_encrypted_len = msg_r3_encrypted_len_with_tag - CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES;

Expand Down

0 comments on commit 1784464

Please sign in to comment.