Hi folks. I think there are a couple of issues that were just raised in
this thread. One is whether the ability to use PasswordCallback exists,
and if so whether that impacts the applicability of this KIP to the
SASL/OAUTHBEARER KIP-255. The second issue is related to how we might
leverage this KIP more broadly (including as an alternative to KIP-76)
while maintaining forward compatibility and not causing unexpected
substitutions/parsing exceptions.
Let me address the second issue (more broad use) first, since I think
Rajini hit on a good possibility. Currently this KIP addresses the
possibility of an unexpected substitution by saying "This would cause a
substitution to be attempted, which of course would fail and raise an
exception." I think Rajini's idea is to explicitly state that any
substitution that cannot be parsed is to be treated as a pass-thru or a
no-op. So, for example, if a configured password happened to look like
"Asd$[,mhsd_4]Q" and a substitution was attempted on that value then the
result should not be an exception simply because "$[,mhsd_4]" couldn't be
parsed according to the required delimited syntax but should instead just
end up doing nothing and the password would remain"Asd$[,mhsd_4]Q".
Rajini, do I have that right? If so, then I think it is worth considering
the possibility that substitution could be turned on more broadly with an
acceptably low risk. In the interest of caution substitution could still
be turned on only as an opt-in, but it could be a global opt-in if we
explicitly take a "do no harm" approach to things that have delimiters but
do not parse as valid substitutions.
Regarding whether the ability to use PasswordCallback exists in
SASL/OAUTHBEARER KIP-255, I don't think it does. The reason is because
customers do not generally write the login module implementation; they use
the implementation provided, which is short and delegates the token
retrieval to the callback handler (which users are expected to provide).
Here is the login module code:
@Override
public boolean login() throws LoginException {
OAuthBearerLoginCallback callback = new OAuthBearerLoginCallback();
try {
this.callbackHandler.handle(new Callback[] {callback});
} catch (IOException | UnsupportedCallbackException e) {
log.error(e.getMessage(), e);
throw new LoginException("An internal error occurred");
}
token = callback.token();
if (token == null) {
log.info(String.format("Login failed: %s : %s (URI=%s)",
callback.errorCode(), callback.errorDescription(),
callback.errorUri()));
throw new LoginException(callback.errorDescription());
}
log.info("Login succeeded");
return true;
}
I don't see the callbackHandler using a PasswordCallback instance to ask
(itself?) to retrieve a password. If the callbackHandler needs a password,
then I imagine it will get that password from a login module option, and
that could in turn come from a file, environment variable, password vault,
etc. if substitution is applicable.
Ron
On Fri, Apr 6, 2018 at 4:41 AM, Rajini Sivaram <rajinisiva...@gmail.com>
wrote:
> Yes, I was going to suggest that we should do this for all configs earlier,
> but was reluctant to do that since in its current form, there is a
> property enableSubstitution
> (in JAAS config at the moment) that indicates if substitution is to be
> performed. If enabled, all values in that config are considered for
> substitution. That works for JAAS configs with a small number of
> properties, but I wasn't sure it was reasonable to do this for all Kafka
> configs where there are several configs that may contain arbitrary
> characters including substitution delimiters. It will be good if some
> configs that contain arbitrary characters can be specified directly in the
> config while others are substituted from elsewhere. Perhaps a substitution
> type that simply uses the value within delimiters would work? Ron, what do
> you think?
>
>
>
> On Fri, Apr 6, 2018 at 7:49 AM, Manikumar <manikumar.re...@gmail.com>
> wrote:
>
> > Hi,
> >
> > Substitution mechanism can be useful to configure regular password
> configs
> > liken ssl.keystore.password, ssl.truststore.password, etc.
> > This is can be good alternative to previously proposed KIP-76 and will
> give
> > more options to the user.
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 76+Enable+getting+password+from+executable+rather+than+
> > passing+as+plaintext+in+config+files
> >
> >
> > Thanks,
> >
> > On Fri, Apr 6, 2018 at 4:29 AM, Rajini Sivaram <rajinisiva...@gmail.com>
> > wrote:
> >
> > > Hi Ron,
> > >
> > > For the password example, you could define a login CallbackHandler that
> > > processes PasswordCallback to provide passwords. We don't currently do
> > this
> > > with PLAIN/SCRAM because login callback handlers were not configurable
> > > earlier and we haven't updated the login modules to do this. But that
> > could
> > > be one way of providing passwords and integrating with other password
> > > sources, now that we have configurable login callback handlers. I was
> > > wondering whether similar approach could be used for the parameters
> that
> > > OAuth needed to obtain at runtime. We could still have this KIP with
> > > built-in substitutable types to handle common cases like getting
> options
> > > from a file without writing any code. But I wasn't sure if there were
> > OAuth
> > > options that couldn't be handled as callbacks using the login callback
> > > handler.
> > >
> > > On Thu, Apr 5, 2018 at 10:25 PM, Ron Dagostino <rndg...@gmail.com>
> > wrote:
> > >
> > > > Hi Rajini. Thanks for the questions. I could see someone wanting to
> > > > retrieve a password from a vended password vault solution (for
> > example);
> > > > that is the kind of scenario that the ability to add new
> substitutable
> > > > types would be meant for. I do still consider this KIP 269 to be a
> > > > prerequisite for the SASL/OAUTHBEARER KIP 255. I am open to a
> > different
> > > > perspective in case I missed or misunderstood your point.
> > > >
> > > > Ron
> > > >
> > > > On Thu, Apr 5, 2018 at 8:13 AM, Rajini Sivaram <
> > rajinisiva...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Ron,
> > > > >
> > > > > Now that login callback handlers are configurable, is this KIP
> still
> > a
> > > > > pre-req for OAuth? I was wondering whether we still need the
> ability
> > to
> > > > add
> > > > > new substitutable types or whether it would be sufficient to add
> the
> > > > > built-in ones to read from file etc.
> > > > >
> > > > >
> > > > > On Thu, Mar 29, 2018 at 6:48 AM, Ron Dagostino <rndg...@gmail.com>
> > > > wrote:
> > > > >
> > > > > > Hi everyone. There have been no comments on this KIP, so I
> intend
> > to
> > > > put
> > > > > > it to a vote next week if there are no comments that might entail
> > > > changes
> > > > > > between now and then. Please take a look in the meantime if you
> > > wish.
> > > > > >
> > > > > > Ron
> > > > > >
> > > > > > On Thu, Mar 15, 2018 at 2:36 PM, Ron Dagostino <
> rndg...@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > > Hi everyone.
> > > > > > >
> > > > > > > I created KIP-269: Substitution Within Configuration Values
> > > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> > > > > > 269+Substitution+Within+Configuration+Values>
> > > > > > > (https://cwiki.apache.org/confluence/display/KAFKA/KIP+269+
> > > > > > > Substitution+Within+Configuration+Values
> > > > > > > <https://cwiki.apache.org/confluence/pages/viewpage.
> > > > > > action?pageId=75968876>
> > > > > > > ).
> > > > > > >
> > > > > > > This KIP proposes adding support for substitution within client
> > > JAAS
> > > > > > > configuration values for PLAIN and SCRAM-related SASL
> mechanisms
> > > in a
> > > > > > > backwards-compatible manner and making the functionality
> > available
> > > to
> > > > > > other
> > > > > > > existing (or future) configuration contexts where it is deemed
> > > > > > appropriate.
> > > > > > >
> > > > > > > This KIP was extracted from (and is now a prerequisite for)
> > > KIP-255:
> > > > > > > OAuth Authentication via SASL/OAUTHBEARER
> > > > > > > <https://cwiki.apache.org/confluence/pages/viewpage.
> > > > > > action?pageId=75968876>
> > > > > > > based on discussion of that KIP.
> > > > > > >
> > > > > > > Ron
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
