fix: serialize the RTSP handshake per client IP (same-NAT clients) - #441
fix: serialize the RTSP handshake per client IP (same-NAT clients)#441kmreisi wants to merge 1 commit into
Conversation
|
Hey, I've reviewed and tested this, but I am having trouble with this PR. In "moonlight.cpp:19-43" and "udp-ping.cpp:12-29", it clearly is demuxing multiple connections from the same IP via the secret payload. The only way this can occur is if the moonlight client is not correctly handling the secret payload. src/moonlight-server/rtp/udp-ping.cpp:30 is where this occurs, and the only way the described behavior would occur is from line 27 on udp-ping.cpp. But if that is occuring, the moonlight client is setup incorrectly. I finished testing this myself and was able to get multiple connections via NAT on moonlight. I need more information on this. |
955719a to
3878027
Compare
|
Hi @JBailes, @ABeltramo, You're right about the UDP layer — The RTSP side is a bit more subtle. In
When that fires, Wolf's Please have another look :-) |
73d89e6 to
12d9d96
Compare
|
You are right about the moonlight code, also, looking at our code, it seems that some clients might not pass back that IP: Other than the added complexity, my concern is that it's kind of a breaking change:
There's an alternative model to this (which is what Sunshine uses IIRC) coupling HTTPS and RTSP. We know that after a Food for thoughts 😉 |
12d9d96 to
a93c0d8
Compare
Two Moonlight clients behind the same NAT can both send RTSP with `Host: 0.0.0.0` (moonlight-common-c does this below the high-quality-audio threshold), which makes Wolf's RTSP demux fall back to source-IP matching and route both to whichever session is first in the list — swapping their AES keys and their ENET `X-SS-Connect-Data` (so one client's input lands in the other's session). Couple the HTTPS launch/resume with the RTSP handshake that follows it (the model Sunshine uses): a launching client gets an exclusive RTSP window for its IP, and a concurrent same-IP launch blocks until that window closes. While a window is open, ambiguous `Host: 0.0.0.0` packets from that IP resolve to its session. The window is held for the whole handshake and closed on the RTSP PLAY (with a safety timeout), so no packet — including the SETUP that carries the ENET secret — escapes to another session. No new ports, no config; single-client and distinct-IP behaviour is unchanged.
a93c0d8 to
91c55f8
Compare
|
I clicked the wrong button, I just wanted to say that @ABeltramo's suggestion works great! I updated the change accordingly, please have a look. :-) |
Wolf gives each session a random fake IP in the launch response and expects the client to echo it back as the RTSP
Host/URI — that's how it tells concurrent sessions apart. But below the high-quality-audio thresholdmoonlight-common-csendsHost: 0.0.0.0and drops it, and there is no other per-session identifier in the RTSP handshake (theSessionheader is a constant). So two clients behind the same NAT both arrive as0.0.0.0from the same source IP, Wolf routes both to the first session, and their AES keys and ENET secrets get swapped — video corrupts and one client's input lands in the other's session.Since the client can't reliably tell us which session it is, this couples each HTTPS launch/resume with the RTSP handshake that follows it: the launching client owns an exclusive RTSP window for its source IP, held until the handshake finishes (RTSP
PLAY). While the window is open, ambiguous0.0.0.0packets from that IP resolve to that session; a concurrent same-IP launch waits until the window closes, so two handshakes never overlap.A second distinct client on the same NAT that launches mid-handshake blocks at its own
/launchuntil the first window closes, so the two handshakes are serialized and never resolve to each other.Clients that do echo the fake IP (above the threshold) are still matched directly by it — the window only covers the
0.0.0.0case.