Skip to content

Commit

Permalink
[msg] Fix #13669 - Skip over MX and SX extension blocks. (#16324)
Browse files Browse the repository at this point in the history 
* [msg] Fix #13669 - Skip over MX and SX extension blocks.

* [msg] Add MX flag parsing unit test.

* [msg] Add SX flag parsing unit tests.

* [restyle]

* Remove redundant check.

* [style] Use constexpr instead of #define.
  • Loading branch information
@turon @pull
turon authored and pull[bot] committed Apr 6, 2022
1 parent 1e6ce71 commit 1400953
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 6 deletions.
22 changes: 20 additions & 2 deletions src/transport/raw/MessageHeader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,17 @@ CHIP_ERROR PacketHeader::Decode(const uint8_t * const data, uint16_t size, uint1
mDestinationGroupId.ClearValue();
}

if (mSecFlags.Has(Header::SecFlagValues::kMsgExtensionFlag))
{
// If present, skip over Message Extension block.
// Spec 4.4.1.8. Message Extensions (variable)
uint16_t mxLength;
SuccessOrExit(err = reader.Read16(&mxLength).StatusCode());
VerifyOrExit(mxLength <= reader.Remaining(), err = CHIP_ERROR_INTERNAL);
reader.Skip(mxLength);
}

octets_read = static_cast<uint16_t>(reader.OctetsRead());
VerifyOrExit(octets_read == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL);
*decode_len = octets_read;

exit:
Expand Down Expand Up @@ -258,8 +267,17 @@ CHIP_ERROR PayloadHeader::Decode(const uint8_t * const data, uint16_t size, uint
mAckMessageCounter.ClearValue();
}

if (mExchangeFlags.Has(Header::ExFlagValues::kExchangeFlag_SecuredExtension))
{
// If present, skip over Secured Extension block.
// Spec 4.4.3.7. Secured Extensions (variable)
uint16_t sxLength;
SuccessOrExit(err = reader.Read16(&sxLength).StatusCode());
VerifyOrExit(sxLength <= reader.Remaining(), err = CHIP_ERROR_INTERNAL);
reader.Skip(sxLength);
}

octets_read = static_cast<uint16_t>(reader.OctetsRead());
VerifyOrExit(octets_read == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL);
*decode_len = octets_read;

exit:
Expand Down
3 changes: 3 additions & 0 deletions src/transport/raw/MessageHeader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ enum class ExFlagValues : uint8_t
/// Set when current message is requesting an acknowledgment from the recipient.
kExchangeFlag_NeedsAck = 0x04,

/// Secured Extension block is present.
kExchangeFlag_SecuredExtension = 0x08,

/// Set when a vendor id is prepended to the Message Protocol Id field.
kExchangeFlag_VendorIdPresent = 0x10,
};
Expand Down
141 changes: 137 additions & 4 deletions src/transport/raw/tests/TestMessageHeader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
* the Message Header class within the transport layer
*
*/

#include <lib/support/CHIPMem.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/ErrorStr.h>
#include <lib/support/UnitTestRegistration.h>
Expand Down Expand Up @@ -304,9 +306,21 @@ void TestPayloadHeaderEncodeDecodeBounds(nlTestSuite * inSuite, void * inContext
}
}

constexpr size_t HDR_LEN = 8; ///< Message header length
constexpr size_t SRC_LEN = 8; ///< Source Node ID length
constexpr size_t DST_LEN = 8; ///< Destination Node ID length
constexpr size_t GID_LEN = 2; ///< Group ID length
constexpr size_t MX_LEN = 6; ///< Message Exchange block length
constexpr size_t SX_LEN = 6; ///< Security Exchange block length
constexpr size_t PRO_LEN = 6; ///< Protocol header length
constexpr size_t APP_LEN = 2; ///< App payload length

/// Size of fixed portion of message header + max source node id + max destination node id.
constexpr size_t MAX_FIXED_HEADER_SIZE = (HDR_LEN + SRC_LEN + DST_LEN);

struct SpecComplianceTestVector
{
uint8_t encoded[8 + 8 + 8]; // Fixed header + max source id + max dest id
uint8_t encoded[MAX_FIXED_HEADER_SIZE]; // Fixed header + max source id + max dest id
uint8_t messageFlags;
uint16_t sessionId;
uint8_t sessionType;
Expand Down Expand Up @@ -363,12 +377,10 @@ struct SpecComplianceTestVector theSpecComplianceTestVector[] = {

const unsigned theSpecComplianceTestVectorLength = sizeof(theSpecComplianceTestVector) / sizeof(struct SpecComplianceTestVector);

#define MAX_HEADER_SIZE (8 + 8 + 8)

void TestSpecComplianceEncode(nlTestSuite * inSuite, void * inContext)
{
struct SpecComplianceTestVector * testEntry;
uint8_t buffer[MAX_HEADER_SIZE];
uint8_t buffer[MAX_FIXED_HEADER_SIZE];
uint16_t encodeSize;

for (unsigned i = 0; i < theSpecComplianceTestVectorLength; i++)
Expand Down Expand Up @@ -412,6 +424,126 @@ void TestSpecComplianceDecode(nlTestSuite * inSuite, void * inContext)
}
}

struct TestVectorMsgExtensions
{
uint8_t payloadOffset;
uint8_t appPayloadOffset;
uint16_t msgLength;
const char * msg;
};

struct TestVectorMsgExtensions theTestVectorMsgExtensions[] = {
{
// SRC=none, DST=none, MX=0, SX=0
.payloadOffset = HDR_LEN,
.appPayloadOffset = PRO_LEN,
.msgLength = HDR_LEN + PRO_LEN + APP_LEN,
.msg = "\x00\x00\x00\x00\xCC\xCC\xCC\xCC"
"\x01\xCC\xEE\xEE\x66\x66\xBB\xBB",
},
// ================== Test MX ==================
{
// SRC=none, DST=none, MX=1, SX=0
.payloadOffset = HDR_LEN + MX_LEN,
.appPayloadOffset = PRO_LEN,
.msgLength = HDR_LEN + MX_LEN + PRO_LEN + APP_LEN,
.msg = "\x00\x00\x00\x20\xCC\xCC\xCC\xCC\x04\x00\xE4\xE3\xE2\xE1"
"\x01\xCC\xEE\xEE\x66\x66\xBB\xBB",
},
{
// SRC=1, DST=none, MX=1, SX=0
.payloadOffset = HDR_LEN + MX_LEN + SRC_LEN,
.appPayloadOffset = PRO_LEN,
.msgLength = HDR_LEN + MX_LEN + SRC_LEN + PRO_LEN + APP_LEN,
.msg = "\x04\x00\x00\x20\xCC\xCC\xCC\xCC\x11\x11\x11\x11\x11\x11\x11\x11\x04\x00\xE4\xE3\xE2\xE1"
"\x01\xCC\xEE\xEE\x66\x66\xBB\xBB",
},
{
// SRC=none, DST=1, MX=1, SX=0
.payloadOffset = HDR_LEN + MX_LEN + DST_LEN,
.appPayloadOffset = PRO_LEN,
.msgLength = HDR_LEN + MX_LEN + DST_LEN + PRO_LEN + APP_LEN,
.msg = "\x01\x00\x00\x20\xCC\xCC\xCC\xCC\xDD\xDD\xDD\xDD\xDD\xDD\xDD\xDD\x04\x00\xE4\xE3\xE2\xE1"
"\x01\xCC\xEE\xEE\x66\x66\xBB\xBB",
},
{
// SRC=1, DST=1, MX=1, SX=0
.payloadOffset = HDR_LEN + MX_LEN + SRC_LEN + DST_LEN,
.appPayloadOffset = PRO_LEN,
.msgLength = HDR_LEN + MX_LEN + SRC_LEN + DST_LEN + PRO_LEN + APP_LEN,
.msg = "\x05\x00\x00\x20\xCC\xCC\xCC\xCC\x11\x11\x11\x11\x11\x11\x11\x11\xDD\xDD\xDD\xDD\xDD\xDD\xDD\xDD\x04\x00\xE4\xE3"
"\xE2\xE1"
"\x01\xCC\xEE\xEE\x66\x66\xBB\xBB",
},
{
// SRC=none, DST=group, MX=1, SX=0
.payloadOffset = HDR_LEN + MX_LEN + GID_LEN,
.appPayloadOffset = PRO_LEN,
.msgLength = HDR_LEN + MX_LEN + GID_LEN + PRO_LEN + APP_LEN,
.msg = "\x02\x00\x00\x21\xCC\xCC\xCC\xCC\xDD\xDD\x04\x00\xE4\xE3\xE2\xE1"
"\x01\xCC\xEE\xEE\x66\x66\xBB\xBB",
},
{
// SRC=1, DST=group, MX=1, SX=0
.payloadOffset = HDR_LEN + MX_LEN + SRC_LEN + GID_LEN,
.appPayloadOffset = PRO_LEN,
.msgLength = HDR_LEN + MX_LEN + SRC_LEN + GID_LEN + PRO_LEN + APP_LEN,
.msg = "\x06\x00\x00\x21\xCC\xCC\xCC\xCC\x11\x11\x11\x11\x11\x11\x11\x11\xDD\xDD\x04\x00\xE4\xE3\xE2\xE1"
"\x01\xCC\xEE\xEE\x66\x66\xBB\xBB",
},
// ================== Test SX ==================
{
// SRC=none, DST=none, MX=0, SX=1
.payloadOffset = HDR_LEN,
.appPayloadOffset = PRO_LEN + SX_LEN,
.msgLength = HDR_LEN + PRO_LEN + SX_LEN + APP_LEN,
.msg = "\x00\x00\x00\x00\xCC\xCC\xCC\xCC"
"\x08\xCC\xEE\xEE\x66\x66\x04\x00\xE4\xE3\xE2\xE1\xBB\xBB",
},
{
// SRC=none, DST=none, MX=1, SX=1
.payloadOffset = HDR_LEN + MX_LEN,
.appPayloadOffset = PRO_LEN + SX_LEN,
.msgLength = HDR_LEN + MX_LEN + PRO_LEN + SX_LEN + APP_LEN,
.msg = "\x00\x00\x00\x20\xCC\xCC\xCC\xCC\x04\x00\xE4\xE3\xE2\xE1"
"\x08\xCC\xEE\xEE\x66\x66\x04\x00\xE4\xE3\xE2\xE1\xBB\xBB",
},
{
// SRC=1, DST=1, MX=1, SX=1
.payloadOffset = HDR_LEN + MX_LEN + SRC_LEN + DST_LEN,
.appPayloadOffset = PRO_LEN + SX_LEN,
.msgLength = HDR_LEN + MX_LEN + SRC_LEN + DST_LEN + PRO_LEN + SX_LEN + APP_LEN,
.msg = "\x05\x00\x00\x20\xCC\xCC\xCC\xCC\x11\x11\x11\x11\x11\x11\x11\x11\xDD\xDD\xDD\xDD\xDD\xDD\xDD\xDD\x04\x00\xE4\xE3"
"\xE2\xE1"
"\x09\xCC\xEE\xEE\x66\x66\x04\x00\xE4\xE3\xE2\xE1\xBB\xBB",
},
};

const unsigned theTestVectorMsgExtensionsLength = sizeof(theTestVectorMsgExtensions) / sizeof(struct TestVectorMsgExtensions);

void TestMsgExtensionsDecode(nlTestSuite * inSuite, void * inContext)
{
struct TestVectorMsgExtensions * testEntry;
PacketHeader packetHeader;
PayloadHeader payloadHeader;
uint16_t decodeSize;

NL_TEST_ASSERT(inSuite, chip::Platform::MemoryInit() == CHIP_NO_ERROR);

for (unsigned i = 0; i < theTestVectorMsgExtensionsLength; i++)
{
testEntry = &theTestVectorMsgExtensions[i];

System::PacketBufferHandle msg = System::PacketBufferHandle::NewWithData(testEntry->msg, testEntry->msgLength);

NL_TEST_ASSERT(inSuite, packetHeader.Decode(msg->Start(), msg->DataLength(), &decodeSize) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, decodeSize == testEntry->payloadOffset);

NL_TEST_ASSERT(inSuite, payloadHeader.Decode(msg->Start() + decodeSize, msg->DataLength(), &decodeSize) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, decodeSize == testEntry->appPayloadOffset);
}
}

} // namespace

// clang-format off
Expand All @@ -425,6 +557,7 @@ static const nlTest sTests[] =
NL_TEST_DEF("PayloadEncodeDecodeBounds", TestPayloadHeaderEncodeDecodeBounds),
NL_TEST_DEF("SpecComplianceEncode", TestSpecComplianceEncode),
NL_TEST_DEF("SpecComplianceDecode", TestSpecComplianceDecode),
NL_TEST_DEF("TestMsgExtensionsDecode", TestMsgExtensionsDecode),
NL_TEST_SENTINEL()
};
// clang-format on
Expand Down

0 comments on commit 1400953

Please sign in to comment.