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
