You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Rajini Sivaram <ra...@gmail.com> on 2018/01/17 14:54:34 UTC
Re: [DISCUSS] KIP-86: Configurable SASL callback handlers
Hi all,
I have made some updates to this KIP to simplify addition of new SASL
mechanisms:
1. The Login interface has been made configurable as well (we have had
this interface for quite some time and it has been stable).
2. The callback handler properties for client-side and server-side have
been separated out since we would never use the same classes for both.
Any feedback or suggestions are welcome.
Thank you,
Rajini
On Mon, Apr 3, 2017 at 12:55 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> If there are no other concerns or suggestions on this KIP, I will start
> vote later this week.
>
> Thank you...
>
> Regards,
>
> Rajini
>
> On Thu, Mar 30, 2017 at 9:42 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
>> I have made a minor change to the callback handler interface to pass in
>> the JAAS configuration entries in *configure,* to work with the multiple
>> listener configuration introduced in KIP-103. I have also renamed the
>> interface to AuthenticateCallbackHandler instead of AuthCallbackHandler to
>> avoid confusion with authorization.
>>
>> I have rebased and updated the PR (https://github.com/apache/kaf
>> ka/pull/2022) as well. Please let me know if there are any other
>> comments or suggestions to move this forward.
>>
>> Thank you...
>>
>> Regards,
>>
>> Rajini
>>
>>
>> On Thu, Dec 15, 2016 at 3:11 PM, Rajini Sivaram <rs...@pivotal.io>
>> wrote:
>>
>>> Ismael,
>>>
>>> The reason for choosing CallbackHandler interface as the configurable
>>> interface is flexibility. As you say, we could instead define a simpler
>>> PlainCredentialProvider and ScramCredentialProvider. But that would tie
>>> users to Kafka's SaslServer implementation for PLAIN and SCRAM.
>>> SaslServer/SaslClient implementations are already pluggable using
>>> standard
>>> Java security provider mechanism. Callback handlers are the configuration
>>> mechanism for SaslServer/SaslClient. By making the handlers configurable,
>>> SASL becomes fully configurable for mechanisms supported by Kafka as well
>>> as custom mechanisms. From the 'Scenarios' section in the KIP, a simpler
>>> PLAIN/SCRAM interface satisfies the first two, but configurable callback
>>> handlers enables all five. I agree that most users don't require this
>>> level
>>> of flexibility, but we have had discussions about custom mechanisms in
>>> the
>>> past for integration with existing authentication servers. So I think it
>>> is
>>> a feature worth supporting.
>>>
>>> On Thu, Dec 15, 2016 at 2:21 PM, Ismael Juma <is...@juma.me.uk> wrote:
>>>
>>> > Thanks Rajini, your answers make sense to me. One more general point:
>>> we
>>> > are following the JAAS callback architecture and exposing that to the
>>> user
>>> > where the user has to write code like:
>>> >
>>> > @Override
>>> > public void handle(Callback[] callbacks) throws IOException,
>>> > UnsupportedCallbackException {
>>> > String username = null;
>>> > for (Callback callback: callbacks) {
>>> > if (callback instanceof NameCallback)
>>> > username = ((NameCallback) callback).getDefaultName();
>>> > else if (callback instanceof PlainAuthenticateCallback) {
>>> > PlainAuthenticateCallback plainCallback =
>>> > (PlainAuthenticateCallback) callback;
>>> > boolean authenticated = authenticate(username,
>>> > plainCallback.password());
>>> > plainCallback.authenticated(authenticated);
>>> > } else
>>> > throw new UnsupportedCallbackException(callback);
>>> > }
>>> > }
>>> >
>>> > protected boolean authenticate(String username, char[] password)
>>> throws
>>> > IOException {
>>> > if (username == null)
>>> > return false;
>>> > else {
>>> > String expectedPassword =
>>> > JaasUtils.jaasConfig(LoginType.SERVER.contextName(), "user_" +
>>> username,
>>> > PlainLoginModule.class.getName());
>>> > return Arrays.equals(password,
>>> expectedPassword.toCharArray()
>>> > );
>>> > }
>>> > }
>>> >
>>> > Since we need to create a new callback type for Plain, Scram and so
>>> on, is
>>> > it really worth it to do it this way? For example, in the code above,
>>> the
>>> > `authenticate` method could be the only thing the user has to
>>> implement and
>>> > we could do the necessary work to unwrap the data from the various
>>> > callbacks when interacting with the SASL API. More work for us, but a
>>> much
>>> > more pleasant API for users. What are the drawbacks?
>>> >
>>> > Ismael
>>> >
>>> > On Thu, Dec 15, 2016 at 1:06 AM, Rajini Sivaram <rs...@pivotal.io>
>>> > wrote:
>>> >
>>> > > Ismael,
>>> > >
>>> > > 1. At the moment AuthCallbackHandler is not a public interface, so I
>>> am
>>> > > assuming that it can be modified. Yes, agree that we should keep
>>> > non-public
>>> > > methods separate. Will do that as part of the implementation of this
>>> KIP.
>>> > >
>>> > > 2. Callback handlers do tend to depend on ordering, including those
>>> > > included in the JVM and these in Kafka. I have specified the
>>> ordering in
>>> > > the KIP. Will make sure they get included in documentation too.
>>> > >
>>> > > 3. Added a note to the KIP. Kafka needs access to the SCRAM
>>> credentials
>>> > to
>>> > > perform SCRAM authentication. For PLAIN, Kafka only needs to know if
>>> the
>>> > > password is valid for the user. We want to support external
>>> > authentication
>>> > > servers whose interface is to validate password, not retrieve it.
>>> > >
>>> > > 4. Added code of ScramCredential to the KIP.
>>> > >
>>> > >
>>> > > On Wed, Dec 14, 2016 at 3:54 PM, Ismael Juma <is...@juma.me.uk>
>>> wrote:
>>> > >
>>> > > > Thanks Rajini, that helps. A few comments:
>>> > > >
>>> > > > 1. The `AuthCallbackHandler` interface already exists and we are
>>> making
>>> > > > breaking changes (removing a parameter from `configure` and adding
>>> > > > additional methods). Is the reasoning that it was not a public
>>> > interface
>>> > > > before? It would be good to clearly separate public versus
>>> non-public
>>> > > > interfaces in the security code (and we should tweak Gradle to
>>> publish
>>> > > > javadoc for the public ones).
>>> > > >
>>> > > > 2. It seems like there is an ordering when it comes to the
>>> invocation
>>> > of
>>> > > > callbacks. At least the current code assumes that `NameCallback` is
>>> > > called
>>> > > > first. If I am interpreting this correctly, we should specify that
>>> > > > ordering.
>>> > > >
>>> > > > 3. The approach taken by `ScramCredentialCallback` is different
>>> than
>>> > the
>>> > > > one taken by `PlainAuthenticateCallback`. The former lets the user
>>> pass
>>> > > the
>>> > > > credentials information while the latter passes the credentials and
>>> > lets
>>> > > > the user do the authentication. It would be good to explain the
>>> > > > inconsistency.
>>> > > >
>>> > > > 4. We reference `ScramCredential` in a few places, so it would be
>>> good
>>> > to
>>> > > > define that class too.
>>> > > >
>>> > > > Ismael
>>> > > >
>>> > > > On Wed, Dec 14, 2016 at 7:32 AM, Rajini Sivaram <
>>> > > > rajinisivaram@googlemail.com> wrote:
>>> > > >
>>> > > > > Have added sample callback handlers for PLAIN and SCRAM.
>>> > > > >
>>> > > > > On Tue, Dec 13, 2016 at 4:10 PM, Rajini Sivaram <
>>> > > > > rajinisivaram@googlemail.com> wrote:
>>> > > > >
>>> > > > > > Ismael,
>>> > > > > >
>>> > > > > > Thank you for the review. I will add an example.
>>> > > > > >
>>> > > > > > On Tue, Dec 13, 2016 at 1:07 PM, Ismael Juma <
>>> ismael@juma.me.uk>
>>> > > > wrote:
>>> > > > > >
>>> > > > > >> Hi Rajini,
>>> > > > > >>
>>> > > > > >> Thanks for the KIP. I think this is useful and users have
>>> asked
>>> > for
>>> > > > > >> something like that. I like that you have a scenarios
>>> section, do
>>> > > you
>>> > > > > >> think
>>> > > > > >> you could provide a rough sketch of what a callback handler
>>> would
>>> > > look
>>> > > > > >> like
>>> > > > > >> for the first 2 scenarios? They seem to be the common ones,
>>> so it
>>> > > > would
>>> > > > > >> help to see a concrete example.
>>> > > > > >>
>>> > > > > >> Ismael
>>> > > > > >>
>>> > > > > >> On Tue, Oct 11, 2016 at 7:28 AM, Rajini Sivaram <
>>> > > > > >> rajinisivaram@googlemail.com> wrote:
>>> > > > > >>
>>> > > > > >> > Hi all,
>>> > > > > >> >
>>> > > > > >> > I have just created KIP-86 make callback handlers in SASL
>>> > > > configurable
>>> > > > > >> so
>>> > > > > >> > that credential providers for SASL/PLAIN (and SASL/SCRAM
>>> when it
>>> > > is
>>> > > > > >> > implemented) can be used with custom credential callbacks:
>>> > > > > >> >
>>> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>> > > > > >> > 86%3A+Configurable+SASL+callback+handlers
>>> > > > > >> >
>>> > > > > >> > Comments and suggestions are welcome.
>>> > > > > >> >
>>> > > > > >> > Thank you...
>>> > > > > >> >
>>> > > > > >> >
>>> > > > > >> > Regards,
>>> > > > > >> >
>>> > > > > >> > Rajini
>>> > > > > >> >
>>> > > > > >>
>>> > > > > >
>>> > > > > >
>>> > > > > >
>>> > > > > > --
>>> > > > > > Regards,
>>> > > > > >
>>> > > > > > Rajini
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > > >
>>> > > > > --
>>> > > > > Regards,
>>> > > > >
>>> > > > > Rajini
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>
>>
>>
>
Re: [DISCUSS] KIP-86: Configurable SASL callback handlers
Posted by Rajini Sivaram <ra...@gmail.com>.
To support extensibility for SASL/OAuth as described in KIP-255
<https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=75968876>,
I have added an extra config sasl.login.callback.handler.class. This
implements the same interface as the other callback handlers. Default
behaviour will be unchanged.
Please let me know if you have any concerns.
Thank you,
Rajini
On Mon, Jan 29, 2018 at 7:07 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi all,
>
> To simplify dynamic update of SASL configs in future (e.g add a new SASL
> mechanism with a new callback handler or Login), I have separated out the
> broker-side configs with a mechanism prefix in the property name (similar
> to listener prefix) instead of including all the classes together as a map.
> This will also make it easier to configure new Login classes when new
> mechanisms are introduced alongside other mechanisms on a single listener.
>
> Please let me know if you have any concerns.
>
> Thank you,
>
> Rajini
>
>
> On Wed, Jan 17, 2018 at 6:54 AM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
>> Hi all,
>>
>> I have made some updates to this KIP to simplify addition of new SASL
>> mechanisms:
>>
>> 1. The Login interface has been made configurable as well (we have
>> had this interface for quite some time and it has been stable).
>> 2. The callback handler properties for client-side and server-side
>> have been separated out since we would never use the same classes for both.
>>
>> Any feedback or suggestions are welcome.
>>
>> Thank you,
>>
>> Rajini
>>
>>
>> On Mon, Apr 3, 2017 at 12:55 PM, Rajini Sivaram <ra...@gmail.com>
>> wrote:
>>
>>> If there are no other concerns or suggestions on this KIP, I will start
>>> vote later this week.
>>>
>>> Thank you...
>>>
>>> Regards,
>>>
>>> Rajini
>>>
>>> On Thu, Mar 30, 2017 at 9:42 PM, Rajini Sivaram <rajinisivaram@gmail.com
>>> > wrote:
>>>
>>>> I have made a minor change to the callback handler interface to pass in
>>>> the JAAS configuration entries in *configure,* to work with the
>>>> multiple listener configuration introduced in KIP-103. I have also renamed
>>>> the interface to AuthenticateCallbackHandler instead of AuthCallbackHandler
>>>> to avoid confusion with authorization.
>>>>
>>>> I have rebased and updated the PR (https://github.com/apache/kaf
>>>> ka/pull/2022) as well. Please let me know if there are any other
>>>> comments or suggestions to move this forward.
>>>>
>>>> Thank you...
>>>>
>>>> Regards,
>>>>
>>>> Rajini
>>>>
>>>>
>>>> On Thu, Dec 15, 2016 at 3:11 PM, Rajini Sivaram <rs...@pivotal.io>
>>>> wrote:
>>>>
>>>>> Ismael,
>>>>>
>>>>> The reason for choosing CallbackHandler interface as the configurable
>>>>> interface is flexibility. As you say, we could instead define a simpler
>>>>> PlainCredentialProvider and ScramCredentialProvider. But that would tie
>>>>> users to Kafka's SaslServer implementation for PLAIN and SCRAM.
>>>>> SaslServer/SaslClient implementations are already pluggable using
>>>>> standard
>>>>> Java security provider mechanism. Callback handlers are the
>>>>> configuration
>>>>> mechanism for SaslServer/SaslClient. By making the handlers
>>>>> configurable,
>>>>> SASL becomes fully configurable for mechanisms supported by Kafka as
>>>>> well
>>>>> as custom mechanisms. From the 'Scenarios' section in the KIP, a
>>>>> simpler
>>>>> PLAIN/SCRAM interface satisfies the first two, but configurable
>>>>> callback
>>>>> handlers enables all five. I agree that most users don't require this
>>>>> level
>>>>> of flexibility, but we have had discussions about custom mechanisms in
>>>>> the
>>>>> past for integration with existing authentication servers. So I think
>>>>> it is
>>>>> a feature worth supporting.
>>>>>
>>>>> On Thu, Dec 15, 2016 at 2:21 PM, Ismael Juma <is...@juma.me.uk>
>>>>> wrote:
>>>>>
>>>>> > Thanks Rajini, your answers make sense to me. One more general
>>>>> point: we
>>>>> > are following the JAAS callback architecture and exposing that to
>>>>> the user
>>>>> > where the user has to write code like:
>>>>> >
>>>>> > @Override
>>>>> > public void handle(Callback[] callbacks) throws IOException,
>>>>> > UnsupportedCallbackException {
>>>>> > String username = null;
>>>>> > for (Callback callback: callbacks) {
>>>>> > if (callback instanceof NameCallback)
>>>>> > username = ((NameCallback)
>>>>> callback).getDefaultName();
>>>>> > else if (callback instanceof PlainAuthenticateCallback) {
>>>>> > PlainAuthenticateCallback plainCallback =
>>>>> > (PlainAuthenticateCallback) callback;
>>>>> > boolean authenticated = authenticate(username,
>>>>> > plainCallback.password());
>>>>> > plainCallback.authenticated(authenticated);
>>>>> > } else
>>>>> > throw new UnsupportedCallbackException(callback);
>>>>> > }
>>>>> > }
>>>>> >
>>>>> > protected boolean authenticate(String username, char[] password)
>>>>> throws
>>>>> > IOException {
>>>>> > if (username == null)
>>>>> > return false;
>>>>> > else {
>>>>> > String expectedPassword =
>>>>> > JaasUtils.jaasConfig(LoginType.SERVER.contextName(), "user_" +
>>>>> username,
>>>>> > PlainLoginModule.class.getName());
>>>>> > return Arrays.equals(password,
>>>>> expectedPassword.toCharArray()
>>>>> > );
>>>>> > }
>>>>> > }
>>>>> >
>>>>> > Since we need to create a new callback type for Plain, Scram and so
>>>>> on, is
>>>>> > it really worth it to do it this way? For example, in the code
>>>>> above, the
>>>>> > `authenticate` method could be the only thing the user has to
>>>>> implement and
>>>>> > we could do the necessary work to unwrap the data from the various
>>>>> > callbacks when interacting with the SASL API. More work for us, but
>>>>> a much
>>>>> > more pleasant API for users. What are the drawbacks?
>>>>> >
>>>>> > Ismael
>>>>> >
>>>>> > On Thu, Dec 15, 2016 at 1:06 AM, Rajini Sivaram <rsivaram@pivotal.io
>>>>> >
>>>>> > wrote:
>>>>> >
>>>>> > > Ismael,
>>>>> > >
>>>>> > > 1. At the moment AuthCallbackHandler is not a public interface, so
>>>>> I am
>>>>> > > assuming that it can be modified. Yes, agree that we should keep
>>>>> > non-public
>>>>> > > methods separate. Will do that as part of the implementation of
>>>>> this KIP.
>>>>> > >
>>>>> > > 2. Callback handlers do tend to depend on ordering, including those
>>>>> > > included in the JVM and these in Kafka. I have specified the
>>>>> ordering in
>>>>> > > the KIP. Will make sure they get included in documentation too.
>>>>> > >
>>>>> > > 3. Added a note to the KIP. Kafka needs access to the SCRAM
>>>>> credentials
>>>>> > to
>>>>> > > perform SCRAM authentication. For PLAIN, Kafka only needs to know
>>>>> if the
>>>>> > > password is valid for the user. We want to support external
>>>>> > authentication
>>>>> > > servers whose interface is to validate password, not retrieve it.
>>>>> > >
>>>>> > > 4. Added code of ScramCredential to the KIP.
>>>>> > >
>>>>> > >
>>>>> > > On Wed, Dec 14, 2016 at 3:54 PM, Ismael Juma <is...@juma.me.uk>
>>>>> wrote:
>>>>> > >
>>>>> > > > Thanks Rajini, that helps. A few comments:
>>>>> > > >
>>>>> > > > 1. The `AuthCallbackHandler` interface already exists and we are
>>>>> making
>>>>> > > > breaking changes (removing a parameter from `configure` and
>>>>> adding
>>>>> > > > additional methods). Is the reasoning that it was not a public
>>>>> > interface
>>>>> > > > before? It would be good to clearly separate public versus
>>>>> non-public
>>>>> > > > interfaces in the security code (and we should tweak Gradle to
>>>>> publish
>>>>> > > > javadoc for the public ones).
>>>>> > > >
>>>>> > > > 2. It seems like there is an ordering when it comes to the
>>>>> invocation
>>>>> > of
>>>>> > > > callbacks. At least the current code assumes that `NameCallback`
>>>>> is
>>>>> > > called
>>>>> > > > first. If I am interpreting this correctly, we should specify
>>>>> that
>>>>> > > > ordering.
>>>>> > > >
>>>>> > > > 3. The approach taken by `ScramCredentialCallback` is different
>>>>> than
>>>>> > the
>>>>> > > > one taken by `PlainAuthenticateCallback`. The former lets the
>>>>> user pass
>>>>> > > the
>>>>> > > > credentials information while the latter passes the credentials
>>>>> and
>>>>> > lets
>>>>> > > > the user do the authentication. It would be good to explain the
>>>>> > > > inconsistency.
>>>>> > > >
>>>>> > > > 4. We reference `ScramCredential` in a few places, so it would
>>>>> be good
>>>>> > to
>>>>> > > > define that class too.
>>>>> > > >
>>>>> > > > Ismael
>>>>> > > >
>>>>> > > > On Wed, Dec 14, 2016 at 7:32 AM, Rajini Sivaram <
>>>>> > > > rajinisivaram@googlemail.com> wrote:
>>>>> > > >
>>>>> > > > > Have added sample callback handlers for PLAIN and SCRAM.
>>>>> > > > >
>>>>> > > > > On Tue, Dec 13, 2016 at 4:10 PM, Rajini Sivaram <
>>>>> > > > > rajinisivaram@googlemail.com> wrote:
>>>>> > > > >
>>>>> > > > > > Ismael,
>>>>> > > > > >
>>>>> > > > > > Thank you for the review. I will add an example.
>>>>> > > > > >
>>>>> > > > > > On Tue, Dec 13, 2016 at 1:07 PM, Ismael Juma <
>>>>> ismael@juma.me.uk>
>>>>> > > > wrote:
>>>>> > > > > >
>>>>> > > > > >> Hi Rajini,
>>>>> > > > > >>
>>>>> > > > > >> Thanks for the KIP. I think this is useful and users have
>>>>> asked
>>>>> > for
>>>>> > > > > >> something like that. I like that you have a scenarios
>>>>> section, do
>>>>> > > you
>>>>> > > > > >> think
>>>>> > > > > >> you could provide a rough sketch of what a callback handler
>>>>> would
>>>>> > > look
>>>>> > > > > >> like
>>>>> > > > > >> for the first 2 scenarios? They seem to be the common ones,
>>>>> so it
>>>>> > > > would
>>>>> > > > > >> help to see a concrete example.
>>>>> > > > > >>
>>>>> > > > > >> Ismael
>>>>> > > > > >>
>>>>> > > > > >> On Tue, Oct 11, 2016 at 7:28 AM, Rajini Sivaram <
>>>>> > > > > >> rajinisivaram@googlemail.com> wrote:
>>>>> > > > > >>
>>>>> > > > > >> > Hi all,
>>>>> > > > > >> >
>>>>> > > > > >> > I have just created KIP-86 make callback handlers in SASL
>>>>> > > > configurable
>>>>> > > > > >> so
>>>>> > > > > >> > that credential providers for SASL/PLAIN (and SASL/SCRAM
>>>>> when it
>>>>> > > is
>>>>> > > > > >> > implemented) can be used with custom credential callbacks:
>>>>> > > > > >> >
>>>>> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>>>> > > > > >> > 86%3A+Configurable+SASL+callback+handlers
>>>>> > > > > >> >
>>>>> > > > > >> > Comments and suggestions are welcome.
>>>>> > > > > >> >
>>>>> > > > > >> > Thank you...
>>>>> > > > > >> >
>>>>> > > > > >> >
>>>>> > > > > >> > Regards,
>>>>> > > > > >> >
>>>>> > > > > >> > Rajini
>>>>> > > > > >> >
>>>>> > > > > >>
>>>>> > > > > >
>>>>> > > > > >
>>>>> > > > > >
>>>>> > > > > > --
>>>>> > > > > > Regards,
>>>>> > > > > >
>>>>> > > > > > Rajini
>>>>> > > > > >
>>>>> > > > >
>>>>> > > > >
>>>>> > > > >
>>>>> > > > > --
>>>>> > > > > Regards,
>>>>> > > > >
>>>>> > > > > Rajini
>>>>> > > > >
>>>>> > > >
>>>>> > >
>>>>> >
>>>>>
>>>>
>>>>
>>>
>>
>
Re: [DISCUSS] KIP-86: Configurable SASL callback handlers
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi all,
To simplify dynamic update of SASL configs in future (e.g add a new SASL
mechanism with a new callback handler or Login), I have separated out the
broker-side configs with a mechanism prefix in the property name (similar
to listener prefix) instead of including all the classes together as a map.
This will also make it easier to configure new Login classes when new
mechanisms are introduced alongside other mechanisms on a single listener.
Please let me know if you have any concerns.
Thank you,
Rajini
On Wed, Jan 17, 2018 at 6:54 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi all,
>
> I have made some updates to this KIP to simplify addition of new SASL
> mechanisms:
>
> 1. The Login interface has been made configurable as well (we have had
> this interface for quite some time and it has been stable).
> 2. The callback handler properties for client-side and server-side
> have been separated out since we would never use the same classes for both.
>
> Any feedback or suggestions are welcome.
>
> Thank you,
>
> Rajini
>
>
> On Mon, Apr 3, 2017 at 12:55 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
>> If there are no other concerns or suggestions on this KIP, I will start
>> vote later this week.
>>
>> Thank you...
>>
>> Regards,
>>
>> Rajini
>>
>> On Thu, Mar 30, 2017 at 9:42 PM, Rajini Sivaram <ra...@gmail.com>
>> wrote:
>>
>>> I have made a minor change to the callback handler interface to pass in
>>> the JAAS configuration entries in *configure,* to work with the
>>> multiple listener configuration introduced in KIP-103. I have also renamed
>>> the interface to AuthenticateCallbackHandler instead of AuthCallbackHandler
>>> to avoid confusion with authorization.
>>>
>>> I have rebased and updated the PR (https://github.com/apache/kaf
>>> ka/pull/2022) as well. Please let me know if there are any other
>>> comments or suggestions to move this forward.
>>>
>>> Thank you...
>>>
>>> Regards,
>>>
>>> Rajini
>>>
>>>
>>> On Thu, Dec 15, 2016 at 3:11 PM, Rajini Sivaram <rs...@pivotal.io>
>>> wrote:
>>>
>>>> Ismael,
>>>>
>>>> The reason for choosing CallbackHandler interface as the configurable
>>>> interface is flexibility. As you say, we could instead define a simpler
>>>> PlainCredentialProvider and ScramCredentialProvider. But that would tie
>>>> users to Kafka's SaslServer implementation for PLAIN and SCRAM.
>>>> SaslServer/SaslClient implementations are already pluggable using
>>>> standard
>>>> Java security provider mechanism. Callback handlers are the
>>>> configuration
>>>> mechanism for SaslServer/SaslClient. By making the handlers
>>>> configurable,
>>>> SASL becomes fully configurable for mechanisms supported by Kafka as
>>>> well
>>>> as custom mechanisms. From the 'Scenarios' section in the KIP, a simpler
>>>> PLAIN/SCRAM interface satisfies the first two, but configurable callback
>>>> handlers enables all five. I agree that most users don't require this
>>>> level
>>>> of flexibility, but we have had discussions about custom mechanisms in
>>>> the
>>>> past for integration with existing authentication servers. So I think
>>>> it is
>>>> a feature worth supporting.
>>>>
>>>> On Thu, Dec 15, 2016 at 2:21 PM, Ismael Juma <is...@juma.me.uk> wrote:
>>>>
>>>> > Thanks Rajini, your answers make sense to me. One more general point:
>>>> we
>>>> > are following the JAAS callback architecture and exposing that to the
>>>> user
>>>> > where the user has to write code like:
>>>> >
>>>> > @Override
>>>> > public void handle(Callback[] callbacks) throws IOException,
>>>> > UnsupportedCallbackException {
>>>> > String username = null;
>>>> > for (Callback callback: callbacks) {
>>>> > if (callback instanceof NameCallback)
>>>> > username = ((NameCallback) callback).getDefaultName();
>>>> > else if (callback instanceof PlainAuthenticateCallback) {
>>>> > PlainAuthenticateCallback plainCallback =
>>>> > (PlainAuthenticateCallback) callback;
>>>> > boolean authenticated = authenticate(username,
>>>> > plainCallback.password());
>>>> > plainCallback.authenticated(authenticated);
>>>> > } else
>>>> > throw new UnsupportedCallbackException(callback);
>>>> > }
>>>> > }
>>>> >
>>>> > protected boolean authenticate(String username, char[] password)
>>>> throws
>>>> > IOException {
>>>> > if (username == null)
>>>> > return false;
>>>> > else {
>>>> > String expectedPassword =
>>>> > JaasUtils.jaasConfig(LoginType.SERVER.contextName(), "user_" +
>>>> username,
>>>> > PlainLoginModule.class.getName());
>>>> > return Arrays.equals(password,
>>>> expectedPassword.toCharArray()
>>>> > );
>>>> > }
>>>> > }
>>>> >
>>>> > Since we need to create a new callback type for Plain, Scram and so
>>>> on, is
>>>> > it really worth it to do it this way? For example, in the code above,
>>>> the
>>>> > `authenticate` method could be the only thing the user has to
>>>> implement and
>>>> > we could do the necessary work to unwrap the data from the various
>>>> > callbacks when interacting with the SASL API. More work for us, but a
>>>> much
>>>> > more pleasant API for users. What are the drawbacks?
>>>> >
>>>> > Ismael
>>>> >
>>>> > On Thu, Dec 15, 2016 at 1:06 AM, Rajini Sivaram <rs...@pivotal.io>
>>>> > wrote:
>>>> >
>>>> > > Ismael,
>>>> > >
>>>> > > 1. At the moment AuthCallbackHandler is not a public interface, so
>>>> I am
>>>> > > assuming that it can be modified. Yes, agree that we should keep
>>>> > non-public
>>>> > > methods separate. Will do that as part of the implementation of
>>>> this KIP.
>>>> > >
>>>> > > 2. Callback handlers do tend to depend on ordering, including those
>>>> > > included in the JVM and these in Kafka. I have specified the
>>>> ordering in
>>>> > > the KIP. Will make sure they get included in documentation too.
>>>> > >
>>>> > > 3. Added a note to the KIP. Kafka needs access to the SCRAM
>>>> credentials
>>>> > to
>>>> > > perform SCRAM authentication. For PLAIN, Kafka only needs to know
>>>> if the
>>>> > > password is valid for the user. We want to support external
>>>> > authentication
>>>> > > servers whose interface is to validate password, not retrieve it.
>>>> > >
>>>> > > 4. Added code of ScramCredential to the KIP.
>>>> > >
>>>> > >
>>>> > > On Wed, Dec 14, 2016 at 3:54 PM, Ismael Juma <is...@juma.me.uk>
>>>> wrote:
>>>> > >
>>>> > > > Thanks Rajini, that helps. A few comments:
>>>> > > >
>>>> > > > 1. The `AuthCallbackHandler` interface already exists and we are
>>>> making
>>>> > > > breaking changes (removing a parameter from `configure` and adding
>>>> > > > additional methods). Is the reasoning that it was not a public
>>>> > interface
>>>> > > > before? It would be good to clearly separate public versus
>>>> non-public
>>>> > > > interfaces in the security code (and we should tweak Gradle to
>>>> publish
>>>> > > > javadoc for the public ones).
>>>> > > >
>>>> > > > 2. It seems like there is an ordering when it comes to the
>>>> invocation
>>>> > of
>>>> > > > callbacks. At least the current code assumes that `NameCallback`
>>>> is
>>>> > > called
>>>> > > > first. If I am interpreting this correctly, we should specify that
>>>> > > > ordering.
>>>> > > >
>>>> > > > 3. The approach taken by `ScramCredentialCallback` is different
>>>> than
>>>> > the
>>>> > > > one taken by `PlainAuthenticateCallback`. The former lets the
>>>> user pass
>>>> > > the
>>>> > > > credentials information while the latter passes the credentials
>>>> and
>>>> > lets
>>>> > > > the user do the authentication. It would be good to explain the
>>>> > > > inconsistency.
>>>> > > >
>>>> > > > 4. We reference `ScramCredential` in a few places, so it would be
>>>> good
>>>> > to
>>>> > > > define that class too.
>>>> > > >
>>>> > > > Ismael
>>>> > > >
>>>> > > > On Wed, Dec 14, 2016 at 7:32 AM, Rajini Sivaram <
>>>> > > > rajinisivaram@googlemail.com> wrote:
>>>> > > >
>>>> > > > > Have added sample callback handlers for PLAIN and SCRAM.
>>>> > > > >
>>>> > > > > On Tue, Dec 13, 2016 at 4:10 PM, Rajini Sivaram <
>>>> > > > > rajinisivaram@googlemail.com> wrote:
>>>> > > > >
>>>> > > > > > Ismael,
>>>> > > > > >
>>>> > > > > > Thank you for the review. I will add an example.
>>>> > > > > >
>>>> > > > > > On Tue, Dec 13, 2016 at 1:07 PM, Ismael Juma <
>>>> ismael@juma.me.uk>
>>>> > > > wrote:
>>>> > > > > >
>>>> > > > > >> Hi Rajini,
>>>> > > > > >>
>>>> > > > > >> Thanks for the KIP. I think this is useful and users have
>>>> asked
>>>> > for
>>>> > > > > >> something like that. I like that you have a scenarios
>>>> section, do
>>>> > > you
>>>> > > > > >> think
>>>> > > > > >> you could provide a rough sketch of what a callback handler
>>>> would
>>>> > > look
>>>> > > > > >> like
>>>> > > > > >> for the first 2 scenarios? They seem to be the common ones,
>>>> so it
>>>> > > > would
>>>> > > > > >> help to see a concrete example.
>>>> > > > > >>
>>>> > > > > >> Ismael
>>>> > > > > >>
>>>> > > > > >> On Tue, Oct 11, 2016 at 7:28 AM, Rajini Sivaram <
>>>> > > > > >> rajinisivaram@googlemail.com> wrote:
>>>> > > > > >>
>>>> > > > > >> > Hi all,
>>>> > > > > >> >
>>>> > > > > >> > I have just created KIP-86 make callback handlers in SASL
>>>> > > > configurable
>>>> > > > > >> so
>>>> > > > > >> > that credential providers for SASL/PLAIN (and SASL/SCRAM
>>>> when it
>>>> > > is
>>>> > > > > >> > implemented) can be used with custom credential callbacks:
>>>> > > > > >> >
>>>> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>>> > > > > >> > 86%3A+Configurable+SASL+callback+handlers
>>>> > > > > >> >
>>>> > > > > >> > Comments and suggestions are welcome.
>>>> > > > > >> >
>>>> > > > > >> > Thank you...
>>>> > > > > >> >
>>>> > > > > >> >
>>>> > > > > >> > Regards,
>>>> > > > > >> >
>>>> > > > > >> > Rajini
>>>> > > > > >> >
>>>> > > > > >>
>>>> > > > > >
>>>> > > > > >
>>>> > > > > >
>>>> > > > > > --
>>>> > > > > > Regards,
>>>> > > > > >
>>>> > > > > > Rajini
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > --
>>>> > > > > Regards,
>>>> > > > >
>>>> > > > > Rajini
>>>> > > > >
>>>> > > >
>>>> > >
>>>> >
>>>>
>>>
>>>
>>
>