Hi Don, I don't think so. We are not making any changes to the Authorizer interface itself. The KafkaPrincipal object does not change either, though we now explicitly allow it to be extended. That means you have to exercise a little caution when combining a custom PrincipalBuilder with a custom Authorizer. For the default principal builder shipped with Kafka, it will work the same as it currently does. Old implementations of PrincipalBuilder will also continue to work exactly as they do now, but please note that I am proposing to deprecate this interface. It will still be supported in 1.0.0, but we may remove it in a future major release.
-Jason On Fri, Aug 25, 2017 at 3:51 PM, Don Bosco Durai <bo...@apache.org> wrote: > Jason > > Do you anticipate any backward compatibility issues with existing custom > implementation of the authorization interface/plugins? > > Thanks > > Bosco > > > On 8/25/17, 3:22 PM, "Jason Gustafson" <ja...@confluent.io> wrote: > > No problem. I'll add a note to the KIP to emphasize that we will use > the > same object built by the KafkaPrincipalBuilder in the Session object > passed > to the authorizer. > > -Jason > > On Fri, Aug 25, 2017 at 3:17 PM, Mayuresh Gharat < > gharatmayures...@gmail.com > > wrote: > > > Perfect. > > As long as there is a way we can access the originally created > Principal in > > the Authorizer, it would solve the KIP-111 issue. > > > > This is really helpful, thanks again. > > > > Thanks, > > > > Mayuresh > > > > On Fri, Aug 25, 2017 at 3:13 PM, Jason Gustafson <ja...@confluent.io > > > > wrote: > > > > > Hi Mayuresh, > > > > > > To clarify, the intention is to use the KafkaPrincipal object > built by > > the > > > KafkaPrincipalBuilder inside the Session. So we would remove the > logic to > > > construct a new KafkaPrincipal using only the name from the > Principal. > > Then > > > it should be possible to pass the `AuthzPrincipal` to the > underlying > > > library through the `Extended_Plugged_In_Class` as you've suggested > > above. > > > Is that reasonable for this use case? > > > > > > Thanks, > > > Jason > > > > > > > > > On Fri, Aug 25, 2017 at 2:44 PM, Mayuresh Gharat < > > > gharatmayures...@gmail.com > > > > wrote: > > > > > > > Hi Jason, > > > > > > > > Thanks for the replies. > > > > > > > > I think it would be better to discuss with an example that we > were > > trying > > > > to address with KIP-111 and see if the current mentioned > solution would > > > > address it. > > > > > > > > Let's consider a third party library called authz_lib that is > provided > > by > > > > some Security team at some company. > > > > > > > > - When we call authz_lib.createPrincipal(X509_cert), it would > > return > > > an > > > > AuthzPrincipal that implements Java.Security.Principal. > > > > > > > > > > > > - The authz_lib also provides an checkAccess(....) call that > takes > > in > > > 3 > > > > parameters : > > > > - authz_principal > > > > - operation type ("Read", "Write"...) > > > > - resource (for simplicity lets consider it as a TopicName) > > > > > > > > > > > > - The AuthzPrincipal looks like this : > > > > > > > > class AuthzPrincipal implements java.security.Principal > > > > { > > > > String name; > > > > String field1; > > > > Object field2; > > > > Object field3; > > > > .....//Some third party logic...... > > > > } > > > > > > > > > > > > - In PrincipalBuilder.buildPrincipal() would return > AuthzPrincipal > > as > > > > follows : > > > > > > > > public Principal buildPrincipal(...) > > > > { > > > > ...... > > > > X509Certificate x509Cert = session.getCert(..); > > > > return authz_lib.createPrincipal(x509Cert); > > > > } > > > > > > > > > > > > - The custom Authorizer (lets call it CustomAuthzAuthorizer), > we > > would > > > > use the checkAccess() function provided by the authz_lib as > follows > > : > > > > > > > > public class CustomAuthzAuthorizer implements Authorizer > > > > { > > > > ......... > > > > public boolean authorize(.....) > > > > { > > > > AuthzPrincipal authz_principal = (AuthzPrincipal) > > > > session.getPrincipal(); > > > > return authz_lib.checkAccess(authz_principal, "Read", "topicX"); > > > > } > > > > .......... > > > > } > > > > > > > > > > > > - The issue with current implementation is that in > > > > processCompletedReceives() in SocketServer we create a > > KafkaPrincipal > > > > that just extracts the name from AuthzPrincipal as follows : > > > > > > > > session = RequestChannel.Session(new > > > > KafkaPrincipal(KafkaPrincipal.USER_TYPE, > > > > *openOrClosingChannel.principal.getName*), > > > > openOrClosingChannel.socketAddress) > > > > > > > > So the "AuthzPrincipal authz_principal = (AuthzPrincipal) > > > > session.getPrincipal()" call in the CustomAuthzAuthorizer would > error > > > > out because we are trying to cast a KafkaPrincipal to > AuthzPrincipal. > > > > > > > > > > > > > > > > In your reply when you said that : > > > > > > > > The KIP says that a user can have a class that extends > KafkaPrincipal. > > > > Would this extended class be used when constructing the Session > object > > > > in the SocketServer instead of constructing a new KafkaPrincipal? > > > > > > > > Yes, that's correct. We want to allow the authorizer to be able > to > > > leverage > > > > > additional information from the authentication layer. > > > > > > > > > > > > Would it make sense to make this extended class pluggable and > when > > > > constructing the Session object in SocketServer check if a > plugin is > > > > defined and use it and if not use the default KafkaPrincipal > something > > > like > > > > : > > > > > > > > if (getConfig("principal.pluggedIn.class").isDefined()) > > > > //"principal.pluggedIn.class" > > > > is just an example name for the config > > > > { > > > > session = RequestChannel.Session(*Extended_Plugged_In_Class*, > > > > openOrClosingChannel.socketAddress) > > > > } > > > > else > > > > { > > > > session = RequestChannel.Session(new > KafkaPrincipal(KafkaPrincipal. > > > > USER_TYPE, > > > > *openOrClosingChannel.principal.getName*), > > > > openOrClosingChannel.socketAddress) > > > > } > > > > > > > > This would solve the issue above as follows : > > > > > > > > We can have something like : > > > > public class Extended_Plugged_In_Class extends KafkaPrincipal > > > > { > > > > AuthzPrincipal authzPrincipal; > > > > > > > > public Extended_Plugged_In_Class(....., AuthzPrincipal > principal) > > > > { > > > > super(...); > > > > authzPrincipal = principal; > > > > > > > > } > > > > > > > > ...... > > > > > > > > public AuthzPrincipal getAuthzPrincipal() > > > > { > > > > return authzPrincipal; > > > > } > > > > } > > > > > > > > In the CustomAuthzAuthorizer we could do something like : > > > > > > > > public class CustomAuthzAuthorizer implements Authorizer > > > > { > > > > ......... > > > > public boolean authorize(.....) > > > > { > > > > Extended_Plugged_In_Class extended_Kafka_Principal = > > > > (Extended_Plugged_In_Class) > > > > session.getPrincipal(); > > > > AuthzPrincipal authz_principal = > > > > extended_Kafka_Principal.getAuthzPrincipal(); > > > > return authz_lib.checkAccess(authz_principal, "Read", "topicX"); > > > > } > > > > .......... > > > > } > > > > > > > > > > > > Thanks, > > > > > > > > Mayuresh > > > > > > > > On Fri, Aug 25, 2017 at 11:53 AM, Jason Gustafson < > ja...@confluent.io> > > > > wrote: > > > > > > > > > Hey Mayuresh, > > > > > > > > > > Thanks for the comments. > > > > > > > > > > - The KIP says that a user can have a class that extends > > > > KafkaPrincipal. > > > > > > Would this extended class be used when constructing the > Session > > > > object > > > > > > in > > > > > > the SocketServer instead of constructing a new > KafkaPrincipal? > > > > > > > > > > > > > > > Yes, that's correct. We want to allow the authorizer to be > able to > > > > leverage > > > > > additional information from the authentication layer. > > > > > > > > > > - The KIP says "A principal is always identifiable by a > principal > > > > type > > > > > > and a name. Nothing else should ever be required." This > might > > not > > > be > > > > > > true > > > > > > always, right? For example, we might have a custom third > party > > ACL > > > > > > library > > > > > > that creates a custom Principal from the passed in cert > (this is > > > > done > > > > > in > > > > > > PrincipalBuilder/KafkaPrincipalBuilder) and the custom > > Authorizer > > > > > might > > > > > > use this third party library to authorize using this > custom > > > > Principal > > > > > > object. The developer who is implementing the Kafka > Authorizer > > > > should > > > > > > not be caring about what the custom Principal would look > like > > and > > > > its > > > > > > details, since it will just pass it to the third party > library > > in > > > > > Kafka > > > > > > Authorizer's authorize() call. > > > > > > > > > > > > > > > I'm not sure I understand this. Are you saying that the > authorizer > > and > > > > > principal builder are implemented by separate individuals? If > the > > > > > authorizer doesn't understand how to identify the principal, > then it > > > > > wouldn't work, right? Maybe I'm missing something? > > > > > > > > > > Let me explain how I see this. The simple ACL authorizer that > Kafka > > > ships > > > > > with understands user principals as consisting of a type and a > name. > > > Any > > > > > principal builder that follows this assumption will work with > the > > > > > SimpleAclAuthorizer. In some cases, the principal builder may > provide > > > > > additional metadata in a KafkaPrincipal extension such as user > groups > > > or > > > > > roles. This information is not needed to identify the user > principal, > > > so > > > > > the builder is still compatible with the SimpleAclAuthorizer. > It > > would > > > > also > > > > > be compatible with a RoleBasedAuthorizer which understood how > to use > > > the > > > > > role metadata provided by the KafkaPrincipal extension. > Basically > > what > > > we > > > > > would have is a user principal which is related to one or more > role > > > > > principals through the KafkaPrincipal extension. Both user and > role > > > > > principals are identifiable with a type and a name, so the ACL > > command > > > > tool > > > > > can then be used (perhaps with a custom authorizer) to define > > > permissions > > > > > in either case. > > > > > > > > > > On the other hand, if a user principal is identified by more > than > > just > > > > its > > > > > name, then it is not compatible with the SimpleAclAuthorizer. > This > > > > doesn't > > > > > necessarily rule out this use case. As long as the authorizer > and the > > > > > principal builder both agree on how user principals are > identified, > > > then > > > > > they can still be used together. But I am explicitly leaving > out > > > support > > > > in > > > > > the ACL command tool for this use case in this KIP. This is > mostly > > > about > > > > > clarifying what is compatible with the authorization system > that > > Kafka > > > > > ships with. Of course we can always reconsider it in the > future. > > > > > > > > > > Thanks, > > > > > Jason > > > > > > > > > > On Fri, Aug 25, 2017 at 10:48 AM, Mayuresh Gharat < > > > > > gharatmayures...@gmail.com> wrote: > > > > > > > > > > > Hi Jason, > > > > > > > > > > > > Thanks a lot for the KIP and sorry for the delayed response. > > > > > > > > > > > > I had a few questions : > > > > > > > > > > > > > > > > > > - The KIP says that a user can have a class that extends > > > > > KafkaPrincipal. > > > > > > Would this extended class be used when constructing the > Session > > > > object > > > > > > in > > > > > > the SocketServer instead of constructing a new > KafkaPrincipal? > > > > > > > > > > > > > > > > > > - The KIP says "A principal is always identifiable by a > > principal > > > > type > > > > > > and a name. Nothing else should ever be required." This > might > > not > > > be > > > > > > true > > > > > > always, right? For example, we might have a custom third > party > > ACL > > > > > > library > > > > > > that creates a custom Principal from the passed in cert > (this is > > > > done > > > > > in > > > > > > PrincipalBuilder/KafkaPrincipalBuilder) and the custom > > Authorizer > > > > > might > > > > > > use this third party library to authorize using this > custom > > > > Principal > > > > > > object. The developer who is implementing the Kafka > Authorizer > > > > should > > > > > > not be caring about what the custom Principal would look > like > > and > > > > its > > > > > > details, since it will just pass it to the third party > library > > in > > > > > Kafka > > > > > > Authorizer's authorize() call. > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > > > On Thu, Aug 24, 2017 at 10:21 AM, Mayuresh Gharat < > > > > > > gharatmayures...@gmail.com> wrote: > > > > > > > > > > > > > Sure. > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > On Wed, Aug 23, 2017 at 5:07 PM, Jun Rao <j...@confluent.io > > > > wrote: > > > > > > > > > > > > > >> Hi, Mayuresh, > > > > > > >> > > > > > > >> Since this KIP covers the requirement in KIP-111, could > you > > review > > > > it > > > > > > too? > > > > > > >> > > > > > > >> Thanks, > > > > > > >> > > > > > > >> Jun > > > > > > >> > > > > > > >> > > > > > > >> On Tue, Aug 22, 2017 at 3:04 PM, Jason Gustafson < > > > > ja...@confluent.io> > > > > > > >> wrote: > > > > > > >> > > > > > > >>> Bump. I'll open a vote in a few days if there are no > comments. > > > > > > >>> > > > > > > >>> Thanks, > > > > > > >>> Jason > > > > > > >>> > > > > > > >>> On Sat, Aug 19, 2017 at 12:28 AM, Ismael Juma < > > ism...@juma.me.uk > > > > > > > > > > wrote: > > > > > > >>> > > > > > > >>> > Thanks for the KIP Jason. It seems reasonable and > cleans up > > > some > > > > > > >>> > inconsistencies in that area. It would be great to get > some > > > > > feedback > > > > > > >>> from > > > > > > >>> > Mayuresh and others who worked on KIP-111. > > > > > > >>> > > > > > > > >>> > Ismael > > > > > > >>> > > > > > > > >>> > On Thu, Aug 17, 2017 at 1:21 AM, Jason Gustafson < > > > > > ja...@confluent.io > > > > > > > > > > > > > >>> > wrote: > > > > > > >>> > > > > > > > >>> > > Hi All, > > > > > > >>> > > > > > > > > >>> > > I've added a new KIP to improve and extend the > principal > > > > building > > > > > > API > > > > > > >>> > that > > > > > > >>> > > Kafka exposes: > > > > > > >>> > > https://cwiki.apache.org/ > confluence/display/KAFKA/KIP- > > > > > > >>> > > 189%3A+Improve+principal+builder+interface+and+add+ > > > > > > support+for+SASL > > > > > > >>> > > . > > > > > > >>> > > > > > > > > >>> > > As always, feedback is appreciated. > > > > > > >>> > > > > > > > > >>> > > Thanks, > > > > > > >>> > > Jason > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > -Regards, > > > > > > > Mayuresh R. Gharat > > > > > > > (862) 250-7125 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > -Regards, > > > > > > Mayuresh R. Gharat > > > > > > (862) 250-7125 > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > -Regards, > > > > Mayuresh R. Gharat > > > > (862) 250-7125 > > > > > > > > > > > > > > > -- > > -Regards, > > Mayuresh R. Gharat > > (862) 250-7125 > > > > > >