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 limiting by IPv6 block #4

Closed
divine opened this issue Oct 24, 2021 · 19 comments
Closed

Rate limiting by IPv6 block #4

divine opened this issue Oct 24, 2021 · 19 comments

Comments

@divine
Copy link

divine commented Oct 24, 2021

Hello,

First of all, thank you for a great library that helps rate-limiting the number of requests.

Secondly, I'm having an issue with IPv6 addresses. As you might know that a simple /64 block assigned to customer contains millions of IP addresses and simple rate_limit * {remote.ip} 1r/m just doesn't work as that customer could just use another IPv6 address from his own block.

Do you have any suggestions or maybe some kind of a feature might be implemented to solve rate-limiting for the IPv6 by blocks as well?

Thanks!

@RussellLuo
Copy link
Owner

Hi @divine, glad to hear that this library helps you ;)

In order to rate-limit requests by IPv6 blocks (or more generally, CIDR blocks), the only solution I can come up with is to use the prefix of the IP address for rate-limiting.

For example, for a /64 block, we can use the first 64 bits of the IP as the actual key, by applying a converter cidr_prefix (with an extra parameter 64) to {remote.ip}:

localhost:8080 {
    route /foo {
        rate_limit {remote.ip}|cidr_prefix:64 2r/m

        respond 200
    }
}

While the syntax becomes a little bit more complicated, I think the format of <key>[|<converter>] may be more versatile for other possible conversions.

Any suggestions?

@divine
Copy link
Author

divine commented Oct 30, 2021

Hello,

Thank you so much for your kind response.

I do like the IPv6 address and trying to use it whenever possible but without rate-limiting, people are trying to abuse the API that we're providing for free.

This looks fine but what about IPv4 addresses? I mean cidr_prefix_v6:64 should be better in terms of separating the two?

Also, this module actually needs a configuration of the white list of trusted proxies, otherwise, this block of code allows bypassing any limitation:

if fwdFor := r.Header.Get("X-Forwarded-For"); fwdFor != "" {
remote = strings.TrimSpace(strings.Split(fwdFor, ",")[0])
}

You get an idea, right? Anyone could just provide a fake X-Forwarded-For header and this module will think that it's another IP address and people have actually noticed it and bypassed any rate limit 😄.

Thanks!

@RussellLuo
Copy link
Owner

This looks fine but what about IPv4 addresses? I mean cidr_prefix_v6:64 should be better in terms of separating the two?

If cidr_prefix is designed to just apply a CIDR mask of leading bits to an IP (like IP.Prefix), it may be agnostic of IP version (IPv4 or IPv6)?

Anyone could just provide a fake X-Forwarded-For header and this module will think that it's another IP address...

Yes, this is the case.

Also, this module actually needs a configuration of the white list of trusted proxies.

I agree that this is a feasible solution, but I'm a little concerned about the increased responsibility of ratelimit, which should be focused on rate-limiting. I think "configuration of trusted proxies (or trusted addresses)" should be the responsibility of other Caddy module, much like the ngx_http_realip_module module in Nginx.

@divine
Copy link
Author

divine commented Oct 30, 2021

If cidr_prefix is designed to just apply a CIDR mask of leading bits to an IP (like IP.Prefix), it may be agnostic of IP version (IPv4 or IPv6)?

This is indeed fine but what will happen if I wanted cidr_prefix:64 for IPv6 and cidr_prefix:24 for IPv4, so you'll need to split configurations I think... otherwise the same configuration might be applied to both versions...

I agree that this is a feasible solution, but I'm a little concerned about the increased responsibility of ratelimit, which should be focused on rate-limiting. I think "configuration of trusted proxies (or trusted addresses)" should be the responsibility of other Caddy module, much like the ngx_http_realip_module module in Nginx.

Looks good, does that mean you'll remove that line of code that detects IP addresses from X-Forwarded-For header?

Thanks!

@RussellLuo
Copy link
Owner

... if I wanted cidr_prefix:64 for IPv6 and cidr_prefix:24 for IPv4

If ratelimit has a matcher, will it be helpful for your use case? For example:

localhost:8080 {
    route /foo {
        @ipv4 {
            remote_ip 192.0.2.1/24  # possible IPv4 addresses or ranges
        }
        rate_limit @ipv4 {remote.host_prefix.24} 2r/m

        @ipv6 {
            remote_ip 2001:db8::1/64  # possible IPv6 addresses or ranges
        }
        rate_limit @ipv6 {remote.host_prefix.64} 2r/m

        respond 200
    }
}

does that mean you'll remove that line of code that detects IP addresses from X-Forwarded-For header?

I find that the builtin placeholder {http.request.remote.host} is just request.RemoteAddr, so you can use it in your case. And I want to keep the {remote.ip} aware of the X-Forwarded-For header for other use cases.

