Skip to content

Commit

Permalink
Fix various issues in CASE Sigma1 processing (#10084)
Browse files Browse the repository at this point in the history 
* Factor out the parsing of a Sigma1 message into a separate function.

This allows testing the parsing, and catching bugs in it.

* Add various sanity checks to Sigma1 processing.

* Verify that the TLV struct is actually close.
* Verify that the octet strings are the right sizes.
* Fix incorrect tag value checking.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Oct 20, 2021
1 parent 6781f3e commit 3453209
Show file tree
Hide file tree
Showing 6 changed files with 348 additions and 88 deletions.
26 changes: 26 additions & 0 deletions src/lib/core/CHIPTLV.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,32 @@ class DLL_EXPORT TLVReader
*/
CHIP_ERROR Next();

/**
* Advances the TLVReader object to the next TLV element to be read, asserting the tag of
* the new element.
*
* The Next(uint64_t expectedTag) method is a convenience method that has the
* same behavior as Next(), but also verifies that the tag of the new TLV element matches
* the supplied argument.
*
* @param[in] expectedTag The expected tag for the next element.
*
* @retval #CHIP_NO_ERROR If the reader was successfully positioned on a new element.
* @retval #CHIP_END_OF_TLV If no further elements are available.
* @retval #CHIP_ERROR_UNEXPECTED_TLV_ELEMENT
* If the tag associated with the new element does not match the
* value of the @p expectedTag argument.
* @retval #CHIP_ERROR_TLV_UNDERRUN If the underlying TLV encoding ended prematurely.
* @retval #CHIP_ERROR_INVALID_TLV_ELEMENT
* If the reader encountered an invalid or unsupported TLV
* element type.
* @retval #CHIP_ERROR_INVALID_TLV_TAG If the reader encountered a TLV tag in an invalid context.
* @retval other Other CHIP or platform error codes returned by the configured
* TLVBackingStore.
*
*/
CHIP_ERROR Next(uint64_t expectedTag);

/**
* Advances the TLVReader object to the next TLV element to be read, asserting the type and tag of
* the new element.
Expand Down
14 changes: 11 additions & 3 deletions src/lib/core/CHIPTLVReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,18 +505,26 @@ CHIP_ERROR TLVReader::Next()
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Next(TLVType expectedType, uint64_t expectedTag)
CHIP_ERROR TLVReader::Next(uint64_t expectedTag)
{
CHIP_ERROR err = Next();
if (err != CHIP_NO_ERROR)
return err;
if (GetType() != expectedType)
return CHIP_ERROR_WRONG_TLV_TYPE;
if (mElemTag != expectedTag)
return CHIP_ERROR_UNEXPECTED_TLV_ELEMENT;
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Next(TLVType expectedType, uint64_t expectedTag)
{
CHIP_ERROR err = Next(expectedTag);
if (err != CHIP_NO_ERROR)
return err;
if (GetType() != expectedType)
return CHIP_ERROR_WRONG_TLV_TYPE;
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Skip()
{
CHIP_ERROR err;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/core/CHIPTLVTags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ enum
* @param[in] tagNum The profile-specific tag number assigned to the tag.
* @return A 64-bit integer representing the tag.
*/
inline uint64_t ProfileTag(uint32_t profileId, uint32_t tagNum)
inline constexpr uint64_t ProfileTag(uint32_t profileId, uint32_t tagNum)
{
return ((static_cast<uint64_t>(profileId)) << kProfileIdShift) | tagNum;
}
Expand All @@ -111,7 +111,7 @@ inline uint64_t ProfileTag(uint16_t vendorId, uint16_t profileNum, uint32_t tagN
* @param[in] tagNum The context-specific tag number assigned to the tag.
* @return A 64-bit integer representing the tag.
*/
inline uint64_t ContextTag(uint8_t tagNum)
inline constexpr uint64_t ContextTag(uint8_t tagNum)
{
return kSpecialTagMarker | tagNum;
}
Expand Down
145 changes: 73 additions & 72 deletions src/protocols/secure_channel/CASESession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,50 +418,38 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg)
{
CHIP_ERROR err = CHIP_NO_ERROR;
System::PacketBufferTLVReader tlvReader;
TLV::TLVType containerType = TLV::kTLVType_Structure;

uint16_t initiatorSessionId;
uint8_t destinationIdentifier[kSHA256_Hash_Length];
uint8_t initiatorRandom[kSigmaParamRandomNumberSize];

uint32_t decodeTagIdSeq = 0;
ByteSpan destinationIdentifier;
ByteSpan initiatorRandom;

ChipLogDetail(SecureChannel, "Received Sigma1 msg");

bool sessionResumptionRequested = false;
ByteSpan resumptionId;
ByteSpan resume1MIC;
ByteSpan initiatorPubKey;

const ByteSpan * ipkListSpan = GetIPKList();
FabricIndex fabricIndex = Transport::kUndefinedFabricIndex;

SuccessOrExit(err = IsResumptionRequestPresent(msg, sessionResumptionRequested, resumptionId, resume1MIC));

SuccessOrExit(err = mCommissioningHash.AddData(ByteSpan{ msg->Start(), msg->DataLength() }));

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

SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.GetBytes(initiatorRandom, sizeof(initiatorRandom)));

SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Get(initiatorSessionId));
SuccessOrExit(err = ParseSigma1(tlvReader, initiatorRandom, initiatorSessionId, destinationIdentifier, initiatorPubKey,
sessionResumptionRequested, resumptionId, resume1MIC));

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

if (sessionResumptionRequested && resumptionId.data_equal(ByteSpan(mResumptionId)))
{
// Cross check resume1MIC with the shared secret
if (ValidateSigmaResumeMIC(resume1MIC, ByteSpan(initiatorRandom), resumptionId, ByteSpan(kKDFS1RKeyInfo),
if (ValidateSigmaResumeMIC(resume1MIC, initiatorRandom, resumptionId, ByteSpan(kKDFS1RKeyInfo),
ByteSpan(kResume1MIC_Nonce)) == CHIP_NO_ERROR)
{
// Send Sigma2Resume message to the initiator
SuccessOrExit(err = SendSigma2Resume(ByteSpan(initiatorRandom)));
SuccessOrExit(err = SendSigma2Resume(initiatorRandom));

mDelegate->OnSessionEstablishmentStarted();

Expand All @@ -472,21 +460,17 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg)

memcpy(mIPK, ipkListSpan->data(), sizeof(mIPK));

SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.GetBytes(destinationIdentifier, sizeof(destinationIdentifier)));

VerifyOrExit(mFabricsTable != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
fabricIndex = mFabricsTable->FindDestinationIDCandidate(ByteSpan(destinationIdentifier), ByteSpan(initiatorRandom), ipkListSpan,
GetIPKListEntries());
fabricIndex =
mFabricsTable->FindDestinationIDCandidate(destinationIdentifier, initiatorRandom, ipkListSpan, GetIPKListEntries());
VerifyOrExit(fabricIndex != Transport::kUndefinedFabricIndex, err = CHIP_ERROR_KEY_NOT_FOUND);

mFabricInfo = mFabricsTable->FindFabricWithIndex(fabricIndex);
VerifyOrExit(mFabricInfo != nullptr, err = CHIP_ERROR_INCORRECT_STATE);

SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.GetBytes(mRemotePubKey, static_cast<uint32_t>(mRemotePubKey.Length())));
// ParseSigma1 ensures that:
// mRemotePubKey.Length() == initiatorPubKey.size() == kP256_PublicKey_Length.
memcpy(mRemotePubKey.Bytes(), initiatorPubKey.data(), mRemotePubKey.Length());

SuccessOrExit(err = SendSigma2());

Expand Down Expand Up @@ -1360,58 +1344,75 @@ CHIP_ERROR CASESession::OnFailureStatusReport(Protocols::SecureChannel::GeneralS
return err;
}

CHIP_ERROR CASESession::IsResumptionRequestPresent(const System::PacketBufferHandle & msg, bool & resumptionRequested,
ByteSpan & resumptionID, ByteSpan & resume1MIC)
CHIP_ERROR CASESession::ParseSigma1(TLV::ContiguousBufferTLVReader & tlvReader, ByteSpan & initiatorRandom,
uint16_t & initiatorSessionId, ByteSpan & destinationId, ByteSpan & initiatorEphPubKey,
bool & resumptionRequested, ByteSpan & resumptionId, ByteSpan & initiatorResumeMIC)
{
CHIP_ERROR err = CHIP_NO_ERROR;

System::PacketBufferTLVReader tlvReader;
TLV::TLVType containerType = TLV::kTLVType_Structure;

constexpr uint32_t kResumptionIDTag = 6;
constexpr uint32_t kResume1MICTag = 7;

uint32_t lastDecodedTLVTag = 0;
bool resumptionIDTagFound = false;
bool resume1MICTagFound = false;

System::PacketBufferHandle msg_clone = msg.CloneData();
using namespace TLV;

constexpr uint8_t kInitiatorRandomTag = 1;
constexpr uint8_t kInitiatorSessionIdTag = 2;
constexpr uint8_t kDestinationIdTag = 3;
constexpr uint8_t kInitiatorPubKeyTag = 4;
constexpr uint8_t kInitiatorMRPParamsTag = 5;
constexpr uint8_t kResumptionIDTag = 6;
constexpr uint8_t kResume1MICTag = 7;

TLVType containerType = kTLVType_Structure;
ReturnErrorOnFailure(tlvReader.Next(containerType, AnonymousTag));
ReturnErrorOnFailure(tlvReader.EnterContainer(containerType));

ReturnErrorOnFailure(tlvReader.Next(ContextTag(kInitiatorRandomTag)));
ReturnErrorOnFailure(tlvReader.GetByteView(initiatorRandom));
VerifyOrReturnError(initiatorRandom.size() == kSigmaParamRandomNumberSize, CHIP_ERROR_INVALID_CASE_PARAMETER);

ReturnErrorOnFailure(tlvReader.Next(ContextTag(kInitiatorSessionIdTag)));
ReturnErrorOnFailure(tlvReader.Get(initiatorSessionId));

ReturnErrorOnFailure(tlvReader.Next(ContextTag(kDestinationIdTag)));
ReturnErrorOnFailure(tlvReader.GetByteView(destinationId));
VerifyOrReturnError(destinationId.size() == kSHA256_Hash_Length, CHIP_ERROR_INVALID_CASE_PARAMETER);

ReturnErrorOnFailure(tlvReader.Next(ContextTag(kInitiatorPubKeyTag)));
ReturnErrorOnFailure(tlvReader.GetByteView(initiatorEphPubKey));
VerifyOrReturnError(initiatorEphPubKey.size() == kP256_PublicKey_Length, CHIP_ERROR_INVALID_CASE_PARAMETER);

// Optional members start here.
CHIP_ERROR err = tlvReader.Next();
if (err == CHIP_NO_ERROR && tlvReader.GetTag() == ContextTag(kInitiatorMRPParamsTag))
{
// We don't handle this yet; just move on.
err = tlvReader.Next();
}

tlvReader.Init(std::move(msg_clone));
SuccessOrExit(err = tlvReader.Next(containerType, TLV::AnonymousTag));
SuccessOrExit(err = tlvReader.EnterContainer(containerType));
bool resumptionIDTagFound = false;
bool resume1MICTagFound = false;

while (!resumptionIDTagFound || !resume1MICTagFound)
if (err == CHIP_NO_ERROR && tlvReader.GetTag() == ContextTag(kResumptionIDTag))
{
SuccessOrExit(err = tlvReader.Next());

// Make sure that tlv tags are in order.
// There are optional TLV elements, so some of them may not be present.
// So the check cannot match the absolute value of the expected tag.
uint32_t tlvTag = TLV::TagNumFromTag(tlvReader.GetTag());
VerifyOrExit(tlvTag > lastDecodedTLVTag, CHIP_ERROR_INVALID_TLV_TAG);
lastDecodedTLVTag = tlvTag;
resumptionIDTagFound = true;
ReturnErrorOnFailure(tlvReader.GetByteView(resumptionId));
VerifyOrReturnError(resumptionId.size() == kCASEResumptionIDSize, CHIP_ERROR_INVALID_CASE_PARAMETER);
err = tlvReader.Next();
}

if (tlvTag == kResumptionIDTag)
{
resumptionIDTagFound = true;
SuccessOrExit(err = tlvReader.GetByteView(resumptionID));
}
else if (tlvTag == kResume1MICTag)
{
VerifyOrExit(resumptionIDTagFound, CHIP_ERROR_INVALID_TLV_TAG);
resume1MICTagFound = true;
SuccessOrExit(err = tlvReader.GetByteView(resume1MIC));
}
if (err == CHIP_NO_ERROR && tlvReader.GetTag() == ContextTag(kResume1MICTag))
{
resume1MICTagFound = true;
ReturnErrorOnFailure(tlvReader.GetByteView(initiatorResumeMIC));
VerifyOrReturnError(initiatorResumeMIC.size() == kTAGSize, CHIP_ERROR_INVALID_CASE_PARAMETER);
err = tlvReader.Next();
}

exit:
if (err != CHIP_NO_ERROR && err != CHIP_END_OF_TLV)
if (err == CHIP_END_OF_TLV)
{
return err;
// We ran out of struct members, but that's OK, because they were optional.
err = CHIP_NO_ERROR;
}

err = CHIP_NO_ERROR;
ReturnErrorOnFailure(err);
ReturnErrorOnFailure(tlvReader.ExitContainer(containerType));

if (resumptionIDTagFound && resume1MICTagFound)
{
resumptionRequested = true;
Expand All @@ -1422,10 +1423,10 @@ CHIP_ERROR CASESession::IsResumptionRequestPresent(const System::PacketBufferHan
}
else
{
err = CHIP_ERROR_UNEXPECTED_TLV_ELEMENT;
return CHIP_ERROR_UNEXPECTED_TLV_ELEMENT;
}

return err;
return CHIP_NO_ERROR;
}

CHIP_ERROR CASESession::ValidateReceivedMessage(ExchangeContext * ec, const PayloadHeader & payloadHeader,
Expand Down
35 changes: 24 additions & 11 deletions src/protocols/secure_channel/CASESession.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#if CHIP_CRYPTO_HSM
#include <crypto/hsm/CHIPCryptoPALHsm.h>
#endif
#include <lib/core/CHIPTLV.h>
#include <lib/support/Base64.h>
#include <messaging/ExchangeContext.h>
#include <messaging/ExchangeDelegate.h>
Expand Down Expand Up @@ -118,20 +119,27 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
SessionEstablishmentDelegate * delegate);

/**
* Parse the message to check if it has a session resumption request.
* A valid session resumption request must have Resumption ID, and InitiationResumeMIC.
* Parse a sigma1 message. This function will return success only if the
* message passes schema checks. Specifically:
* * The tags come in order.
* * The required tags are present.
* * The values for the tags that are present satisfy schema requirements
* (e.g. constraints on octet string lengths)
* * Either resumptionID and initiatorResume1MIC are both present or both
* absent.
*
* If the message is a valid session resumption request, the output parameter resumptionRequested is set to true,
* and corresponding resumption ID and MIC are returned in the output parameters. Return value is set to CHIP_NO_ERROR.
* On success, the initiatorRandom, initiatorSessionId, destinationId,
* initiatorEphPubKey outparams will be set to the corresponding values in
* the message.
*
* If the message is not a session resumption request (i.e. it doesn't contain both Resumption ID, and InitiationResumeMIC),
* the output parameter resumptionRequested is set to false. Return value is set to CHIP_NO_ERROR.
*
* If the message doesn't contain either Resumption ID or InitiationResumeMIC (i.e. contains only one of these fields), the
* function returns CHIP_ERROR_INVALID_ARGUMENT.
* On success, either the resumptionRequested outparam will be set to true
* and the resumptionID and initiatorResumeMIC outparams will be set to
* valid values, or the resumptionRequested outparam will be set to false.
*/
static CHIP_ERROR IsResumptionRequestPresent(const System::PacketBufferHandle & message, bool & resumptionRequested,
ByteSpan & resumptionID, ByteSpan & resume1MIC);
static CHIP_ERROR ParseSigma1(TLV::ContiguousBufferTLVReader & tlvReader, ByteSpan & initiatorRandom,
uint16_t & initiatorSessionId, ByteSpan & destinationId, ByteSpan & initiatorEphPubKey,
// TODO: MRP param parsing
bool & resumptionRequested, ByteSpan & resumptionId, ByteSpan & initiatorResumeMIC);

/**
* @brief
Expand Down Expand Up @@ -191,6 +199,11 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
**/
void Clear();

/**
* Parse the TLV for Sigma1 message.
*/
CHIP_ERROR ParseSigma1();

private:
enum State : uint8_t
{
Expand Down
Loading

0 comments on commit 3453209

Please sign in to comment.