fix(publisher): cap concurrent playback connections to prevent resource-exhaustion DoS#18
Merged
Merged
Conversation
…ce-exhaustion DoS The long-lived play/push protocols (RTSP, RTMP, SRT, HTTP-MPEGTS) each allocate heavyweight per-connection state — a demux pipeline, a multi-MB write/ts buffer, 2-3 goroutines, an FD — with no cap. None of the handlers authenticate, so an unauthenticated client that knows one active stream code could open many concurrent connections and exhaust goroutines / FDs / memory, OOM-killing the shared process and taking every stream on the host down. Add a connLimiter (per-stream + global) on the publisher Service and check it BEFORE allocating any per-connection state; release on close: - HandleMPEGTS: acquire → 503, defer release. - HandleRTMPPlay: acquire → error, defer release. - srtHandleSubscribe: acquire → close+return, defer release (leak-proof). - RTSP OnPlay: acquire → 503; release in OnSessionClose via the rtspSessions entry, with a repeated-PLAY guard (PAUSE→PLAY can't double-count) and nil-ps cap-only entries handled by the touch loop / pollBytes. Caps are configurable (MaxPlaybackConnPerStream / MaxPlaybackConnTotal): unset applies a protective default (256 / 4096), negative means unlimited. HLS/DASH viewers go over stateless HTTP and are not counted here. Tests: TestConnLimiter_* (per-stream/global cap, isolation, unlimited, no-underflow, concurrent -race balance), TestResolvePlaybackCap, TestHandleMPEGTS_PlaybackCapRejectsAndReleases (503 over cap; slot frees after the connection closes). The acquire/release balance across all four handlers was reviewed for leak / double-release paths.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (audit A-1, HIGH)
The long-lived play/push protocols (RTSP, RTMP, SRT, HTTP-MPEGTS) each allocate heavyweight per-connection state — a demux pipeline, a multi-MB write/ts buffer, 2-3 goroutines, an FD — with no cap. None of the handlers authenticate (the
tokenparam is attribution-only), so an unauthenticated client that knows one active stream code can open many concurrent connections and exhaust goroutines / FDs / memory, OOM-killing the shared process and taking every stream on the host down.Fix
New
connLimiter(internal/publisher/conn_limiter.go) on the publisherServicecaps concurrent playback connections per-stream and globally, checked before any per-connection state is allocated, released on close:HandleMPEGTS503deferHandleRTMPPlayerrordefersrtHandleSubscribedefer(moved to the subscribe path so it's leak-proof)OnPlay503OnSessionClosevia thertspSessionsentry, with a repeated-PLAY guard (PAUSE→PLAY can't double-count) + nil-pscap-only entries handled by the touch loop /pollBytesCaps are configurable —
PublisherConfig.MaxPlaybackConnPerStream/MaxPlaybackConnTotal: unset → protective default (256 / 4096), negative → unlimited. HLS/DASH viewers go over stateless HTTP and are not counted here.The acquire/release balance across all four handlers was adversarially audited (including against gortsplib's serialized-per-session + guaranteed-
OnSessionCloselifecycle for the non-defer RTSP path) — no slot-leak or double-release found, andreleaseitself is underflow-safe.Test
TestConnLimiter_*(per-stream cap, global cap, per-stream isolation, unlimited, spurious-release no-underflow, concurrent-racebalance back to zero),TestResolvePlaybackCap,TestHandleMPEGTS_PlaybackCapRejectsAndReleases(2nd conn over cap → 503; slot frees after the first closes).go test -race ./internal/publisher/green,golangci-lint0 issues, fullgo build ./...green.Note: this is the resource-cap half of A-1; authentication (the other half) remains tracked as S-1.