feat: Add PROXY protocol support for RTMP and RTSP servers#5754
feat: Add PROXY protocol support for RTMP and RTSP servers#5754alexmck wants to merge 3 commits into
Conversation
Support PROXY protocol v1/v2 on RTMP, RTMPS, RTSP, and RTSPS TCP listeners so real client IPs are visible when running behind L4 proxies (nginx stream, HAProxy, AWS NLB). New per-protocol config fields rtmpTrustedProxies and rtspTrustedProxies follow the existing trustedProxies naming convention. Empty by default. When set, connections from trusted IPs have their PROXY header parsed if present, while other connections pass through unchanged. This is a non-breaking change.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5754 +/- ##
==========================================
- Coverage 63.12% 63.00% -0.13%
==========================================
Files 217 218 +1
Lines 18328 18411 +83
==========================================
+ Hits 11569 11599 +30
- Misses 5818 5866 +48
- Partials 941 946 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aler9
left a comment
There was a problem hiding this comment.
thanks for providing this long-awaited feature, i left some comments. In particular, we must find a way to make dumpPackets and trustedProxies coexist.
- Move proxyprotocol package to internal/protocols/proxyprotocol - Add package comment and fix prealloc lint issue - Deduplicate v1/v2 tests into table-driven test - Add rtspTrustedProxies and rtmpTrustedProxies to openapi.yaml - Fix DumpPackets and TrustedProxies to coexist for RTSP and RTMP
Reorder listener wrapping so proxy protocol is applied before the packet dumper. This ensures the packetdumper sees connections with the real client IP and the TLS secret logging works correctly. Add InnerListen field to packetdumper.Listen to allow injecting a custom listen function underneath the packet dumper.
|
Thank you for your review and feedback on this PR. I now have addressed all feedback items and lint issues. I have made one tradeoff so the PROXY header bytes themselves don't appear in the packet dump, since they're stripped before the packetdumper sees the connection. Adding these headers to the dump would require a change to the packetdumper. I feel this could be achieved in a follow up PR if requested, but I gather the main reason for this feature is inspecting the media stream which this change will still allow. Please let me know if you have any other feedback or concerns. |
Summary
Adds support for PROXY protocol v1/v2 for RTMP, RTMPS, RTSP, and RTSPS ingest, allowing the real client IP to be preserved when connections pass through a load balancer or reverse proxy. This feature was first mentioned in #602.
Two new configuration options control this:
rtspTrustedProxieslist of IPs/CIDRs trusted to send PROXY headers on RTSP connectionsrtmpTrustedProxiessame for RTMP connectionsThis follows the same format that
webrtcTrustedProxiesuses.Connections from trusted proxies have their PROXY protocol header parsed, and the original client IP is used for logging and authentication. Connections from non-trusted IPs ignore any PROXY header and are passed through unchanged.
For RTMPS, the PROXY header is read before TLS negotiation to ensure correct ordering.
Also, when
rtspTrustedProxiesis configured with RTSPS encryption, thedumpPacketsfeature will not capture TLS-level traffic for RTSP. I assume the undocumenteddumpPacketsis for debug only, but wanted to mention it just in case this is intentionally here. I can make a revision here if requested.This is a non-breaking change. I have also tested this PR in production and I can confirm that it works as expected.
New Dependency
This PR uses the go-proxyproto library. I wanted to make sure it's very clear that this PR introduces a new dependency to the library.
Please let me know if you have any questions.