After some more thought, I also think <key>[|<converter>] is complicated. For simplicity, we can just support more keys as below:

  • {remote.host} # Existing (X-Forwarded-For unaware)
  • {remote.host_prefix.<bits >} # Newly added
  • {remote.ip} # Existing (X-Forwarded-For aware)
  • {remote.ip_prefix.<bits >} # Newly added

@divine
Copy link
Author

divine commented Nov 2, 2021

Hello,

Sorry for the late response.

If ratelimit has a matcher, will it be helpful for your use case?

Actually, no and let me explain why.

I'll just be using only one global prefix as a rate limiter : rate_limit {remote.host_prefix.64} 2r/m. No exceptions for all addresses.

After thinking a little bit, maybe it will work if I add all 0.0.0.0/0.

Objections I had it was what if I want to rate-limit global /64 prefix for IPv6 and global /24 prefix for IPv4 what will happen?

I'm seeing a lot of proxies from the same /24 network prefix as well, they look same and it's definitely bots that might create issues later (an example):
1.1.1.0
1.1.1.1
1.1.1.2
...
and etc.

Thanks!

@RussellLuo
Copy link
Owner

Sorry for the late response.

Never mind ;)

I'll just be using only one global prefix as a rate limiter : rate_limit {remote.host_prefix.64} 2r/m. No exceptions for all addresses.

...

Objections I had it was what if I want to rate-limit global /64 prefix for IPv6 and global /24 prefix for IPv4 what will happen?

Suppose that we already have two different keys, say, as below:

  • {remote.host_v4_prefix.<bits>}
  • {remote.host_v6_prefix.<bits>}

I'm not sure how to leverage these keys to apply rate-limiting by using only one rate_limit handler, which allows only one <key>. Do you have any ideas?

@divine
Copy link
Author

divine commented Nov 2, 2021

Hello,

Using different keys will be fine as well, I just wanted to say that one key wasn't enough, otherwise looks good :)

Thank you!

@RussellLuo
Copy link
Owner

By "using different keys will be fine", do you mean that you will use two rate_limit handlers (one for IPv4, and the other for IPv6)?

It will be very helpful if you can share me the complete Caddyfile configuration in your mind :) Thanks!

@RussellLuo
Copy link
Owner

RussellLuo commented Nov 4, 2021

If I understand your case correctly, I think the following configuration should work:

{
  auto_https off
}

:80 {
  route /foo {
    @ipv4 {
      remote_ip 0.0.0.0/0  # All IPv4 addresses
    }
    # Only apply the following limit if it's an IPv4 address.
    rate_limit @ipv4 {remote.host_prefix.24} 1r/m

    @ipv6 {
      remote_ip ::/0  # All IPv6 addresses
    }
    # Only apply the following limit if it's an IPv6 address.
    rate_limit @ipv6 {remote.host_prefix.64} 2r/m

    respond 200
  }
}

This is the result I got on my Mac:

# IPv4 case (1r/m)
$ curl -w "%{http_code}" 'http://localhost/foo'
200
$ curl -w "%{http_code}" 'http://localhost/foo'
429

# IPv6 case (2r/m)
$ curl -g -6 -w "%{http_code}" 'http://[::1]/foo'
200
$ curl -g -6 -w "%{http_code}" 'http://[::1]/foo'
200
$ curl -g -6 -w "%{http_code}" 'http://[::1]/foo'
429

And the corresponding DEBUG logs (for the above two 429s):

DEBUG   http.handlers.rate_limit        request is rejected     {"variable": "{remote.host_prefix.24}", "value": "127.0.0.0/24"}
DEBUG   http.handlers.rate_limit        request is rejected     {"variable": "{remote.host_prefix.64}", "value": "::/64"}

@divine
Copy link
Author

divine commented Nov 5, 2021

Hello,

I just wanted to say thank you, it was quite fast with adding that feature.

I have currently deployed with your example configuration and it works like a charm.

The only question I have what if I want to white list some IPv4 adress block like 192.168.1.0/24 (no limits at all)
would another configuration override the global remote_ip 0.0.0.0/0?

{
  auto_https off
}

:80 {
  route /foo {
    @ipv4 {
      remote_ip 0.0.0.0/0  # All IPv4 addresses
    }

    @ipv4local {
      remote_ip 192.168.1.0/24  # Local network IPv4 addresses
    }

    # Only apply the following limit if it's an IPv4 address.
    rate_limit @ipv4 {remote.host_prefix.24} 1r/m

    # Apply no limits?
    rate_limit @ipv4local {remote.host_prefix.24} ??

    @ipv6 {
      remote_ip ::/0  # All IPv6 addresses
    }
    # Only apply the following limit if it's an IPv6 address.
    rate_limit @ipv6 {remote.host_prefix.64} 2r/m

    respond 200
  }
}

Thanks!

@RussellLuo
Copy link
Owner

RussellLuo commented Nov 5, 2021

Caddy supports a not matcher, you can try this configuration for IPv4 case:

