From 9144e5d839cc67b4f5037d5ab2e482591e3cce78 Mon Sep 17 00:00:00 2001 From: jepenven-silabs Date: Thu, 4 Nov 2021 13:16:19 -0400 Subject: [PATCH] Add header validity check for group and MCSP msg --- src/transport/SessionManager.cpp | 37 ++++++++++--------- src/transport/raw/MessageHeader.h | 14 +++++++ src/transport/raw/tests/TestMessageHeader.cpp | 6 +++ 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index f9a810a6d0d981..3b0bb58bc00bde 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -463,31 +463,32 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade VerifyOrExit(!msg.IsNull(), ChipLogError(Inet, "Secure transport received NULL packet, discarding")); - // MCSP check - if (packetHeader.IsSecureSessionControlMsg()) + // Check if Message Header is valid first + if (!(packetHeader.IsValidMCSPMsg() || packetHeader.IsValidGroupMsg())) { - if (packetHeader.GetDestinationNodeId().HasValue() && packetHeader.HasPrivacyFlag()) - { - // TODO - // if (packetHeader.GetDestinationNodeId().Value() == ThisDeviceNodeID) - // { - // MCSP processing.. - // } - } - else - { - ChipLogError(Inet, "Invalid condition found in packet header"); - ExitNow(err = CHIP_ERROR_INCORRECT_STATE); - } + ChipLogError(Inet, "Invalid condition found in packet header"); + ExitNow(err = CHIP_ERROR_INCORRECT_STATE); } - // TODO: Handle Group message counter here spec 4.7.3 - // spec 4.5.1.2 for msg counter - // Trial decryption with GroupDataProvider. TODO: Implement the GroupDataProvider Class // VerifyOrExit(CHIP_NO_ERROR == groups->DecryptMessage(packetHeader, payloadHeader, msg), // ChipLogError(Inet, "Secure transport received group message, but failed to decode it, discarding")); + // MCSP check + if (packetHeader.IsValidMCSPMsg()) + { + // TODO + // if (packetHeader.GetDestinationNodeId().Value() == ThisDeviceNodeID) + // { + // MCSP processing.. + // } + + ExitNow(err = CHIP_NO_ERROR); + } + + // TODO: Handle Group message counter here spec 4.7.3 + // spec 4.5.1.2 for msg counter + if (isDuplicate == SessionManagerDelegate::DuplicateMessage::Yes && !payloadHeader.NeedsAck()) { ChipLogDetail(Inet, diff --git a/src/transport/raw/MessageHeader.h b/src/transport/raw/MessageHeader.h index 230a8180e0c063..14e49272db62b3 100644 --- a/src/transport/raw/MessageHeader.h +++ b/src/transport/raw/MessageHeader.h @@ -196,6 +196,20 @@ class PacketHeader } } + bool IsValidGroupMsg() const + { + // Check is based on spec 4.11.2 + return (IsGroupSession() && GetSourceNodeId().HasValue() && GetDestinationGroupId().HasValue() && + !IsSecureSessionControlMsg() && HasPrivacyFlag()); + } + + bool IsValidMCSPMsg() const + { + // Check is based on spec 4.9.2.4 + return (IsGroupSession() && GetSourceNodeId().HasValue() && GetDestinationNodeId().HasValue() && + IsSecureSessionControlMsg() && HasPrivacyFlag()); + } + bool IsEncrypted() const { return !((mSessionId == kMsgUnicastSessionIdUnsecured) && IsUnicastSession()); } uint16_t MICTagLength() const { return (IsEncrypted()) ? chip::Crypto::CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES : 0; } diff --git a/src/transport/raw/tests/TestMessageHeader.cpp b/src/transport/raw/tests/TestMessageHeader.cpp index 8f938397db2ac0..9dab02c5346ccf 100644 --- a/src/transport/raw/tests/TestMessageHeader.cpp +++ b/src/transport/raw/tests/TestMessageHeader.cpp @@ -149,6 +149,8 @@ void TestPacketHeaderEncodeDecode(nlTestSuite * inSuite, void * inContext) header.ClearDestinationNodeId(); header.SetSessionType(Header::SessionType::kGroupSession); + header.SetFlags(Header::SecFlagValues::kPrivacyFlag); + header.SetSecureSessionControlMsg(false); NL_TEST_ASSERT(inSuite, header.Encode(buffer, &encodeLen) == CHIP_NO_ERROR); // change it to verify decoding @@ -157,9 +159,12 @@ void TestPacketHeaderEncodeDecode(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, header.GetMessageCounter() == 234); NL_TEST_ASSERT(inSuite, header.GetDestinationGroupId() == Optional::Value((uint16_t) 45)); NL_TEST_ASSERT(inSuite, header.GetSourceNodeId() == Optional::Value(77ull)); + NL_TEST_ASSERT(inSuite, !header.IsSecureSessionControlMsg()); + NL_TEST_ASSERT(inSuite, header.IsValidGroupMsg()); // Verify MCSP state header.ClearDestinationGroupId().SetDestinationNodeId(42).SetFlags(Header::SecFlagValues::kPrivacyFlag); + header.SetSecureSessionControlMsg(true); NL_TEST_ASSERT(inSuite, header.Encode(buffer, &encodeLen) == CHIP_NO_ERROR); // change it to verify decoding @@ -168,6 +173,7 @@ void TestPacketHeaderEncodeDecode(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, header.GetDestinationNodeId() == Optional::Value(42ull)); NL_TEST_ASSERT(inSuite, !header.GetDestinationGroupId().HasValue()); NL_TEST_ASSERT(inSuite, header.HasPrivacyFlag()); + NL_TEST_ASSERT(inSuite, header.IsValidMCSPMsg()); } void TestPayloadHeaderEncodeDecode(nlTestSuite * inSuite, void * inContext)