Add header validity check for group and MCSP msg (#11441)

project-chip · Dec 11, 2023 · 1433324 · 1433324
1 parent 62d6484
commit 1433324
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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<uint16_t>::Value((uint16_t) 45));
  NL_TEST_ASSERT(inSuite, header.GetSourceNodeId() == Optional<uint64_t>::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<uint64_t>::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)