Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close TCP connection if received message length exceeds max permissible size at local node. #33307

Closed
pidarped opened this issue May 5, 2024 · 0 comments · Fixed by #33768
Closed
Assignees

Comments

@pidarped
Copy link
Contributor

pidarped commented May 5, 2024

Feature description

If a peer has sent a message that exceeds the max length that is supported, then close the connection.
The buffers in the receive queue would be, potentially, chained together and the receive path would have to parse the length field at the beginning of the head to determine whether to process the remaining data or not.

Platform

core

Platform Version(s)

No response

Anything else?

No response

@pidarped pidarped self-assigned this May 5, 2024
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 5, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 15, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 15, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 15, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 17, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 22, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 23, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 25, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 26, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 26, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 28, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.
- Disable TCP on nrfconnect platform because of resource requirements.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue May 31, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.
- Disable TCP on nrfconnect platform because of resource requirements.
  Stack allocations for large buffer with default size is exceeding
  limits. Disabling the Test file altogether for this platform would
  prevent all tests from running. Instead, only disabling TCP on
  nrfConnect for now, since it is unused.

Fixes Issues project-chip#31779, project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue Jun 2, 2024
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Close the connection to peer if a message is received with length
exceeding the amount supported by the local node.

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.
- Disable TCP on nrfconnect platform because of resource requirements.
  Stack allocations for large buffer with default size is exceeding
  limits. Disabling the Test file altogether for this platform would
  prevent all tests from running. Instead, only disabling TCP on
  nrfConnect for now, since it is unused.

Fixes Issues project-chip#31779, project-chip#33307.
@pidarped pidarped changed the title Insert checks against max permissible length during receiving over TCP Close TCP connection if received message length exceeds max permissible size at local node. Jun 5, 2024
pidarped added a commit to pidarped/connectedhomeip that referenced this issue Jun 5, 2024
When the framing length value of a received message is larger
than what the local node can process, abort the connection
with the peer.
The peer was already aware of the max length this node was
willing to receive during its TCP advertisement.

Fixes project-chip#33307.
pidarped added a commit to pidarped/connectedhomeip that referenced this issue Jun 7, 2024
When the framing length value of a received message is larger
than what the local node can process, abort the connection
with the peer.

Sending a StatusResponse message back to the peer as a
notification may not be feasible in all circumstances for
reasons, such as:
1) It would require a cross-layered feedback up to the Exchange
   layer to generate such a message in response to a failure at the
   transport layer.
2) A Status Response is sent in response to a message on an
   ExchangeContext and that may not be the case in scnearios
   where this message is the first unsolicited message.

The receiver could drain out the bits from the offending message
and move on to the next message in the stream but that may not
guarantee correct behavior and would consume resources unnecessarily.

Given that the peer was already aware of the max length this node was
willing to receive during its TCP advertisement, it seems prudent
to fail fast and close the connection.

Fixes project-chip#33307.
@mergify mergify bot closed this as completed in #33768 Jun 7, 2024
mergify bot pushed a commit that referenced this issue Jun 7, 2024
When the framing length value of a received message is larger
than what the local node can process, abort the connection
with the peer.

Sending a StatusResponse message back to the peer as a
notification may not be feasible in all circumstances for
reasons, such as:
1) It would require a cross-layered feedback up to the Exchange
   layer to generate such a message in response to a failure at the
   transport layer.
2) A Status Response is sent in response to a message on an
   ExchangeContext and that may not be the case in scnearios
   where this message is the first unsolicited message.

The receiver could drain out the bits from the offending message
and move on to the next message in the stream but that may not
guarantee correct behavior and would consume resources unnecessarily.

Given that the peer was already aware of the max length this node was
willing to receive during its TCP advertisement, it seems prudent
to fail fast and close the connection.

Fixes #33307.
@github-project-automation github-project-automation bot moved this from Todo to Done in [Device Type] Cameras Jun 7, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in [Feature] TCP Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant