Allow quotas to be keyed by proxy-forwarded IP address; add support for PROXY protocol#17707
Allow quotas to be keyed by proxy-forwarded IP address; add support for PROXY protocol#17707alexey-milovidov merged 15 commits intomasterfrom
Conversation
robot-clickhouse
added
doc-alert
pr-feature
alexey-milovidov
force-pushed
the
quota-by-x-forwarded-for
branch
from
7c1b810 to
c9aa412
Compare
|
It is similar to |
| params.quota_key = client_info.quota_key; | ||
|
|
||
| /// Extract the last entry from comma separated list of X-Forwarded-For addresses. | ||
| /// Only the last proxy can be trusted (if any). |
There was a problem hiding this comment.
Still it seems to be more correct to get the first address from X-Forwarded-For if we want to count quota usage for each client IP separately because the client IP is the first address in X-Forwarded-For.
There was a problem hiding this comment.
It will allow clients to sent arbitrary quota keys (by forging X-Forwarded-For header)... but the purpose of this feature is to ensure per-IP limits similar to keyed_by_ip but with presense of proxy.
There was a problem hiding this comment.
So, the goal is to "unwrap" one level of proxy.
| params.quota_key = client_info.quota_key; | ||
|
|
||
| /// Extract the last entry from comma separated list of X-Forwarded-For addresses. | ||
| /// Only the last proxy can be trusted (if any). |
There was a problem hiding this comment.
Why only the last proxy can be trusted?
There was a problem hiding this comment.
Only the last proxy can be trusted, because we can limit access from that proxy with the list of IP networks.
Otherwise user can manually send request with any X-Forwarded-For.
For example, I will limit the list of IP networks for user with this: https://www.cloudflare.com/ips/ and then make quota keyed by X-Forwarded-For.
| NONE, /// All users share the same quota. | ||
| USER_NAME, /// Connections with the same user name share the same quota. | ||
| IP_ADDRESS, /// Connections from the same IP share the same quota. | ||
| FORWARDED_IP_ADDRESS, /// Use X-Forwarded-For HTTP header instead of IP address. |
There was a problem hiding this comment.
The name FORWARDED_IP_ADDRESS doesn't seem to be clear enough. Maybe it's better to rename it into something like PROXY_ORIGIN_IP_ADDRESS?
There was a problem hiding this comment.
The address that proxy server forwards to us... also the analogy with X-Forwarded-For HTTP header.
There was a problem hiding this comment.
I understood, it just seems that the name FORWARDED_IP_ADDRESS is not very good because it requires to know that HTTP header and doesn't even contain the word PROXY.
alexey-milovidov
changed the title
robot-clickhouse
added
the
submodule changed
|
Looks like this breaks perf tests (due to port conflict for |
4 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add support for PROXYv1 protocol to wrap native TCP interface. Allow quotas to be keyed by proxy-forwarded IP address (applied for PROXYv1 address and for X-Forwarded-For from HTTP interface). This is useful when you provide access to ClickHouse only via trusted proxy (e.g. CloudFlare) but want to account user resources by their original IP addresses. This fixes #17268.