[
https://issues.apache.org/jira/browse/KAFKA-16707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17856929#comment-17856929
]
Franck LEDAY edited comment on KAFKA-16707 at 6/22/24 1:49 PM:
---------------------------------------------------------------
Hi Greg.
I'm pleased to get a return on my JIRA.
To answer you, I hope my explanation won't be too confusing : you're right, I'm
adding new Principal Type, BUT, the perimeter of validity is only in
StandardACL part of the KRAFT implementation, and to defined how the ACL will
check the credential of the client connected to the broker. Those new types
oare not to be used in a PrincipalKafka used for a connected client.
Also, I understand that you fear a confusing usage of this, and so, I suggest
that I change those specific Principal Type for Standard ACL by adding a
prefix like Acl to avoid confusion, nor collision, it would give somehting like:
* Regex > AclRegex
* StartsWith > AclStartsWith
* EndsWith > AclEndsWith
* Contains > AclContains
* User > User to keep it as before, in order to not change the basic behavior.
With those informations, do you believe I have to create a KIP?
(Note : I switch the PR to WIP as I will have less time over next month on this
subject as previous months, but I'm still present for modification and test).
(Also, as I didn't remember reference of other Principal Type than User in the
Kafka doc, may I ask you to point me where in the doc (and code) there are
checks on Principal Type of other value than User?)
was (Author: handfreezer):
Hi Greg.
I'm pleased to get a return on my JIRA.
To answer you, I hope my explanation won't be too confusing : you're right, I'm
adding new Principal Type, BUT, the perimeter of validity is only in
StandardACL part of the KRAFT implementation, and to defined how the ACL will
check the credential of the client connected to the broker. Those new types
oare not to be used in a PrincipalKafka used for a connected client.
Also, I understand that you fear a confusing usage of this, and so, I suggest
that I change those specific Principal Type for Standard ACL by adding a
prefix like Acl to avoid confusion, nor collision, it would give somehting like:
* Regex > AclRegex
* StartsWith > AclStartsWith
* EndsWith > AclEndsWith
* Contains > AclContains
* User > User to keep it as before, in order to not change the basic behavior.
With those informations, do you believe I have to create a KIP?
(Note : I switch the PR to WIP as I will have less time over next month on this
subject as previous months, but I'm still present for modification and test).
> Kafka Kraft : adding Principal Type in StandardACL for matching with
> KafkaPrincipal of connected client in order to defined ACL with a notion of
> group
> ------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: KAFKA-16707
> URL: https://issues.apache.org/jira/browse/KAFKA-16707
> Project: Kafka
> Issue Type: Improvement
> Components: kraft, security
> Affects Versions: 3.7.0, 3.8.0, 3.7.1
> Reporter: Franck LEDAY
> Assignee: Franck LEDAY
> Priority: Major
> Labels: KafkaPrincipal, acl, authorization, group, metadata,
> security, user
> Original Estimate: 0h
> Remaining Estimate: 0h
>
> Default StandardAuthorizer in Kraft mode is defining a KafkaPrincpal as
> type=User and a name, and a special wildcard eventually.
> The difficulty with this solution is that we can't define ACL by group of
> KafkaPrincipal.
> There is a way for the moment to do so by defining RULE to rewrite the
> KafkaPrincipal name field, BUT, to introduce this way the notion of group,
> you have to set rules which will make you loose the uniq part of the
> KafkaPrincipal name of the connected client.
> The concept here, in the StandardAuthorizer of Kafka Kraft, is to add the
> management of KafkaPrincipal type:
> * Regex
> * StartsWith
> * EndsWith
> * Contains
> * (User is still available and keep working as before to avoid any
> regression/issue with current configurations)
> This would be done in the StandardAcl class of metadata/authorizer, and the
> findresult method of StandardAuthorizerData will delegate the match to the
> StandardAcl class (for performance reason, see below explanation).
> By this way, you can still use RULEs to rewrite KafkaPrincipal name of
> connected client (say you want to transform a DN of SSL certificate :
> cn=myCN,ou=myOU,c=FR becomes myCN@myOU), and then, you can define a new ACL
> with principal like: 'Regex:^.*@my[oO]U$' that will match all connected
> client with a certificate bind to ou=myOU . Note in this particular case, the
> same can be done with 'EndsWtih:@myOU', and the type 'Contains' can work, but
> I imagine more the usage of this type for matching in a multigroup definition
> in a KafkaPrincipal.
>
> Note about performance reason : for the moment, I have it implemented in a
> fork of StandardAuthroizer/StandardAuthroizerData/StandardAcl defined by the
> property authorizer.class.name in a cluster of Kraft with SSL authentication
> required and tested fine. But, by this way, every time that an ACL is checked
> against a KafkaPrincipal, I do a strcmp of the KafkaPrincipal type of the ACL
> to determine the matching method to be done. By implementing it in
> StandardAcl class, and then delegating the matching from
> StandardAuthorizerData to the StandardAcl class, this allow to analyse and
> store the type of the KafkaPrincipal method for matching as an enum, and the
> KafkaPrincipal name separately in order to avoid redoing the job each time a
> match has to be checked.
>
> Here is my status of the implementation:
> * I have this solution ('performance reason') implemented in fork (then
> branch) of the 3.7.0 github repo,
> * I added few unit test, and a gradlew metadata:test is working fine on all
> tests except one (witch is failing also on branch 3.7.0 without my changes),
> * I added few lines about in security.html .
>
> I'm opening the issue to discuss it with you, because I would like to create
> a PR on Github for next version.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
