Hi Ron,
Thanks for the responses.
For broader use as configs, opt-in makes sense to avoid any surprises and a
global opt-in ought to be fine.
If we do want to use this for all configs, a few things to consider:
- How will sasl.jaas.config property will get parsed? This is
essentially a compound config which may contain some options that are
substitutable. Will it be possible to handle this and static JAAS
configuration files in the same way?
- At the moment I think the whole option is redacted if one substitution
is marked redact. Would it make sense to define values that consist of
some redactable components. I am thinking of sasl.jaas.config as a
single property, but perhaps treating this alone separately is good enough.
- If we did treat failed substitutions as pass-thru, would it cover all
cases, or do we also need to be concerned about valid substitutions that
weren't intended to be substitutions? I am thinking that we don't need to
worry about the latter if substitutions are only by opt-in.
- It will be good to get more feedback on this KIP before updating the
code to use it for all configs since the code may need to change quite a
bit to fit in with the config classes.
For the callbacks, I agree that we want a LoginModule for OAuth that can be
reused. But to use OAuth, you will probably have your own callback handler
implementation to process OAuthBearerLoginCallback . From the example, it
is not clear to me why the callback handler that processes
OAuthBearerLoginCallback cannot do whatever a custom substitution class
would do, e,g. read some options like passwordVaultUrl from the JAAS config
(which it has access to) and retrieve passwords from a password vault? If
we are going to have extensible substitution anyway, then obviously, we
could use that as an option here too.
On Fri, Apr 6, 2018 at 2:47 PM, Ron Dagostino <rndg...@gmail.com> wrote:
> 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
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
