You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/10/30 10:59:24 UTC
[GitHub] [kafka] dongjinleekr opened a new pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
dongjinleekr opened a new pull request #11454:
URL: https://github.com/apache/kafka/pull/11454
I inspected this issue a little bit.
As shown in the first, proof-of-problem commit, the quota limit does not work correctly when the user is `""` or `ANONYMOUS` and the client id is `""` or `null`. We can fix it by treating `ANONYMOUS` as a default user and using client quota (i.e., `/config/clients/<default>`) when the client id is `null`.
As far as I understood, this issue is just an edge case in the current implementation, not a regression.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dongjinleekr commented on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-962420341
Hi @splett2,
I inspected this issue a little bit.
According to [KIP-55](https://cwiki.apache.org/confluence/display/KAFKA/KIP-55%3A+Secure+Quotas+for+Authenticated+Users), the notion of client quota and user principal was introduced in 0.9, and [the per-user principal quota configuration was adopted in 0.10](https://github.com/apache/kafka/pull/1753) by @rajinisivaram.
> User principal is an authenticated user or a grouping of unauthenticated users chosen by the broker using a configurable PrincipalBuilder and is currently used for ACLs.
> Multi-user systems have a hierarchy - user owns zero or more clients. <user-principal, client-id> defines a safe group of clients. The shorter unsafe client-id is sufficient in client metrics and logs, but **quotas should be allocated to safe groups to avoid clients of one user throttling clients of another user with the same client-id.**
> For a given client connection, the most specific quota matching the connection will be applied.
Every time Kafka client creates a `KafkaChannel` to the server with `ChannelBuilder`, it makes a `KafkaPrincipal` - the default implementation, `DefaultKafkaPrincipalBuilder#build`, returns `KafkaPrincipal.ANONYMOUS` when Plaintext connection of SSL authentication fails. So, it seems like we should treat `KafkaPrincipal.ANONYMOUS` as a default user principal and not applying the (null or empty) client quota limit to `KafkaPrincipal.ANONYMOUS` seems like a bug.
In fact, after reviewing the related code, I found that I was taking the wrong approach here; Since `DefaultQuotaCallback#quotaMetricTags` returns appropriate metric tag per given <user principal, client id> and `DefaultQuotaCallback#quotaLimit` calculates the quota per given metric tag in precedence order, the null or empty client case should be handled by `DefaultQuotaCallback#quotaMetricTags`, not `DefaultQuotaCallback#quotaMetricTags`.
So I updated the PR. Following [KIP-55](https://cwiki.apache.org/confluence/display/KAFKA/KIP-55%3A+Secure+Quotas+for+Authenticated+Users), it will use `/config/users/<default>` and `/config/clients/<default>` in order.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dongjinleekr commented on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-955650953
Hi @splett2,
It seems like this issue is somewhat overlapped one, like:
1. Null client IDs were being improperly propagated (addressed by #11385.)
2. `ClientQuotaManager` does not handle the null or empty client id (i.e., this one.)
Since 1 is already fixed, this PR should focus on 2. And what we need to decide are:
1. Whether to treat 'ANONYMOUS' user principal to an empty one.
2. Whether to use `/config/clients/<default>` or `/config/user/<default>` if we have both an empty user and empty clientId.
Are we on the same page now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] flysen edited a comment on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
flysen edited a comment on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-963164070
@dongjinleekr Ill follow up to this with our findings. Not 100% sure it's relevant. When `/config/clients/<default>` exist no new JMX metrics for `kafka.server:type=Produce,user=xx` will be populated, only `kafka.server:type=Request,client-id=producer-NNNN`
How to reproduce:
```
# Create a default clients quota
kafka-configs.sh --zookeeper zk-1.net --alter --entity-type clients --entity-default --add-config producer_byte_rate=1000
# Confirm created
kafka-configs.sh --zookeeper zk-1.net --describe --entity-type clients --entity-default
--> Configs for default client-id are producer_byte_rate=1000
# Check ZK
ls /config/clients --> [<default>]
get /config/clients/<default> --> {"version":1,"config":{"producer_byte_rate":"1000"}}
# Delete default quota for client
kafka-configs.sh --zookeeper zk-1.net --alter --entity-type clients --entity-default --delete-config producer_byte_rate
# Confirm deleted
kafka-configs.sh --zookeeper zk-1.net --describe --entity-type clients --entity-default
--> Configs for default client-id are
# Check ZK
ls /config/clients --> [<default>]
get /config/clients/<default> --> {"version":1,"config":{}}
```
Now no sensors will be created for users until `/config/clients/<default>` is deleted
```
delete /config/clients/<default>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dongjinleekr commented on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-963550148
Hi @flysen, Thanks for the reporting. It seems like this issue is a little bit coupled with your case. I will have a look around.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dongjinleekr commented on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-955191199
@splett2 Is this the phenomenon you described in [KAFKA-13341](https://issues.apache.org/jira/browse/KAFKA-13341)?
@dajac Could you have a look when you are free? According to the commit logs, you did [the last non-trivial work](https://issues.apache.org/jira/browse/KAFKA-12591) in `ClientQuotaManager.scala`. :bow:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dongjinleekr commented on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
dongjinleekr commented on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-962420365
@rajinisivaram Excuse me. Could you kindly confirm how the client quota is applied to a `KafkaPrincipal.ANONYMOUS` user with null or empty client id? As of present, the quota is not applied to these cases (see the first, proof-of-problem commit), and this PR tries to resolve it by applying quota in `/config/users/<default>` and `/config/clients/<default>` in order. Is this correspond with your original intention?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] flysen commented on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
flysen commented on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-963164070
@dongjinleekr Ill follow up to this with our findings. Not 100% sure it's relevant. When `/config/clients/<default>` exist no new JMX metrics for `kafka.server:type=Produce,user=xx` will be populated, only `kafka.server:type=Request,client-id=producer-NNNN`
How to reproduce:
```
# Create a default clients quota
kafka-configs.sh --zookeeper zk-1.testing.eu1.kaas.klarna.net --alter --entity-type clients --entity-default --add-config producer_byte_rate=1000
# Confirm created
kafka-configs.sh --zookeeper zk-1.testing.eu1.kaas.klarna.net --describe --entity-type clients --entity-default
--> Configs for default client-id are producer_byte_rate=1000
# Check ZK
ls /config/clients --> [<default>]
get /config/clients/<default> --> {"version":1,"config":{"producer_byte_rate":"1000"}}
# Delete default quota for client
kafka-configs.sh --zookeeper zk-1.testing.eu1.kaas.klarna.net --alter --entity-type clients --entity-default --delete-config producer_byte_rate
# Confirm deleted
kafka-configs.sh --zookeeper zk-1.testing.eu1.kaas.klarna.net --describe --entity-type clients --entity-default
--> Configs for default client-id are
# Check ZK
ls /config/clients --> [<default>]
get /config/clients/<default> --> {"version":1,"config":{}}
```
Now no sensors will be created for users until `/config/clients/<default>` is deleted
```
delete /config/clients/<default>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] splett2 commented on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
splett2 commented on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-955601970
thanks. The phenomenon I was describing was addressed by #11385.
Null client IDs were being improperly propagated up from the RPC layer to the broker server-side handling.
I didn't notice that the `ClientQuotaManager` code does not handle empty clientId.
I didn't take a deep look at the client quotas hierarchy spec, but I'm not sure that we would want to translate an `ANONYMOUS` user principal to empty. That seems like a change in behavior.
One thing I would note, it seems weird that if we have both an empty user and empty clientId, we apply `/config/clients/<default>`. For instance, I don't see why we would apply the default clientId quota rather than a default user principal quota. It seems like an arbitrary choice either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] splett2 edited a comment on pull request #11454: KAFKA-13341: Quotas are not applied to requests with null clientId
Posted by GitBox <gi...@apache.org>.
splett2 edited a comment on pull request #11454:
URL: https://github.com/apache/kafka/pull/11454#issuecomment-955601970
thanks. The phenomenon I was describing was addressed by #11385.
Null client IDs were being improperly propagated up from the RPC layer to the broker server-side handling, which led to skipping through the quotas logic entirely.
I didn't notice that the `ClientQuotaManager` code does not handle empty clientId.
I didn't take a deep look at the client quotas hierarchy spec, but I'm not sure that we would want to translate an `ANONYMOUS` user principal to empty. That seems like a change in behavior.
One thing I would note, it seems weird that if we have both an empty user and empty clientId, we apply `/config/clients/<default>`. For instance, I don't see why we would apply the default clientId quota rather than a default user principal quota. It seems like an arbitrary choice either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org