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

uTP: Handle cases of unexpected packets #714

Merged
merged 6 commits into from
Jan 17, 2025
Merged

uTP: Handle cases of unexpected packets #714

merged 6 commits into from
Jan 17, 2025

Conversation

ScottyPoi
Copy link
Collaborator

@ScottyPoi ScottyPoi commented Jan 17, 2025

This PR implements solutions for cases where Ultralight receives a uTP packet for a closed or non-existent connection.

Previously Ultralight would ignore these packets on the uTP level. The packet arrives via talkReq, UL responds with talkResp, and sends the packet to the uTP packet queue to process. During handleCurrentPacket, uTP would find no open request for the packet, and simply throw it away and move on to the next packet in the queue.

There are generally 3 scenarios where this may occur:

  1. We completed a uTP stream, sending a FIN-ACK packet to close the stream, but our peer failed to receive this packet. Since the peer still believes the connection to be open, they resend the FIN packet one or more times, attempting to complete the transfer.
  2. We aborted a uTP stream, sending a RESET packet to close the stream, but our peer failed to receive the RESET. Unaware that we have closed our end, they continue to send packets, awaiting a reply that will never come.
  3. A malicious peer sends bogus uTP packets as a DoS attack.

In the first 2 scenarios, we are dealing with a well-meaning peer who is behaving as expected, unaware that packet-loss has corrupted the connection. The proper response in these cases should be to send this peer a RESET packet, so that they know to close the connection on their side.

In the attack scenario, we waste resources by processing the malicious packets and responding with our own RESET packets. For each malicious packet, we would be responding with 2 of our own packets (talkResp, and RESET). With no safeguards, a bad actor could send infinite malicious packets as a DoS attack.

PR: #712 enabled Ultralight to BLACKLIST peers on the UDP level. UDP packets from a blacklisted peer will never be processed by the client.

This PR modifies uTP to respond with BLACKLIST when an unexpected packet can't be traced to a closed connection, and to respond with RESET when a packet arrives for a previously closed stream. An exception to this is when receiving a RESET packet for an already closed connection. In that case we do not send a RESET, as this could result in an infinite RESET loop between peers.

This should improve network health by closing connections that linger open, and improve client health by safeguarding against this DoS attack vector.

@ScottyPoi ScottyPoi marked this pull request as draft January 17, 2025 17:36
@ScottyPoi ScottyPoi marked this pull request as ready for review January 17, 2025 18:34
Copy link
Collaborator

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as far as it goes. Not sure I understand the blacklist at the onTalkResp level though so want to understand that before we merge.

@acolytec3 acolytec3 merged commit 3186f87 into master Jan 17, 2025
5 checks passed
@holgerd77 holgerd77 deleted the uTP-spam branch February 16, 2025 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants