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...@googlemail.com> on 2016/10/11 14:28:56 UTC

[DISCUSS] KIP-86: Configurable SASL callback handlers

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

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
>>>> > > > >
>>>> > > >
>>>> > >
>>>> >
>>>>
>>>
>>>
>>
>

Re: [DISCUSS] KIP-86: Configurable SASL callback handlers

Posted by Rajini Sivaram <ra...@gmail.com>.
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>.
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/
> kafka/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>.
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/kafka/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 <is...@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 <rs...@pivotal.io>.
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 <is...@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 Ismael Juma <is...@juma.me.uk>.
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 <is...@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 <rs...@pivotal.io>.
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 <is...@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 Ismael Juma <is...@juma.me.uk>.
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 <is...@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...@googlemail.com>.
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 <is...@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...@googlemail.com>.
Ismael,

Thank you for the review. I will add an example.

On Tue, Dec 13, 2016 at 1:07 PM, Ismael Juma <is...@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

Re: [DISCUSS] KIP-86: Configurable SASL callback handlers

Posted by Ismael Juma <is...@juma.me.uk>.
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
>

Re: [DISCUSS] KIP-86: Configurable SASL callback handlers

Posted by Jun Rao <ju...@confluent.io>.
Hi, Rajini,

Thanks for the explanation. So AccessController.getContext() returns the
context specific to the calling thread. Then, this should work.

Jun

On Thu, Oct 27, 2016 at 2:32 AM, Rajini Sivaram <
rajinisivaram@googlemail.com> wrote:

> Hi Jun,
>
> We will be using the AccessControlContext corresponding to the current
> calling context (i.e. on the current thread). So concurrent Subject.doAs()
> in other threads will not have any impact. On the current thread, we are
> using the Subject corresponding to the latest Subject.doAs(). This is safe
> since callbacks are triggered by SaslClient only in code invoked within a
> Subject.doAs() block in Kafka. For example (from
> SaslClientAuthenticator.java):
>
>         return Subject.doAs(subject, new PrivilegedExceptionAction<
> byte[]>()
> {
>
>             public byte[] run() throws SaslException {
>
>                 return saslClient.evaluateChallenge(saslToken);
>
>             }
>
>         }
>
>
> Kerberos implementation in the JRE already relies on Subject from the
> current calling context. So Kafka code does have to guarantee that these
> operations are performed under a Subject.doAs() anyway.
>
>
>
> On Wed, Oct 26, 2016 at 9:26 PM, Jun Rao <ju...@confluent.io> wrote:
>
> > Hi, Rajini,
> >
> > The javadoc of Subject.getSubjectAccessControlContext
> > <https://docs.oracle.com/javase/7/docs/api/java/
> > security/AccessControlContext.html>
> > acc)
> > says the following. So, are we depending on the correct ordering to get
> the
> > right subject? Is there any issue if two Subject.doAs() are called
> > concurrently?
> >
> > "The AccessControlContext may contain many Subjects (from nested doAs
> > calls).
> > In this situation, the most recent Subject associated with the
> > AccessControlContext is returned"
> >
> > Thanks,
> >
> > Jun
> >
> > On Wed, Oct 26, 2016 at 5:25 AM, Rajini Sivaram <
> > rajinisivaram@googlemail.com> wrote:
> >
> > > Hi Jun,
> > >
> > > The main processing method in the callback handler is
> "handle(Callback[]
> > > callbacks)". This method is invoked by SaslClient either when the
> > > SaslClient
> > > is constructed or when Kafka's SASL authentication code invokes
> > > saslClient.evaluateChallenge(). These are always done under
> > > Subject.doAs() -
> > > this is already the case in Kafka. Hence handle() method of the shared
> > > callback handler can get Subject from the calling context and this
> > Subject
> > > corresponds to the client connection for which callback is being
> > requested.
> > >
> > > *Current per-connection SaslClientCallbackHandler:*
> > >
> > >
> > >     private Subject subject;
> > >
> > >     @Override
> > >
> > >     public void configure(Map<String, ?> configs, Mode mode, Subject
> > > subject, String mechanism) {
> > >
> > >         this.isKerberos = mechanism.equals(SaslConfigs.
> > GSSAPI_MECHANISM);
> > >
> > >         this.subject = subject;
> > >
> > >     }
> > >
> > >     @Override
> > >
> > >     public void handle(Callback[] callbacks) throws
> > > UnsupportedCallbackException {
> > >
> > >         // Uses this.subject
> > >
> > >         ....
> > >
> > >     }
> > >
> > >
> > > *Proposed shared **SaslClientCallbackHandler**:*
> > >
> > >     @Override
> > >
> > >     public void configure(Map<String, ?> configs, String
> saslMechanism) {
> > >
> > >     }
> > >
> > >     @Override
> > >
> > >     public void handle(Callback[] callbacks) throws
> > > UnsupportedCallbackException {
> > >
> > >         Subject subject = Subject.getSubject(
> > > AccessController.getContext());
> > >
> > >         ....
> > >
> > >     }
> > >
> > >
> > >
> > >
> > > On Wed, Oct 26, 2016 at 2:58 AM, Jun Rao <ju...@confluent.io> wrote:
> > >
> > > > Hi, Rajini,
> > > >
> > > > Thanks for the proposal.
> > > >
> > > > One quick question. The KIP says "SaslClientCallbackHandler will be
> > > > modified to obtain Subject using
> > > > *Subject.getSubject(AccessController.getContext())* to avoid the
> > current
> > > > per-connection state." Since subject is going to be different for
> > > different
> > > > connections, how do we get the connection specific subject if there
> is
> > > only
> > > > a single instance of the callback handler? The modification seems to
> > call
> > > > only some static methods.
> > > >
> > > > Jun
> > > >
> > > >
> > > >
> > > > 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...@googlemail.com>.
Hi Jun,

We will be using the AccessControlContext corresponding to the current
calling context (i.e. on the current thread). So concurrent Subject.doAs()
in other threads will not have any impact. On the current thread, we are
using the Subject corresponding to the latest Subject.doAs(). This is safe
since callbacks are triggered by SaslClient only in code invoked within a
Subject.doAs() block in Kafka. For example (from
SaslClientAuthenticator.java):

        return Subject.doAs(subject, new PrivilegedExceptionAction<byte[]>()
{

            public byte[] run() throws SaslException {

                return saslClient.evaluateChallenge(saslToken);

            }

        }


Kerberos implementation in the JRE already relies on Subject from the
current calling context. So Kafka code does have to guarantee that these
operations are performed under a Subject.doAs() anyway.



On Wed, Oct 26, 2016 at 9:26 PM, Jun Rao <ju...@confluent.io> wrote:

> Hi, Rajini,
>
> The javadoc of Subject.getSubjectAccessControlContext
> <https://docs.oracle.com/javase/7/docs/api/java/
> security/AccessControlContext.html>
> acc)
> says the following. So, are we depending on the correct ordering to get the
> right subject? Is there any issue if two Subject.doAs() are called
> concurrently?
>
> "The AccessControlContext may contain many Subjects (from nested doAs
> calls).
> In this situation, the most recent Subject associated with the
> AccessControlContext is returned"
>
> Thanks,
>
> Jun
>
> On Wed, Oct 26, 2016 at 5:25 AM, Rajini Sivaram <
> rajinisivaram@googlemail.com> wrote:
>
> > Hi Jun,
> >
> > The main processing method in the callback handler is "handle(Callback[]
> > callbacks)". This method is invoked by SaslClient either when the
> > SaslClient
> > is constructed or when Kafka's SASL authentication code invokes
> > saslClient.evaluateChallenge(). These are always done under
> > Subject.doAs() -
> > this is already the case in Kafka. Hence handle() method of the shared
> > callback handler can get Subject from the calling context and this
> Subject
> > corresponds to the client connection for which callback is being
> requested.
> >
> > *Current per-connection SaslClientCallbackHandler:*
> >
> >
> >     private Subject subject;
> >
> >     @Override
> >
> >     public void configure(Map<String, ?> configs, Mode mode, Subject
> > subject, String mechanism) {
> >
> >         this.isKerberos = mechanism.equals(SaslConfigs.
> GSSAPI_MECHANISM);
> >
> >         this.subject = subject;
> >
> >     }
> >
> >     @Override
> >
> >     public void handle(Callback[] callbacks) throws
> > UnsupportedCallbackException {
> >
> >         // Uses this.subject
> >
> >         ....
> >
> >     }
> >
> >
> > *Proposed shared **SaslClientCallbackHandler**:*
> >
> >     @Override
> >
> >     public void configure(Map<String, ?> configs, String saslMechanism) {
> >
> >     }
> >
> >     @Override
> >
> >     public void handle(Callback[] callbacks) throws
> > UnsupportedCallbackException {
> >
> >         Subject subject = Subject.getSubject(
> > AccessController.getContext());
> >
> >         ....
> >
> >     }
> >
> >
> >
> >
> > On Wed, Oct 26, 2016 at 2:58 AM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Hi, Rajini,
> > >
> > > Thanks for the proposal.
> > >
> > > One quick question. The KIP says "SaslClientCallbackHandler will be
> > > modified to obtain Subject using
> > > *Subject.getSubject(AccessController.getContext())* to avoid the
> current
> > > per-connection state." Since subject is going to be different for
> > different
> > > connections, how do we get the connection specific subject if there is
> > only
> > > a single instance of the callback handler? The modification seems to
> call
> > > only some static methods.
> > >
> > > Jun
> > >
> > >
> > >
> > > 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 Jun Rao <ju...@confluent.io>.
Hi, Rajini,

The javadoc of Subject.getSubjectAccessControlContext
<https://docs.oracle.com/javase/7/docs/api/java/security/AccessControlContext.html>
acc)
says the following. So, are we depending on the correct ordering to get the
right subject? Is there any issue if two Subject.doAs() are called
concurrently?

