Hi Rajini. I've also been thinking about how sasl.jaas.config will be
parsed. Something that is implicit in the current proposal needs to be
made explicit if this is to be applied more broadly, and that is the fact
that there is a namespacing of sorts going on. For example, in the current
proposal, when you indicate that you want to somehow refer to another value
(via defaultKey=<key> or fromValueOfKey) then the key being referred to is
taken as a JAAS module option name. If we allow substitution at the
cluster/producer/consumer config level then within the context of something
like ssl.keystore.password any such key being referred to would be a
cluster/producer/consumer config key. But I think within the context of
sasl.jaas.config any key reference should still be referring to a JAAS
module option. I think sasl.jaas.config would need to be special-cased in
the sense that its value would not have substitution applied to it up front
but instead would have substitution applied to it later on. In other
words, the login module would be where the substitution logic is applied.
Note that we re-use an existing kerberos login module, so we do not control
that code and cannot apply substitution logic there, so I think the
statement regarding if/how sasl.jaas.config (or any JAAS configuration
file) is processed with respect to substitution is to say that it is
determined by the login module.
I think the chances of an unintended substitution accidentally parsing
correctly is pretty close to zero, but making substitution an opt-in means
the possibility -- regardless of how small it might be -- will be
explicitly acknowledged. I think that makes it fine.
I suspect you are right that adding substitution into cluster/producer/consumer
configs will require careful code changes given how configs are
currently implemented. I will take a closer look to see how it might be
done; if it isn't obvious to me then perhaps it would be best to split that
change out into a separate JIRA issue so that someone with more experience
with that part of the code can do it. But I'll still take a look just in
case I can see how it should be done.
I agree that the OAuth callback handler that needs to be written anyway
could simply go out and talk directly to a password vault. With
substitution as an option, though, the callback handler can just ask for
the value from a JAAS module option, and then whether that goes out to a
password vault or not would be up to how the app is configured. I think
the ability to encapsulate the actual mechanism behind a module option is
valuable.
Ron
On Sun, Apr 8, 2018 at 4:47 AM, Rajini Sivaram <rajinisiva...@gmail.com>
wrote:
> 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
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
