Jason, thanks for the clarification.
Bosco
On 8/25/17, 4:59 PM, "Jason Gustafson" <ja...@confluent.io> wrote:
Hey Don,
That is not actually part of the KIP. It was a (somewhat pedantic) example
used to illustrate how the kafka principal semantics could be applied to
authorizers which understood group-level ACLs. The key point is this:
although a principal is identified only by its type and name, the
KafkaPrincipal can be used to represent relations to other principals. In
this case, we have a user principal which is related to a group principal
through the UserPrincipalAndGroup object. A GroupAuthorizer could then
leverage this relation. As you suggest, a true implementation would allow
multiple groups.
I will add a note to the KIP to emphasize that this is just an example.
Thanks,
Jason
On Fri, Aug 25, 2017 at 4:37 PM, Don Bosco Durai <bo...@apache.org> wrote:
> Jason, thanks for confirming that. Since there are existing custom
> plugins, we might have to give enough time for them to start using the
> newer interface.
>
> I quickly glanced over the KIP, it looks good. Here is one comment:
>
> -------
> In the future, we may add support for groups to Kafka. This was brought up
> in the KIP-111 discussion. To support this, we can provide a groupId()
> method in KafkaPrincipal which defaults to a null value or an empty
string.
> Extensions can override this just as before. Also note that it is still
> possible for the Authorizer implementation to derive its own group
> information for enforcement.
> class UserPrincipalAndGroup extends KafkaPrincipal {
> private final String userId;
> private final String groupId;
> -------
>
> We should assume that users might belong to multiple groups.
>
> Also, not sure what the below method is really doing?
> ---
> class UserPrincipalAndGroup extends KafkaPrincipal {
>
> public KafkaPrincipal group() {
> return new KafkaPrincipal(KafkaPrincipal.GROUP_TYPE, groupId);
> }
> ---
> Thanks
>
> Bosco
>
>
>
> On 8/25/17, 4:11 PM, "Jason Gustafson" <ja...@confluent.io> wrote:
>
> 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
> > >
> >
> >
> >
> >
>
>
>
>