"The AccessControlContext may contain many Subjects (from nested doAs calls).
In this situation, the most recent Subject associated with the
AccessControlContext is returned"

Thanks,

Jun

On Wed, Oct 26, 2016 at 5:25 AM, Rajini Sivaram <
rajinisivaram@googlemail.com> wrote:

> Hi Jun,
>
> The main processing method in the callback handler is "handle(Callback[]
> callbacks)". This method is invoked by SaslClient either when the
> SaslClient
> is constructed or when Kafka's SASL authentication code invokes
> saslClient.evaluateChallenge(). These are always done under
> Subject.doAs() -
> this is already the case in Kafka. Hence handle() method of the shared
> callback handler can get Subject from the calling context and this Subject
> corresponds to the client connection for which callback is being requested.
>
> *Current per-connection SaslClientCallbackHandler:*
>
>
>     private Subject subject;
>
>     @Override
>
>     public void configure(Map<String, ?> configs, Mode mode, Subject
> subject, String mechanism) {
>
>         this.isKerberos = mechanism.equals(SaslConfigs.GSSAPI_MECHANISM);
>
>         this.subject = subject;
>
>     }
>
>     @Override
>
>     public void handle(Callback[] callbacks) throws
> UnsupportedCallbackException {
>
>         // Uses this.subject
>
>         ....
>
>     }
>
>
> *Proposed shared **SaslClientCallbackHandler**:*
>
>     @Override
>
>     public void configure(Map<String, ?> configs, String saslMechanism) {
>
>     }
>
>     @Override
>
>     public void handle(Callback[] callbacks) throws
> UnsupportedCallbackException {
>
>         Subject subject = Subject.getSubject(
> AccessController.getContext());
>
>         ....
>
>     }
>
>
>
>
> On Wed, Oct 26, 2016 at 2:58 AM, Jun Rao <ju...@confluent.io> wrote:
>
> > Hi, Rajini,
> >
> > Thanks for the proposal.
> >
> > One quick question. The KIP says "SaslClientCallbackHandler will be
> > modified to obtain Subject using
> > *Subject.getSubject(AccessController.getContext())* to avoid the current
> > per-connection state." Since subject is going to be different for
> different
> > connections, how do we get the connection specific subject if there is
> only
> > a single instance of the callback handler? The modification seems to call
> > only some static methods.
> >
> > Jun
> >
> >
> >
> > 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
>

Re: [DISCUSS] KIP-86: Configurable SASL callback handlers

Posted by Rajini Sivaram <ra...@googlemail.com>.
Hi Jun,

The main processing method in the callback handler is "handle(Callback[]
callbacks)". This method is invoked by SaslClient either when the SaslClient
is constructed or when Kafka's SASL authentication code invokes
saslClient.evaluateChallenge(). These are always done under Subject.doAs() -
this is already the case in Kafka. Hence handle() method of the shared
callback handler can get Subject from the calling context and this Subject
corresponds to the client connection for which callback is being requested.

*Current per-connection SaslClientCallbackHandler:*


    private Subject subject;

    @Override

    public void configure(Map<String, ?> configs, Mode mode, Subject
subject, String mechanism) {

        this.isKerberos = mechanism.equals(SaslConfigs.GSSAPI_MECHANISM);

        this.subject = subject;

    }

    @Override

    public void handle(Callback[] callbacks) throws
UnsupportedCallbackException {

        // Uses this.subject

        ....

    }


*Proposed shared **SaslClientCallbackHandler**:*

    @Override

    public void configure(Map<String, ?> configs, String saslMechanism) {

    }

    @Override

    public void handle(Callback[] callbacks) throws
UnsupportedCallbackException {

        Subject subject = Subject.getSubject(AccessController.getContext());

        ....

    }




On Wed, Oct 26, 2016 at 2:58 AM, Jun Rao <ju...@confluent.io> wrote:

> Hi, Rajini,
>
> Thanks for the proposal.
>
> One quick question. The KIP says "SaslClientCallbackHandler will be
> modified to obtain Subject using
> *Subject.getSubject(AccessController.getContext())* to avoid the current
> per-connection state." Since subject is going to be different for different
> connections, how do we get the connection specific subject if there is only
> a single instance of the callback handler? The modification seems to call
> only some static methods.
>
> Jun
>
>
>
> 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

Re: [DISCUSS] KIP-86: Configurable SASL callback handlers

Posted by Jun Rao <ju...@confluent.io>.
Hi, Rajini,

Thanks for the proposal.

One quick question. The KIP says "SaslClientCallbackHandler will be
modified to obtain Subject using
*Subject.getSubject(AccessController.getContext())* to avoid the current
per-connection state." Since subject is going to be different for different
connections, how do we get the connection specific subject if there is only
a single instance of the callback handler? The modification seems to call
only some static methods.

Jun



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
>