fix(security): reject per-IP TLS pending overflow instead of evicting peers - #270
Conversation
… peers When max_pending_tls_per_addr is reached, drop the incoming stalled handshake rather than removing the oldest pending entry from the same source IP. Eviction allowed a co-located attacker on shared NAT/egress addresses to repeatedly deny RTMPS completion for legitimate peers. Fixes #269 Co-authored-by: Alexander Wagner <info@alexanderwagnerdev.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b40b4f6f-0c60-4810-83af-e3f0485ce09b) |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |



Security fix
Fixes #269
Finding
Severity: Medium
Location:
src/server/mod.rsWhen
max_pending_tls_per_addrwas reached,queue_pending_tlsevicted the first pending TLS handshake from the same source IP to admit a new stalled connection. An attacker sharing a NAT/carrier egress IP could repeatedly open stalled RTMPS connects and drop legitimate peers' in-flight handshakes before they complete.Impact
Cross-connection DoS: RTMPS clients behind the same public IP could not complete TLS while an attacker saturated the per-IP pending cap.
Fix
Reject the incoming stalled handshake when the per-IP cap is full, mirroring the existing global-cap behavior that already rejects instead of evicting unrelated peers.
Test
Added
pending_tls_per_addr_cap_rejects_new_handshake_instead_of_evicting_oldestregression test.Note
Medium Risk
Changes TLS admission under load in a security-sensitive path; behavior is intentionally stricter (reject vs evict) with a targeted regression test.
Overview
Security fix for per-IP pending TLS handshake admission when
max_pending_tls_per_addris saturated.queue_pending_tlsno longer removes the oldest pending handshake from the same source IP to admit a new stalled RTMPS connection. It now drops the incoming handshake and returns, matching how the global pending-TLS cap already behaves. That closes a NAT/shared-egress DoS where one co-located attacker could repeatedly open stalled connects and evict legitimate peers’ in-flight handshakes.Adds regression test
pending_tls_per_addr_cap_rejects_new_handshake_instead_of_evicting_oldestto assert the queue length and oldest entry stay unchanged when a sixth connect arrives at the default per-IP cap.Reviewed by Cursor Bugbot for commit 1df3250. Bugbot is set up for automated code reviews on this repo. Configure here.