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

Reply via email to