@ipv4 not {  # All IPv4 addresses except Local ones
  remote_ip ::/0  #  not IPv6 addresses
  remote_ip 192.168.1.0/24  # and not local addresses
}
rate_limit @ipv4 {remote.host_prefix.24} 1r/m

@divine
Copy link
Author

divine commented Nov 5, 2021

Hello,

I do really appreciate your help, this works like a charm!

Enabled IPv6 back and deployed to production for a few million of users.

Thank you so much!

@divine divine closed this as completed Nov 5, 2021
@divine
Copy link
Author

divine commented Nov 12, 2021

Hello,

Sorry for reopening this again but looks like somebody was able to bypass remote.host_prefix and looking at documentation https://caddyserver.com/docs/modules/http.matchers.remote_host this is definitely possible.

http.matchers.remote_host matches based on the remote IP of the connection. A host name can be specified, whose A and AAAA DNS records will be resolved to a corresponding IP for matching.

Note that IPs can sometimes be spoofed, so do not rely on this as a replacement for actual authentication.

So I don't think that remote host is what I should be using as the number of connections is not rate limited:

741 175.100.x.x
1312 209.212.x.x

741 & 1312 are the total number of connections.

My caddy config:

@ipv4 {
  remote_ip 0.0.0.0/0  # All IPv4 addresses
}
# Only apply the following limit if it's an IPv4 address.
rate_limit @ipv4 {remote.host_prefix.32} 8r/s

@ipv6 {
  remote_ip ::/0  # All IPv6 addresses
}
# Only apply the following limit if it's an IPv6 address.
rate_limit @ipv6 {remote.host_prefix.64} 8r/s

After that I've modified rate limit plugin it and disabled using "X-Forwarded-For" here:

return v.evaluatePrefix(r, true)

and now everything looks quite good.

@ipv4 {
  remote_ip 0.0.0.0/0  # All IPv4 addresses
}
# Only apply the following limit if it's an IPv4 address.
rate_limit @ipv4 {remote.ip_prefix.32} 8r/s

@ipv6 {
  remote_ip ::/0  # All IPv6 addresses
}
# Only apply the following limit if it's an IPv6 address.
rate_limit @ipv6 {remote.ip_prefix.64} 8r/s

Thanks!

@divine divine reopened this Nov 12, 2021
@RussellLuo
Copy link
Owner

RussellLuo commented Nov 13, 2021

Hello, I think these plugin names are confusing here ;)

In our discussion there are two types of plugins in Caddy:

  1. Matchers
    • http.matchers.remote_ip (STANDARD)
    • http.matchers.remote_host (NON-STANDARD, see here)
  2. Placeholders
    • {remote.host} (STANDARD, shorthand of {http.request.remote.host})
    • {remote.host_prefix} (NON-STANDARD, implemented by ratelimit)
    • {remote.ip} (NON-STANDARD, implemented by ratelimit)
    • {remote.ip_prefix} (NON-STANDARD, implemented by ratelimit)

The evaluation of {remote.host_prefix} is as below:

case "{http.request.remote.host_prefix}":
return v.evaluatePrefix(r, false)

As you can see, the second parameter false means that the forwarded flag is disabled. Thus when calculating the client IP, the X-Forwarded-For header will be ignored:

if forwarded {
if fwdFor := r.Header.Get("X-Forwarded-For"); fwdFor != "" {
remote = strings.TrimSpace(strings.Split(fwdFor, ",")[0])
}
}

In conclusion, I think {remote.host_prefix should work as expected. The problem you mentioned above may be due to other reasons?

@divine
Copy link
Author

divine commented Nov 13, 2021

Hello,

Thank you so much for your kind reply and explanation!

Understood now but I think somehow somebody was bypassing host_prefix limit or just the problem is with the /32* prefix and the limit doesn't work in that case as expected?

*rate_limit @ipv4 {remote.ip_prefix.32} 8r/s

Thanks!

@RussellLuo
Copy link
Owner

After that I've modified rate limit plugin it and disabled using "X-Forwarded-For" here:

return v.evaluatePrefix(r, true)

and now everything looks quite good.

In theory, your modified version of {remote.ip_prefix.<bits>} should behave just the same as {remote.host_prefix.<bits>}, since the only difference between them is the forwarded flag.

741 175.100.x.x
1312 209.212.x.x

741 & 1312 are the total number of connections.

Here are some questions I think we need to figure out:

  1. Where did these IP come from in the requests? Were they passed via the X-Forwarded-For header?
  2. Can we try to do some experiments locally by sending requests with the same IPs, and see what will happen? (Furthermore, we can also enable the DEBUG mode, and then watch the DEBUG logs.)

@divine
Copy link
Author

divine commented Apr 8, 2022

Hello,

Sorry about the late reply but everything has been working like a charm! Well the issue I had wasn't related to this plugin.

I'll report if any issues will arise.

Thank you!

@divine divine closed this as completed Apr 8, 2022
@RussellLuo
Copy link
Owner

Glad to hear that! Feel free to open new issues if needed :)

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

No branches or pull requests

2 participants