@Jun thanks for the comments.Please see the replies inline. Currently kafka-acl.sh just creates an ACL path in ZK with the principal name string. ----> Yes, the kafka-acl.sh calls the addAcl() on the inbuilt SimpleAclAuthorizer which in turn creates an ACL in ZK with the Principal name string. This is because we supply the SimpleAclAuthorizer as a commandline argument in the Kafka-acls.sh command.
The authorizer module in the broker reads the principal name string from the acl path in ZK and creates the expected KafkaPrincipal for matching. As you can see, the expected principal is created on the broker side, not by the kafka-acl.sh tool. ----> This is considering the fact that the user is using the SimpleAclAuthorizer on the broker side and not his own custom Authorizer. The SimpleAclAuthorizer will take the Principal it gets from the Session class . Currently the Principal is KafkaPrincipal. This KafkaPrincipal is generated from the name of the actual channel Principal, in SocketServer class when processing completed receives. With this KIP, this will no longer be the case as the Session class will store a java.security.Principal instead of specific KafkaPrincipal. So the SimpleAclAuthorizer will construct the KafkaPrincipal from the channel Principal it gets from the Session class. User might not want to use the SimpleAclAuthorizer but use his/her own custom Authorizer. The broker already has the ability to configure PrincipalBuilder. That's why I am not sure if there is a need for kafka-acl.sh to customize PrincipalBuilder. ----> This is exactly the reason why we want to propose a PrincipalBuilder in kafka-acls.sh so that the Principal generated by the PrincipalBuilder on broker is consistent with that generated while creating ACLs using the kafka-acls.sh command line tool. *To summarize the above discussions :* What if we only make the following changes: pass the java principal in session and in SimpleAuthorizer, construct KafkaPrincipal from java principal name. Will that work for LinkedIn? ------> Yes, this works for Linkedin as we are not using the kafka-acls.sh tool to create/update/add ACLs, for now. Do you think there is a use case for a customized authorizer and kafka-acl at the same time? If not, it's better not to complicate the kafka-acl api. -----> At Linkedin, we don't use this tool for now. So we are fine with the minimal change for now. Initially, our change was minimal, just getting the Kafka to preserve the channel principal. Since there was a discussion how kafka-acls.sh would work with this change, on the ticket, we designed a detailed solution to make this tool generally usable with all sorts of combinations of Authorizers and PrincipalBuilders and give more flexibility to the end users. Without the changes proposed for kafka-acls.sh in this KIP, it cannot be used with a custom Authorizer/PrinipalBuilder but will only work with SimpleAclAuthorizer. Although, I would actually like it to work for general scenario, I am fine with separating it under a separate KIP and limit the scope of this KIP. I will update the KIP accordingly and put this under rejected alternatives and create a new KIP for the Kafka-acls.sh changes. @Manikumar Since we are limiting the scope of this KIP by not making any changes to kafka-acls.sh, I will cover your concern in a separate KIP that I will put up for kafka-acls.sh. Does that work? Thanks, Mayuresh On Wed, Feb 15, 2017 at 9:18 AM, radai <radai.rosenbl...@gmail.com> wrote: > @jun: > "Currently kafka-acl.sh just creates an ACL path in ZK with the principal > name string" - yes, but not directly. all it actually does it spin-up the > Authorizer and call Authorizer.addAcl() on it. > the vanilla Authorizer goes to ZK. > but generally speaking, users can plug in their own Authorizers (that can > store/load ACLs to/from wherever). > > it would be nice if users who customize Authorizers (and PrincipalBuilders) > did not immediately lose the ability to use kafka-acl.sh with their new > Authorizers. > > On Wed, Feb 15, 2017 at 5:50 AM, Manikumar <manikumar.re...@gmail.com> > wrote: > > > Sorry, I am late to this discussion. > > > > PrincipalBuilder is only used for SSL Protocol. > > For SASL, we use "sasl.kerberos.principal.to.local.rules" config to map > > SASL principal names to short names. To make it consistent, > > Do we also need to pass the SASL full principal name to authorizer ? > > We may need to use PrincipalBuilder for mapping SASL names. > > > > Related JIRA is here: > > https://issues.apache.org/jira/browse/KAFKA-2854 > > > > > > On Wed, Feb 15, 2017 at 7:47 AM, Jun Rao <j...@confluent.io> wrote: > > > > > Hi, Radai, > > > > > > Currently kafka-acl.sh just creates an ACL path in ZK with the > principal > > > name string. The authorizer module in the broker reads the principal > name > > > string from the acl path in ZK and creates the expected KafkaPrincipal > > for > > > matching. As you can see, the expected principal is created on the > broker > > > side, not by the kafka-acl.sh tool. The broker already has the ability > to > > > configure PrincipalBuilder. That's why I am not sure if there is a need > > for > > > kafka-acl.sh to customize PrincipalBuilder. > > > > > > Thanks, > > > > > > Jun > > > > > > > > > On Mon, Feb 13, 2017 at 7:01 PM, radai <radai.rosenbl...@gmail.com> > > wrote: > > > > > > > if i understand correctly, kafka-acls.sh spins up an instance of (the > > > > custom, in our case) Authorizer, and calls things like addAcls(acls: > > > > Set[Acl], resource: Resource) on it, which are defined in the > > interface, > > > > hence expected to be "extensible". > > > > > > > > (side note: if Authorizer and PrincipalBuilder are defined as > > extensible > > > > interfaces, why doesnt class Acl, which is in the signature for > > > Authorizer > > > > calls, use java.security.Principal?) > > > > > > > > we would like to be able to use the standard kafka-acl command line > for > > > > defining ACLs even when replacing the vanilla Authorizer and > > > > PrincipalBuilder (even though we have a management UI for these > > > operations > > > > within linkedin) - simply because thats the correct thing to do from > an > > > > extensibility point of view. > > > > > > > > On Mon, Feb 13, 2017 at 1:39 PM, Jun Rao <j...@confluent.io> wrote: > > > > > > > > > Hi, Mayuresh, > > > > > > > > > > I seems to me that there are two common use cases of authorizer. > (1) > > > Use > > > > > the default SimpleAuthorizer and the kafka-acl to do authorization. > > (2) > > > > Use > > > > > a customized authorizer and an external tool for authorization. Do > > you > > > > > think there is a use case for a customized authorizer and kafka-acl > > at > > > > the > > > > > same time? If not, it's better not to complicate the kafka-acl api. > > > > > > > > > > Thanks, > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > On Mon, Feb 13, 2017 at 10:35 AM, Mayuresh Gharat < > > > > > gharatmayures...@gmail.com> wrote: > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > Thanks for the review and comments. Please find the replies > inline > > : > > > > > > > > > > > > This is so that in the future, we can extend to types like group. > > > > > > ---> Yep, I did think the same. But since the SocketServer was > > always > > > > > > creating User type, it wasn't actually used. If we go ahead with > > > > changes > > > > > in > > > > > > this KIP, we will give this power of creating different Principal > > > types > > > > > to > > > > > > the PrincipalBuilder (which users can define there own). In that > > way > > > > > Kafka > > > > > > will not have to deal with handling this. So the Principal > building > > > and > > > > > > Authorization will be opaque to Kafka which seems like an > expected > > > > > > behavior. > > > > > > > > > > > > > > > > > > Hmm, normally, the configurations you specify for plug-ins refer > to > > > > those > > > > > > needed to construct the plug-in object. So, it's kind of weird to > > use > > > > > that > > > > > > to call a method. For example, why can't > > > principalBuilderService.rest. > > > > > url > > > > > > be passed in through the configure() method and the > implementation > > > can > > > > > use > > > > > > that to build principal. This way, there is only a single method > to > > > > > compute > > > > > > the principal in a consistent way in the broker and in the > > kafka-acl > > > > > tool. > > > > > > ----> We can do that as well. But since the rest url is not > related > > > to > > > > > the > > > > > > Principal, it seems out of place to me to pass it every time we > > have > > > to > > > > > > create a Principal. I should replace "principalConfigs" with > > > > > > "principalProperties". > > > > > > I was trying to differentiate the configs/properties that are > used > > to > > > > > > create the PrincipalBuilder class and the Principal/Principals > > > itself. > > > > > > > > > > > > > > > > > > For LinkedIn's use case, do you actually use the kafka-acl tool? > My > > > > > > understanding is that LinkedIn does authorization through an > > external > > > > > tool. > > > > > > ----> For Linkedin's use case we don't actually use the kafka-acl > > > tool > > > > > > right now. As per the discussion that we had on > > > > > > https://issues.apache.org/jira/browse/KAFKA-4454, we thought > that > > it > > > > > would > > > > > > be good to make kafka-acl tool changes, to make it flexible and > we > > > > might > > > > > be > > > > > > even able to use it in future. > > > > > > > > > > > > It seems it's simpler if kafka-acl doesn't to need to understand > > the > > > > > > principal builder. The tool does authorization based on a string > > > name, > > > > > > which is expected to match the principal name. So, I am wondering > > why > > > > the > > > > > > tool needs to know the principal builder. > > > > > > ----> If we don't make this change, I am not sure how clients/end > > > users > > > > > > will be able to use this tool if they have there own Authorizer > > that > > > > does > > > > > > Authorization based on Principal, that has more information apart > > > from > > > > > name > > > > > > and type. > > > > > > > > > > > > What if we only make the following changes: pass the java > principal > > > in > > > > > > session and in > > > > > > SimpleAuthorizer, construct KafkaPrincipal from java principal > > name. > > > > Will > > > > > > that work for LinkedIn? > > > > > > ----> This can work for Linkedin but as explained above, it does > > not > > > > seem > > > > > > like a complete design from open source point of view. > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > > > On Thu, Feb 9, 2017 at 11:29 AM, Jun Rao <j...@confluent.io> > wrote: > > > > > > > > > > > > > Hi, Mayuresh, > > > > > > > > > > > > > > Thanks for the reply. A few more comments below. > > > > > > > > > > > > > > On Wed, Feb 8, 2017 at 9:14 PM, Mayuresh Gharat < > > > > > > > gharatmayures...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > Thanks for the review. Please find the responses inline. > > > > > > > > > > > > > > > > 1. It seems the problem that you are trying to address is > that > > > java > > > > > > > > principal returned from KafkaChannel may have additional > fields > > > > than > > > > > > name > > > > > > > > that are needed during authorization. Have you considered a > > > > > customized > > > > > > > > PrincipleBuilder that extracts all needed fields from java > > > > principal > > > > > > and > > > > > > > > squeezes them as a json in the name of the returned > principal? > > > > Then, > > > > > > the > > > > > > > > authorizer can just parse the json and extract needed fields. > > > > > > > > ---> Yes we had thought about this. We use a third party > > library > > > > that > > > > > > > takes > > > > > > > > in the passed in cert and creates the Principal. This > Principal > > > is > > > > > then > > > > > > > > used by the library to make the decision (ALLOW/DENY) when we > > > call > > > > it > > > > > > in > > > > > > > > the Authorizer. It does not have an API to create the > Principal > > > > from > > > > > a > > > > > > > > String. If it did support, still we would have to be aware of > > the > > > > > > > internal > > > > > > > > details of the library, like the field values it creates from > > the > > > > > > certs, > > > > > > > > defaults and so on. > > > > > > > > > > > > > > > > 2. Could you explain how the default authorizer works now? > > > > Currently, > > > > > > the > > > > > > > > code just compares the two principal objects. Are we > converting > > > the > > > > > > java > > > > > > > > principal to a KafkaPrincipal there? > > > > > > > > ---> The SimpleAclAuthorizer currently expects that, the > > > Principal > > > > it > > > > > > > > fetches from the Session object is an instance of > > KafkaPrincipal. > > > > It > > > > > > then > > > > > > > > uses it compare with the KafkaPrincipal extracted from the > > stored > > > > > ACLs. > > > > > > > In > > > > > > > > this case, we can construct the KafkaPrincipal object on the > > fly > > > by > > > > > > using > > > > > > > > the name of the Principal as follows : > > > > > > > > > > > > > > > > *val principal = session.principal* > > > > > > > > *val kafkaPrincipal = new KafkaPrincipal(KafkaPrincipal. > > > USER_TYPE, > > > > > > > > principal.getName)* > > > > > > > > I was also planning to get rid of the principalType field in > > > > > > > > KafkaPrincipal as > > > > > > > > it is always set to *"*User*"* in the SocketServer currently. > > > After > > > > > > this > > > > > > > > KIP, it will no longer be used in SocketServer. But to > maintain > > > > > > backwards > > > > > > > > compatibility of kafka-acls.sh, I preserved it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This is so that in the future, we can extend to types like > group. > > > > > > > > > > > > > > > > > > > > > > 3. Do we need to add the following method in > PrincipalBuilder? > > > The > > > > > > > configs > > > > > > > > are already passed in through configure() and an > implementation > > > can > > > > > > cache > > > > > > > > it and use it in buildPrincipal(). It's also not clear to me > > > where > > > > we > > > > > > > call > > > > > > > > the new and the old method, and whether both will be called > or > > > one > > > > of > > > > > > > them > > > > > > > > will be called. > > > > > > > > Principal buildPrincipal(Map<String, ?> principalConfigs); > > > > > > > > ---> My thought was that the configure() method will be used > to > > > > build > > > > > > the > > > > > > > > PrincipalBuilder class object itself. It follows the same way > > as > > > > > > > Authorizer > > > > > > > > gets configured. The buildPrincipal(Map<String, ?> > > > > principalConfigs) > > > > > > will > > > > > > > > be used to build individual principals. > > > > > > > > Let me give an example, with the kafka-acls.sh : > > > > > > > > > > > > > > > > - bin/kafka-acls.sh --principalBuilder > > > > > > > > userDefinedPackage.kafka.security.PrincipalBuilder > > > > > > > > --principalBuilder-properties > > > > > > > > principalBuilderService.rest.url=URL --authorizer > > > > > > > > kafka.security.auth.SimpleAclAuthorizer > > > --authorizer-properties > > > > > > > > zookeeper.connect=localhost:2181 --add --allow-principal > > > > name=bob > > > > > > > > type=USER_PRINCIPAL --allow-principal > > name=ALPHA-GAMMA-SERVICE > > > > > > > > type=SERVICE_PRINCIPAL --allow-hosts Host1,Host2 > > --operations > > > > > > > Read,Write > > > > > > > > --topic Test-topic > > > > > > > > 1. *userDefinedPackage.kafka. > security.PrincipalBuilder* > > is > > > > the > > > > > > > user > > > > > > > > defined PrincipalBuilder class. > > > > > > > > 2. *principalBuilderService.rest.url=URL* can be a > > remote > > > > > > service > > > > > > > > that provides you an HTTP endpoint which takes in a set > > of > > > > > > > > parameters and > > > > > > > > provides you with the Principal. > > > > > > > > 3. *name=bob type=USER_PRINCIPAL* can be used by > > > > > PrincipalBuilder > > > > > > > to > > > > > > > > create UserPrincipal with name as bob > > > > > > > > 4. *name=ALPHA-GAMMA-SERVICE type=SERVICE_PRINCIPAL > *can > > be > > > > > used > > > > > > by > > > > > > > > PrincipalBuilder to create a ServicePrincipal with name > > as > > > > > > > > ALPHA-GAMMA-SERVICE. > > > > > > > > - This seems more flexible and intuitive to me from end > > user's > > > > > > > > perspective. > > > > > > > > > > > > > > > > > > > > > > > Hmm, normally, the configurations you specify for plug-ins > refer > > to > > > > > those > > > > > > > needed to construct the plug-in object. So, it's kind of weird > to > > > use > > > > > > that > > > > > > > to call a method. For example, why can't > > > > principalBuilderService.rest. > > > > > > url > > > > > > > be passed in through the configure() method and the > > implementation > > > > can > > > > > > use > > > > > > > that to build principal. This way, there is only a single > method > > to > > > > > > compute > > > > > > > the principal in a consistent way in the broker and in the > > > kafka-acl > > > > > > tool. > > > > > > > > > > > > > > For LinkedIn's use case, do you actually use the kafka-acl > tool? > > My > > > > > > > understanding is that LinkedIn does authorization through an > > > external > > > > > > tool. > > > > > > > It seems it's simpler if kafka-acl doesn't to need to > understand > > > the > > > > > > > principal builder. The tool does authorization based on a > string > > > > name, > > > > > > > which is expected to match the principal name. So, I am > wondering > > > why > > > > > the > > > > > > > tool needs to know the principal builder. What if we only make > > the > > > > > > > following changes: pass the java principal in session and in > > > > > > > SimpleAuthorizer, construct KafkaPrincipal from java principal > > > name. > > > > > Will > > > > > > > that work for LinkedIn? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Principal buildPrincipal(Map<String, ?> principalConfigs) > will > > be > > > > > > called > > > > > > > > from the commandline client kafka-acls.sh while the other API > > can > > > > be > > > > > > > called > > > > > > > > at runtime when Kafka receives a client request over request > > > > channel. > > > > > > > > > > > > > > > > 4. The KIP has "If users use there custom PrincipalBuilder, > > they > > > > will > > > > > > > have > > > > > > > > to implement there custom Authorizer as the out of box > > Authorizer > > > > > that > > > > > > > > Kafka provides uses KafkaPrincipal." This is not ideal for > > > existing > > > > > > > users. > > > > > > > > Could we avoid that? > > > > > > > > ---> Yes, this is possible to avoid if we do point 2. > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > On Wed, Feb 8, 2017 at 3:31 PM, Jun Rao <j...@confluent.io> > > > wrote: > > > > > > > > > > > > > > > > > Hi, Mayuresh, > > > > > > > > > > > > > > > > > > Thanks for the KIP. A few comments below. > > > > > > > > > > > > > > > > > > 1. It seems the problem that you are trying to address is > > that > > > > java > > > > > > > > > principal returned from KafkaChannel may have additional > > fields > > > > > than > > > > > > > name > > > > > > > > > that are needed during authorization. Have you considered a > > > > > > customized > > > > > > > > > PrincipleBuilder that extracts all needed fields from java > > > > > principal > > > > > > > and > > > > > > > > > squeezes them as a json in the name of the returned > > principal? > > > > > Then, > > > > > > > the > > > > > > > > > authorizer can just parse the json and extract needed > fields. > > > > > > > > > > > > > > > > > > 2. Could you explain how the default authorizer works now? > > > > > Currently, > > > > > > > the > > > > > > > > > code just compares the two principal objects. Are we > > converting > > > > the > > > > > > > java > > > > > > > > > principal to a KafkaPrincipal there? > > > > > > > > > > > > > > > > > > 3. Do we need to add the following method in > > PrincipalBuilder? > > > > The > > > > > > > > configs > > > > > > > > > are already passed in through configure() and an > > implementation > > > > can > > > > > > > cache > > > > > > > > > it and use it in buildPrincipal(). It's also not clear to > me > > > > where > > > > > we > > > > > > > > call > > > > > > > > > the new and the old method, and whether both will be called > > or > > > > one > > > > > of > > > > > > > > them > > > > > > > > > will be called. > > > > > > > > > Principal buildPrincipal(Map<String, ?> principalConfigs); > > > > > > > > > > > > > > > > > > 4. The KIP has "If users use there custom PrincipalBuilder, > > > they > > > > > will > > > > > > > > have > > > > > > > > > to implement there custom Authorizer as the out of box > > > Authorizer > > > > > > that > > > > > > > > > Kafka provides uses KafkaPrincipal." This is not ideal for > > > > existing > > > > > > > > users. > > > > > > > > > Could we avoid that? > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Feb 3, 2017 at 11:25 AM, Mayuresh Gharat < > > > > > > > > > gharatmayures...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi All, > > > > > > > > > > > > > > > > > > > > It seems that there is no further concern with the > KIP-111. > > > At > > > > > this > > > > > > > > point > > > > > > > > > > we would like to start the voting process. The KIP can be > > > found > > > > > at > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage. > > > > > > > > > action?pageId=67638388 > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > -- > > > > > > > > -Regards, > > > > > > > > Mayuresh R. Gharat > > > > > > > > (862) 250-7125 > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > -Regards, > > > > > > Mayuresh R. Gharat > > > > > > (862) 250-7125 > > > > > > > > > > > > > > > > > > > > > -- -Regards, Mayuresh R. Gharat (862) 250-7125
