kenellorando / cadence

All-in-one web radio suite with browser UI, music search, request, artwork, and real-time stream information.
https://cadenceradio.com
61 stars 17 forks source link

Rate-limiter receiving internal IP of proxy container #263

Open sre3 opened 3 months ago

sre3 commented 3 months ago

I would like to preface this issue with a note that my setup is rather unusual - I using an Oracle free tier to mess around with podman before deploying to my main server, and as a result I tend to be a very niche use case.

Initially, I had the stack running rootless with caddy as a reverse proxy. This was working fine, with one notable exception: in rootless podman, all remote address will have an IP of the forwarded container, due to the way podman handles rootless port forwarding (https://github.com/containers/podman/issues/8193). This means, of course that the rate-limiter also receives that IP. Now, being myself, I looked around for alternatives and found this[1], which I am now using.

Here is the run command for the cadence server, for example:

sudo podman run -d \
--name=cadence \
--uidmap="0:$(id -u opc):1" \
--uidmap="1:$(grep -Po '(?<=^opc:).*$' /etc/subuid | head -1)" \
--requires=liquidsoap,postgres,icecast2,redis \
--env-file /home/opc/cadence/config/cadence.env \
--label "io.containers.autoupdate=registry" \
-v /home/opc/radio:/home/opc/radio:z \
--user 103 \
--net cadencenet \
--net caddynet \
--restart always \
docker.io/kenellorando/cadence

This has the desired effect - caddy now has the proper IP:

"request":{"remote_ip":"198.54.132.254","remote_port":"59365","client_ip":"198.54.132.254","proto":"HTTP/2.0","method":"GET","host":"cadence:8080","uri":"/api/radiodata/sse"

Unfortunately, cadence doesn't seem to agree:

2024/06/09 19:29:21 INFO IP <10.89.0.7> is rate limited. func=rateLimitArt

Where 10.89.0.7 is caddy's IP on caddynet. This leads me to believe that cadence is not properly trusting external proxies, as caddy sends all required headers by default. Everything else is working fine, as with the completely rootless setup.

[1] There are other options, such as --network slirp4netns:port_handler=slirp4netns and using a pod, but I was too lazy to reconfigure cadence's networking.

kenellorando commented 3 months ago

Hi @sre3, thanks for your interest in my repo!

As I understand the issue (please correct me if I'm wrong), we're running Cadence as a service behind a reverse proxy provided by Caddy @ 10.89.0.7. A request from 198.54.132.254 is forwarded by Caddy to the Cadence API, but is showing up as Caddy's own IP 10.89.0.7, thus rate limiting all requests forwarded through Caddy.

I just replicated the issue on my own deployment with the Cadence-provided nginx reverse proxy. It's logging the proxy's IP regardless of client. Your setup is niche but I think it's revealed to me a problem which has been flying under my radar for a long time. It must have been present in my deployment since I introduced the reverse proxy component, and I think I missed it because I imagine most Cadence users run on a small enough scale where this is not noticeable.

Admittedly, Cadence's current rate limiter is a quick in-house invention which does a basic check through Go's RemoteAddr, which I realize now simply returns the last proxy IP. I think what I'd probably need to do is specifically check for the presence of a different header, or look at the leftmost X-Forwarded-For value for the originating address.

(... Of course, on a six year old version of Cadence written in Python, @za419 seems to have already done it.)

https://github.com/kenellorando/cadence/blob/9709d9ad420b1f8a90b43319b0cc78376689e853/server.py#L602-L609

If I re-implement something like this into the current version, there is a security consideration, as selecting an X-Forwarded-For "most original" address does make it easier for someone to spoof an address and bypass rate limiting. I'll give it some thought to how I will approach this and return to it another time.

Thanks for raising this issue.

sre3 commented 3 months ago

Hi, Thanks for your prompt reply!

Yes, exactly.

One way of mitigating spoofing is in the reverse proxy itself - caddy, for instance, will strip X-Forwarded-For values unless the proxy is trusted

Perhaps a similar system could be implemented in cadence, where only X-Forwarded-For values from a trusted proxy will be used, and nginx could be setup to wipe the X-Forwarded-For values and replace it with RemoteAddr (as caddy does by default with an untrusted proxy)... I believe the only pitfall here is that a proxy server being used by multiple people can be blocked... But as you've said, small deployments, probably an acceptable edge case, etc. etc.

Then with docker's network aliases, I guess you could just set nginx to be trusted by default, which should resolve to the proxy internal IP, and then offer an environment variable to change that value? And I guess if the user doesn't enable the proxy, the script could ask for an IP or string and if nothing is provided it could be set to a value that would disable the use of X-Forwarded-For and use RemoteAddr instead and revert back to the current behaviour.

Anyway, that's just my two cents. There is a very exhaustive article on the perils of X-Forwarded-For here

Also, thank you for creating cadence - I was looking for something reasonably lightweight and simple, and all the other alternatives are either terribly outdated or super resource hungry and overkill (cough cough azuracast cough cough) I was about to try setting up a liquidsoap + icecast setup myself, but cadence is perfect :)