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 <ism...@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 <rsiva...@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 <ism...@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 <
> > > rajinisiva...@googlemail.com> wrote:
> > >
> > > > Have added sample callback handlers for PLAIN and SCRAM.
> > > >
> > > > On Tue, Dec 13, 2016 at 4:10 PM, Rajini Sivaram <
> > > > rajinisiva...@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 <ism...@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 <
> > > > >> rajinisiva...@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
> > > >
> > >
> >
>
