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

using with trusted_proxies / behind another proxy #19

Open
robgordon89 opened this issue Dec 15, 2022 · 8 comments
Open

using with trusted_proxies / behind another proxy #19

robgordon89 opened this issue Dec 15, 2022 · 8 comments
Labels
question Further information is requested

Comments

@robgordon89
Copy link

Thanks for your work on this, I am looking to implement this plugin to stop spam at the caddy level and rate limiting seems to be the best thing to do.

I am having an issue whereby the limits work but they will rate limit all requests with placeholder remote_host, I believe this is because its outside of the reverse_proxy handler and so trusted_proxies does not run before rate_limit

Is there a way to accomplish this?

@mholt
Copy link
Owner

mholt commented Dec 18, 2022

I'm not sure I understand the question -- can you elaborate? And maybe share your config, the problem, etc?

@robgordon89
Copy link
Author

Hey, sorry let me try and explain better, I am trying to use this plugin to rate limit users to our landing pages, the issue I am having is that the rate limit is applied to everyone as we use caddy behind a proxy / CDN we get the wrong IP.

I am pretty sure it is related to caddyserver/caddy#4924

Here is my rate limit config

rate_limit {
    distributed
    zone ip_rate {
        key    {remote_host}
        events {$CADDY_RATE_EVENTS:100}
        window {$CADDY_RATE_WINDOW:60s}
    }
}

The key here will be one of Cloudflare's IPs and so we rate limit multiple users.

Thanks
Bob

@mholt
Copy link
Owner

mholt commented Dec 19, 2022

Oh, yes I think that is related. You could also use the X-Forwarded-For header (if you trust that your clients are all coming from your CDN). @francislavoie might actually have more expertise with this.

@mholt mholt added the question Further information is requested label Dec 19, 2022
@francislavoie
Copy link

francislavoie commented Dec 19, 2022

I think once caddyserver/caddy#5103 is merged, this plugin could inherit from that functionality to get the "real client IP".

For now, there's no "safe" way of doing it for this plugin on its own.

The caddyhttp package should probably provide an API for making this easy for plugins to add/implement (i.e. parsing/loading IP CIDRs for trusted proxies, and comparing the request against the trust list, pulling trust list from the server if necessary, etc).

@robgordon89
Copy link
Author

👋 hey @francislavoie so looks like we are on the last part of this and we will soon be able to use client_ip in the above rate limit is that correct?

caddyserver/caddy#5104

rate_limit {
    distributed
    zone ip_rate {
        key    {client_ip}
        events {$CADDY_RATE_EVENTS:100}
        window {$CADDY_RATE_WINDOW:60s}
    }
}

Might see if I can build from this branch and see if i can get it working

@francislavoie
Copy link

That's right. There's no {client_ip} placeholder in that branch yet though I don't think. There's still some work to be done on it to make sure everything is correct. It's security sensitive so I need to make sure we get it right.

@robgordon89
Copy link
Author

cool, no worries it looks good to me anyways cant wait to test it out 👍

@mholt
Copy link
Owner

mholt commented Apr 28, 2023

I believe this got merged in now if you want to try it (requires latest on master, as it's unreleased still).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants