Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rate-limiter receiving internal IP of proxy container #263

Open
sre3 opened this issue Jun 9, 2024 · 2 comments
Open

Rate-limiter receiving internal IP of proxy container #263

sre3 opened this issue Jun 9, 2024 · 2 comments
Assignees
Labels
domain: cadence-api Relates to cadence API

Comments

@sre3
Copy link

sre3 commented Jun 9, 2024

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 (containers/podman#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
Copy link
Owner

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.)

cadence/server.py

Lines 602 to 609 in 9709d9a

# Handle standard headers which require more processing
if vals[0]=="X-Forwarded-For":
# X-Forwarded-For includes a list of forwarding proxies we don't care about.
value=value.partition(",")[0]
# Set our IP to be that value
self.IP=value
return

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.

@kenellorando kenellorando added the domain: cadence-api Relates to cadence API label Jun 12, 2024
@kenellorando kenellorando self-assigned this Jun 12, 2024
@sre3
Copy link
Author

sre3 commented Jun 12, 2024

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: cadence-api Relates to cadence API
Projects
None yet
Development

No branches or pull requests

2 participants