You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Mayuresh Gharat <gh...@gmail.com> on 2016/05/21 00:00:44 UTC
KAFKA-3722 : Discussion about custom PrincipalBuilder and Authorizer configs
Hi All,
I came across an issue with plugging in a custom PrincipalBuilder class
using the config "principal.builder.class" along with a custom Authorizer
class using the config "authorizer.class.name".
Consider the following scenario :
For PlainText we don't supply any PrincipalBuilder. For SSL we want to
supply a PrincipalBuilder using the property "principal.builder.class".
a) Now consider we have a broker running on these 2 ports and supply that
custom principalBuilder class using that config.
b) The interbroker communication is using PlainText. I am using a single
broker cluster for testing.
c) Now we issue a produce request on the SSL port of the broker.
d) The controller tries to build a channel for plaintext with this broker
for the new topic instructions.
e) PlainText tries to use the principal builder specified in the
"principal.builder.class" config which was meant only for SSL port since
the code path is same "ChannelBuilders.createPrincipalBuilder(configs)".
f) In the custom principal Builder if we are trying to do some cert checks
or down conversion of transportLayer to SSLTransportLayer so that we can
use its functionality we get error/exception at runtime.
The basic idea is the PlainText channel should not be using the
PrincipalBuilder meant for other types of channels.
Now there are few options/workarounds to avoid this :
1) Do instanceOf check in Authorizer.authorize() on TransportLayer instance
passed in and do the correct handling. This is not intuitive and imposes a
strict coding rule on the programmer.
2) TransportLayer should expose an API for telling the security protocol
type. This is not too intuitive either.
3) Add extra configs for Authorizer and PrincipalBuilder for each channel
type. This gives us a flexibility for the PrincipalBuilder and Authorizer
handle requests on different types of ports in a different way.
4) PrincipalBuilder.buildPrincipal() should take in extra parameter for the
type of protocol and we should document this in javadoc to use it to handle
the type of request. This is little better than 1) and 2) but again imposes
a strict coding rule on the programmer.
Just wanted to know what the community thinks about this and get any
suggestions/feedback . There's some discussion about this here :
https://github.com/apache/kafka/pull/1403
Thanks,
Mayuresh
Re: KAFKA-3722 : Discussion about custom PrincipalBuilder and
Authorizer configs
Posted by Mayuresh Gharat <gh...@gmail.com>.
Cool. I will upload a patch for this.
Thanks,
Mayuresh
On Fri, May 27, 2016 at 12:35 AM, Harsha <ka...@harsha.io> wrote:
> Mayuresh & Ismael,
> Agree on not breaking interfaces on public API.
> +1 on option 2.
> Thanks,
> Harsha
>
> On Mon, May 23, 2016, at 10:30 AM, Mayuresh Gharat wrote:
> > Hi Harsha and Ismael,
> >
> > Option 2 sounds like a good idea if we want to make this quick fix I
> > think.
> > Option 4 might require a KIP as its public interface change. I can
> > resubmit
> > a patch for option 2 or create a KIP if necessary for option 4.
> >
> > From the previous conversation here, I think Ismael prefers option 2.
> > I don't have a strong opinion here since I understand its not easy to
> > make
> > public API changes but IMO, would go with option 4.
> >
> > Harsha what do you think on this?
> >
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Mon, May 23, 2016 at 5:45 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Hi Mayuresh and Harsha,
> > >
> > > If we were doing this from scratch, I would prefer option 4 too.
> However,
> > > users have their own custom principal builders now and option 2 with a
> > > suitably updated javadoc is the way to go in my opinion.
> > >
> > > Ismael
> > >
> > > On Sat, May 21, 2016 at 2:28 AM, Harsha <ka...@harsha.io> wrote:
> > >
> > > > Mayuresh,
> > > > Thanks for the write up. With principal builder,
> > > > the idea is to reuse a single principal builder
> > > > across all the security protocols where its
> > > > applicable and given that principal builder has
> > > > access to transportLayer and authenticator it
> > > > should be able to figure out what type of
> > > > transportLayer it is and it should be able
> > > > construct the principal based on that and it
> should
> > > > handle all the security protocols that we
> support.
> > > > In your options 1,2 & 4 seems to be doing the
> same
> > > > thing i.e checking what security protocol that a
> > > > given transportLayer is and building a principal
> ,
> > > > correct me if I am wrong here. I like going
> with 4
> > > > as others stated on PR . As passing
> > > > security_protocol makes it more specific to the
> > > > method that its need to be handled . In the
> interest
> > > > of having less config I think option 4 seems to
> be
> > > > better even though it breaks the interface.
> > > >
> > > > Thanks,
> > > > Harsha
> > > > On Fri, May 20, 2016, at 05:00 PM, Mayuresh Gharat wrote:
> > > > > Hi All,
> > > > >
> > > > > I came across an issue with plugging in a custom PrincipalBuilder
> class
> > > > > using the config "principal.builder.class" along with a custom
> > > Authorizer
> > > > > class using the config "authorizer.class.name".
> > > > >
> > > > > Consider the following scenario :
> > > > >
> > > > > For PlainText we don't supply any PrincipalBuilder. For SSL we
> want to
> > > > > supply a PrincipalBuilder using the property
> "principal.builder.class".
> > > > >
> > > > > a) Now consider we have a broker running on these 2 ports and
> supply
> > > that
> > > > > custom principalBuilder class using that config.
> > > > >
> > > > > b) The interbroker communication is using PlainText. I am using a
> > > single
> > > > > broker cluster for testing.
> > > > >
> > > > > c) Now we issue a produce request on the SSL port of the broker.
> > > > >
> > > > > d) The controller tries to build a channel for plaintext with this
> > > broker
> > > > > for the new topic instructions.
> > > > >
> > > > > e) PlainText tries to use the principal builder specified in the
> > > > > "principal.builder.class" config which was meant only for SSL port
> > > since
> > > > > the code path is same
> > > "ChannelBuilders.createPrincipalBuilder(configs)".
> > > > >
> > > > > f) In the custom principal Builder if we are trying to do some cert
> > > > > checks
> > > > > or down conversion of transportLayer to SSLTransportLayer so that
> we
> > > can
> > > > > use its functionality we get error/exception at runtime.
> > > > >
> > > > > The basic idea is the PlainText channel should not be using the
> > > > > PrincipalBuilder meant for other types of channels.
> > > > >
> > > > > Now there are few options/workarounds to avoid this :
> > > > >
> > > > > 1) Do instanceOf check in Authorizer.authorize() on TransportLayer
> > > > > instance
> > > > > passed in and do the correct handling. This is not intuitive and
> > > imposes
> > > > > a
> > > > > strict coding rule on the programmer.
> > > > >
> > > > > 2) TransportLayer should expose an API for telling the security
> > > protocol
> > > > > type. This is not too intuitive either.
> > > > >
> > > > > 3) Add extra configs for Authorizer and PrincipalBuilder for each
> > > channel
> > > > > type. This gives us a flexibility for the PrincipalBuilder and
> > > Authorizer
> > > > > handle requests on different types of ports in a different way.
> > > > >
> > > > > 4) PrincipalBuilder.buildPrincipal() should take in extra
> parameter for
> > > > > the
> > > > > type of protocol and we should document this in javadoc to use it
> to
> > > > > handle
> > > > > the type of request. This is little better than 1) and 2) but again
> > > > > imposes
> > > > > a strict coding rule on the programmer.
> > > > >
> > > > > Just wanted to know what the community thinks about this and get
> any
> > > > > suggestions/feedback . There's some discussion about this here :
> > > > > https://github.com/apache/kafka/pull/1403
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Mayuresh
> > > >
> > >
> >
> >
> >
> > --
> > -Regards,
> > Mayuresh R. Gharat
> > (862) 250-7125
>
--
-Regards,
Mayuresh R. Gharat
(862) 250-7125
Re: KAFKA-3722 : Discussion about custom PrincipalBuilder and Authorizer
configs
Posted by Harsha <ka...@harsha.io>.
Mayuresh & Ismael,
Agree on not breaking interfaces on public API.
+1 on option 2.
Thanks,
Harsha
On Mon, May 23, 2016, at 10:30 AM, Mayuresh Gharat wrote:
> Hi Harsha and Ismael,
>
> Option 2 sounds like a good idea if we want to make this quick fix I
> think.
> Option 4 might require a KIP as its public interface change. I can
> resubmit
> a patch for option 2 or create a KIP if necessary for option 4.
>
> From the previous conversation here, I think Ismael prefers option 2.
> I don't have a strong opinion here since I understand its not easy to
> make
> public API changes but IMO, would go with option 4.
>
> Harsha what do you think on this?
>
>
> Thanks,
>
> Mayuresh
>
> On Mon, May 23, 2016 at 5:45 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Mayuresh and Harsha,
> >
> > If we were doing this from scratch, I would prefer option 4 too. However,
> > users have their own custom principal builders now and option 2 with a
> > suitably updated javadoc is the way to go in my opinion.
> >
> > Ismael
> >
> > On Sat, May 21, 2016 at 2:28 AM, Harsha <ka...@harsha.io> wrote:
> >
> > > Mayuresh,
> > > Thanks for the write up. With principal builder,
> > > the idea is to reuse a single principal builder
> > > across all the security protocols where its
> > > applicable and given that principal builder has
> > > access to transportLayer and authenticator it
> > > should be able to figure out what type of
> > > transportLayer it is and it should be able
> > > construct the principal based on that and it should
> > > handle all the security protocols that we support.
> > > In your options 1,2 & 4 seems to be doing the same
> > > thing i.e checking what security protocol that a
> > > given transportLayer is and building a principal ,
> > > correct me if I am wrong here. I like going with 4
> > > as others stated on PR . As passing
> > > security_protocol makes it more specific to the
> > > method that its need to be handled . In the interest
> > > of having less config I think option 4 seems to be
> > > better even though it breaks the interface.
> > >
> > > Thanks,
> > > Harsha
> > > On Fri, May 20, 2016, at 05:00 PM, Mayuresh Gharat wrote:
> > > > Hi All,
> > > >
> > > > I came across an issue with plugging in a custom PrincipalBuilder class
> > > > using the config "principal.builder.class" along with a custom
> > Authorizer
> > > > class using the config "authorizer.class.name".
> > > >
> > > > Consider the following scenario :
> > > >
> > > > For PlainText we don't supply any PrincipalBuilder. For SSL we want to
> > > > supply a PrincipalBuilder using the property "principal.builder.class".
> > > >
> > > > a) Now consider we have a broker running on these 2 ports and supply
> > that
> > > > custom principalBuilder class using that config.
> > > >
> > > > b) The interbroker communication is using PlainText. I am using a
> > single
> > > > broker cluster for testing.
> > > >
> > > > c) Now we issue a produce request on the SSL port of the broker.
> > > >
> > > > d) The controller tries to build a channel for plaintext with this
> > broker
> > > > for the new topic instructions.
> > > >
> > > > e) PlainText tries to use the principal builder specified in the
> > > > "principal.builder.class" config which was meant only for SSL port
> > since
> > > > the code path is same
> > "ChannelBuilders.createPrincipalBuilder(configs)".
> > > >
> > > > f) In the custom principal Builder if we are trying to do some cert
> > > > checks
> > > > or down conversion of transportLayer to SSLTransportLayer so that we
> > can
> > > > use its functionality we get error/exception at runtime.
> > > >
> > > > The basic idea is the PlainText channel should not be using the
> > > > PrincipalBuilder meant for other types of channels.
> > > >
> > > > Now there are few options/workarounds to avoid this :
> > > >
> > > > 1) Do instanceOf check in Authorizer.authorize() on TransportLayer
> > > > instance
> > > > passed in and do the correct handling. This is not intuitive and
> > imposes
> > > > a
> > > > strict coding rule on the programmer.
> > > >
> > > > 2) TransportLayer should expose an API for telling the security
> > protocol
> > > > type. This is not too intuitive either.
> > > >
> > > > 3) Add extra configs for Authorizer and PrincipalBuilder for each
> > channel
> > > > type. This gives us a flexibility for the PrincipalBuilder and
> > Authorizer
> > > > handle requests on different types of ports in a different way.
> > > >
> > > > 4) PrincipalBuilder.buildPrincipal() should take in extra parameter for
> > > > the
> > > > type of protocol and we should document this in javadoc to use it to
> > > > handle
> > > > the type of request. This is little better than 1) and 2) but again
> > > > imposes
> > > > a strict coding rule on the programmer.
> > > >
> > > > Just wanted to know what the community thinks about this and get any
> > > > suggestions/feedback . There's some discussion about this here :
> > > > https://github.com/apache/kafka/pull/1403
> > > >
> > > > Thanks,
> > > >
> > > > Mayuresh
> > >
> >
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
Re: KAFKA-3722 : Discussion about custom PrincipalBuilder and
Authorizer configs
Posted by Mayuresh Gharat <gh...@gmail.com>.
Hi Harsha and Ismael,
Option 2 sounds like a good idea if we want to make this quick fix I think.
Option 4 might require a KIP as its public interface change. I can resubmit
a patch for option 2 or create a KIP if necessary for option 4.
From the previous conversation here, I think Ismael prefers option 2.
I don't have a strong opinion here since I understand its not easy to make
public API changes but IMO, would go with option 4.
Harsha what do you think on this?
Thanks,
Mayuresh
On Mon, May 23, 2016 at 5:45 AM, Ismael Juma <is...@juma.me.uk> wrote:
> Hi Mayuresh and Harsha,
>
> If we were doing this from scratch, I would prefer option 4 too. However,
> users have their own custom principal builders now and option 2 with a
> suitably updated javadoc is the way to go in my opinion.
>
> Ismael
>
> On Sat, May 21, 2016 at 2:28 AM, Harsha <ka...@harsha.io> wrote:
>
> > Mayuresh,
> > Thanks for the write up. With principal builder,
> > the idea is to reuse a single principal builder
> > across all the security protocols where its
> > applicable and given that principal builder has
> > access to transportLayer and authenticator it
> > should be able to figure out what type of
> > transportLayer it is and it should be able
> > construct the principal based on that and it should
> > handle all the security protocols that we support.
> > In your options 1,2 & 4 seems to be doing the same
> > thing i.e checking what security protocol that a
> > given transportLayer is and building a principal ,
> > correct me if I am wrong here. I like going with 4
> > as others stated on PR . As passing
> > security_protocol makes it more specific to the
> > method that its need to be handled . In the interest
> > of having less config I think option 4 seems to be
> > better even though it breaks the interface.
> >
> > Thanks,
> > Harsha
> > On Fri, May 20, 2016, at 05:00 PM, Mayuresh Gharat wrote:
> > > Hi All,
> > >
> > > I came across an issue with plugging in a custom PrincipalBuilder class
> > > using the config "principal.builder.class" along with a custom
> Authorizer
> > > class using the config "authorizer.class.name".
> > >
> > > Consider the following scenario :
> > >
> > > For PlainText we don't supply any PrincipalBuilder. For SSL we want to
> > > supply a PrincipalBuilder using the property "principal.builder.class".
> > >
> > > a) Now consider we have a broker running on these 2 ports and supply
> that
> > > custom principalBuilder class using that config.
> > >
> > > b) The interbroker communication is using PlainText. I am using a
> single
> > > broker cluster for testing.
> > >
> > > c) Now we issue a produce request on the SSL port of the broker.
> > >
> > > d) The controller tries to build a channel for plaintext with this
> broker
> > > for the new topic instructions.
> > >
> > > e) PlainText tries to use the principal builder specified in the
> > > "principal.builder.class" config which was meant only for SSL port
> since
> > > the code path is same
> "ChannelBuilders.createPrincipalBuilder(configs)".
> > >
> > > f) In the custom principal Builder if we are trying to do some cert
> > > checks
> > > or down conversion of transportLayer to SSLTransportLayer so that we
> can
> > > use its functionality we get error/exception at runtime.
> > >
> > > The basic idea is the PlainText channel should not be using the
> > > PrincipalBuilder meant for other types of channels.
> > >
> > > Now there are few options/workarounds to avoid this :
> > >
> > > 1) Do instanceOf check in Authorizer.authorize() on TransportLayer
> > > instance
> > > passed in and do the correct handling. This is not intuitive and
> imposes
> > > a
> > > strict coding rule on the programmer.
> > >
> > > 2) TransportLayer should expose an API for telling the security
> protocol
> > > type. This is not too intuitive either.
> > >
> > > 3) Add extra configs for Authorizer and PrincipalBuilder for each
> channel
> > > type. This gives us a flexibility for the PrincipalBuilder and
> Authorizer
> > > handle requests on different types of ports in a different way.
> > >
> > > 4) PrincipalBuilder.buildPrincipal() should take in extra parameter for
> > > the
> > > type of protocol and we should document this in javadoc to use it to
> > > handle
> > > the type of request. This is little better than 1) and 2) but again
> > > imposes
> > > a strict coding rule on the programmer.
> > >
> > > Just wanted to know what the community thinks about this and get any
> > > suggestions/feedback . There's some discussion about this here :
> > > https://github.com/apache/kafka/pull/1403
> > >
> > > Thanks,
> > >
> > > Mayuresh
> >
>
--
-Regards,
Mayuresh R. Gharat
(862) 250-7125
Re: KAFKA-3722 : Discussion about custom PrincipalBuilder and
Authorizer configs
Posted by Ismael Juma <is...@juma.me.uk>.
Hi Mayuresh and Harsha,
If we were doing this from scratch, I would prefer option 4 too. However,
users have their own custom principal builders now and option 2 with a
suitably updated javadoc is the way to go in my opinion.
Ismael
On Sat, May 21, 2016 at 2:28 AM, Harsha <ka...@harsha.io> wrote:
> Mayuresh,
> Thanks for the write up. With principal builder,
> the idea is to reuse a single principal builder
> across all the security protocols where its
> applicable and given that principal builder has
> access to transportLayer and authenticator it
> should be able to figure out what type of
> transportLayer it is and it should be able
> construct the principal based on that and it should
> handle all the security protocols that we support.
> In your options 1,2 & 4 seems to be doing the same
> thing i.e checking what security protocol that a
> given transportLayer is and building a principal ,
> correct me if I am wrong here. I like going with 4
> as others stated on PR . As passing
> security_protocol makes it more specific to the
> method that its need to be handled . In the interest
> of having less config I think option 4 seems to be
> better even though it breaks the interface.
>
> Thanks,
> Harsha
> On Fri, May 20, 2016, at 05:00 PM, Mayuresh Gharat wrote:
> > Hi All,
> >
> > I came across an issue with plugging in a custom PrincipalBuilder class
> > using the config "principal.builder.class" along with a custom Authorizer
> > class using the config "authorizer.class.name".
> >
> > Consider the following scenario :
> >
> > For PlainText we don't supply any PrincipalBuilder. For SSL we want to
> > supply a PrincipalBuilder using the property "principal.builder.class".
> >
> > a) Now consider we have a broker running on these 2 ports and supply that
> > custom principalBuilder class using that config.
> >
> > b) The interbroker communication is using PlainText. I am using a single
> > broker cluster for testing.
> >
> > c) Now we issue a produce request on the SSL port of the broker.
> >
> > d) The controller tries to build a channel for plaintext with this broker
> > for the new topic instructions.
> >
> > e) PlainText tries to use the principal builder specified in the
> > "principal.builder.class" config which was meant only for SSL port since
> > the code path is same "ChannelBuilders.createPrincipalBuilder(configs)".
> >
> > f) In the custom principal Builder if we are trying to do some cert
> > checks
> > or down conversion of transportLayer to SSLTransportLayer so that we can
> > use its functionality we get error/exception at runtime.
> >
> > The basic idea is the PlainText channel should not be using the
> > PrincipalBuilder meant for other types of channels.
> >
> > Now there are few options/workarounds to avoid this :
> >
> > 1) Do instanceOf check in Authorizer.authorize() on TransportLayer
> > instance
> > passed in and do the correct handling. This is not intuitive and imposes
> > a
> > strict coding rule on the programmer.
> >
> > 2) TransportLayer should expose an API for telling the security protocol
> > type. This is not too intuitive either.
> >
> > 3) Add extra configs for Authorizer and PrincipalBuilder for each channel
> > type. This gives us a flexibility for the PrincipalBuilder and Authorizer
> > handle requests on different types of ports in a different way.
> >
> > 4) PrincipalBuilder.buildPrincipal() should take in extra parameter for
> > the
> > type of protocol and we should document this in javadoc to use it to
> > handle
> > the type of request. This is little better than 1) and 2) but again
> > imposes
> > a strict coding rule on the programmer.
> >
> > Just wanted to know what the community thinks about this and get any
> > suggestions/feedback . There's some discussion about this here :
> > https://github.com/apache/kafka/pull/1403
> >
> > Thanks,
> >
> > Mayuresh
>
Re: KAFKA-3722 : Discussion about custom PrincipalBuilder and Authorizer
configs
Posted by Harsha <ka...@harsha.io>.
Mayuresh,
Thanks for the write up. With principal builder,
the idea is to reuse a single principal builder
across all the security protocols where its
applicable and given that principal builder has
access to transportLayer and authenticator it
should be able to figure out what type of
transportLayer it is and it should be able
construct the principal based on that and it should
handle all the security protocols that we support.
In your options 1,2 & 4 seems to be doing the same
thing i.e checking what security protocol that a
given transportLayer is and building a principal ,
correct me if I am wrong here. I like going with 4
as others stated on PR . As passing
security_protocol makes it more specific to the
method that its need to be handled . In the interest
of having less config I think option 4 seems to be
better even though it breaks the interface.
Thanks,
Harsha
On Fri, May 20, 2016, at 05:00 PM, Mayuresh Gharat wrote:
> Hi All,
>
> I came across an issue with plugging in a custom PrincipalBuilder class
> using the config "principal.builder.class" along with a custom Authorizer
> class using the config "authorizer.class.name".
>
> Consider the following scenario :
>
> For PlainText we don't supply any PrincipalBuilder. For SSL we want to
> supply a PrincipalBuilder using the property "principal.builder.class".
>
> a) Now consider we have a broker running on these 2 ports and supply that
> custom principalBuilder class using that config.
>
> b) The interbroker communication is using PlainText. I am using a single
> broker cluster for testing.
>
> c) Now we issue a produce request on the SSL port of the broker.
>
> d) The controller tries to build a channel for plaintext with this broker
> for the new topic instructions.
>
> e) PlainText tries to use the principal builder specified in the
> "principal.builder.class" config which was meant only for SSL port since
> the code path is same "ChannelBuilders.createPrincipalBuilder(configs)".
>
> f) In the custom principal Builder if we are trying to do some cert
> checks
> or down conversion of transportLayer to SSLTransportLayer so that we can
> use its functionality we get error/exception at runtime.
>
> The basic idea is the PlainText channel should not be using the
> PrincipalBuilder meant for other types of channels.
>
> Now there are few options/workarounds to avoid this :
>
> 1) Do instanceOf check in Authorizer.authorize() on TransportLayer
> instance
> passed in and do the correct handling. This is not intuitive and imposes
> a
> strict coding rule on the programmer.
>
> 2) TransportLayer should expose an API for telling the security protocol
> type. This is not too intuitive either.
>
> 3) Add extra configs for Authorizer and PrincipalBuilder for each channel
> type. This gives us a flexibility for the PrincipalBuilder and Authorizer
> handle requests on different types of ports in a different way.
>
> 4) PrincipalBuilder.buildPrincipal() should take in extra parameter for
> the
> type of protocol and we should document this in javadoc to use it to
> handle
> the type of request. This is little better than 1) and 2) but again
> imposes
> a strict coding rule on the programmer.
>
> Just wanted to know what the community thinks about this and get any
> suggestions/feedback . There's some discussion about this here :
> https://github.com/apache/kafka/pull/1403
>
> Thanks,
>
> Mayuresh