kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colin McCabe <cmcc...@apache.org>
Subject Re: [DISCUSS] KIP-269: Substitution Within Configuration Values
Date Sat, 14 Apr 2018 03:51:06 GMT
On Fri, Apr 13, 2018, at 10:30, Rajini Sivaram wrote:
> Hi Colin,
> 
> JAAS configuration can be provided in a separate file, but that has been
> the cause of various problems in itself. The configuration option
> `sasl.jaas.config` was added to overcome this. This is already a dynamic
> configuration option stored in ZooKeeper since we allow listeners to be
> added dynamically. Going forward, the property should be the preferred way
> to configure SASL. I don't think we should allow any form of configuration
> substitutions for JAAS that don't make sense for regular configs. And if we
> are going to have a substitution mechanism, e.g. for password configs, then
> it makes sense to allow for SSL as well as SASL.
> 
> So the question really is what forms of substitution, if any, make sense. I
> agree that use of system properties and environment variables are not a
> good idea, but should references to files be allowed? Sounds like that is a
> bad idea too from your experience. Does it make sense to have a extensible
> substitution mechanism to allow users to integrate with password vaults or
> other sources of config values?

Hmm.  It seems like in order to authenticate with any kind of password vault, you would need a JAAS configuration, right?  So you can't really store your JAAS config in a password vault, although there may be other things you could usefully store there.  That's why I think JAAS really is a special case here, worth considering separately.  Storing your private key in a local file seems like a reasonable idea; storing the value of max.poll.records in a file does not seem that useful or reasonable.

best,
Colin

> 
> 
> On Fri, Apr 13, 2018 at 5:56 PM, Colin McCabe <cmccabe@apache.org> wrote:
> 
> > I think we need to be a very careful here.  Configuration complexity can
> > get out of control very quickly.  There are also some conflicting goals
> > here.
> >
> > As much as possible, we want the configuration to be a full description of
> > what the broker is going to do.  If the configuration pulls in environment
> > variables, system properties, local files, or other aspects of the local
> > system environment, it is no longer a complete description  of what the
> > broker is going to do.  Instead, you have to know about the full UNIX
> > environment to understand what is going on.  This makes deployments less
> > repeatable and will lead to hard-to-track-down problems if one node has a
> > different set of environment variables than the others, etc.
> >
> > We want it to be easy to roll out a new configuration to all brokers
> > without restarting them all.  We should expect that in the future, more and
> > more configurations will be KIP-226 style dynamic configurations that are
> > stored in ZooKeeper and centrally rolled out to all brokers without a
> > restart.  If we have to restart processes with different environment or
> > system properties, or change local files, in order to reconfigure, we can't
> > accomplish this goal.  As much as possible, the centrally managed
> > configurations should not refer to local system properties.
> >
> > Configurations should be loaded efficiently.  But if loading the
> > configuration requires opening and reading local files, it could get
> > extremely slow.  I saw this problem firsthand in Hadoop, where invoking
> > "new Configuration()" causes dozens of XML files to be loaded and parsed.
> > Also, keep in mind that things other than the broker need to load
> > configurations.  Every client and every tool needs to perform the same
> > process.
> >
> > If configuration keys can reference and include other configuration keys,
> > renaming or deprecating something becomes even harder than it is now.  And
> > if one configuration key changes because it is a dynamic configuration key,
> > should all the configuration keys that included that one change as well?
> > This feature simply doesn't work well with our other features.
> >
> > It seems like most of these problems can be solved better and more easily
> > outside Kafka.  For example, it's straightforward to write a bash script
> > that examines some environment variables, constructs a Kafka configuration
> > file and then runs the Kafka broker with that file.  This also makes it
> > straightforward to set configuration keys in tandem, if that's what you
> > want.
> >
> > I think we should focus just on what JAAS needs, which seems very
> > different than what other configurations need.  In the specific case of
> > JAAS, it makes sense to consider loading stuff from a separate file, to
> > avoid having credentials stored in the properties file.  (But I thought we
> > already had a way to do that?)
> >
> > best,
> > Colin
> >
> >
> > On Fri, Apr 13, 2018, at 07:16, Rajini Sivaram wrote:
> > > Hi Ron,
> > >
> > > I think we should be able to process substitutions for both static JAAS
> > > configuration file as well as `sasl.jaas.config` property. We load the
> > > configuration using org.apache.kafka.common.security.
> > > JaasContext.loadXXXContext() and that would be a good place to do any
> > > substitution. The method has access to the producer/consumer/broker
> > configs
> > > as well in case we want keys to refer to these.
> > >
> > > On Fri, Apr 13, 2018 at 2:15 PM, Ron Dagostino <rndgstn@gmail.com>
> > wrote:
> > >
> > > > Hi Rajini.  Regarding processing the sasl.jaas.config value up-front,
> > there
> > > > are a couple of things that occur to me about it.  First, the older
> > way of
> > > > storing the JAAS config in a separate file is still supported (and is
> > at
> > > > this time the prevalent mechanism on the broker side since
> > sasl.jaas.config
> > > > support for brokers was only recently added via KIP 226).  We could
> > state
> > > > that substitution is only supported via sasl.jaas.config and force
> > people
> > > > to convert over to get substitution functionality, but that wouldn't be
> > > > necessary if we let the login module do the substitution later on.
> > > >
> > > > The second thing that occurs to me is related to namespacing and
> > creates
> > > > tension with the first point above.  If we refer to the "fubar" key in
> > the
> > > > config, is that key a JAAS module option or is it a value in the
> > > > cluster/producer/consumer config?  It would be very positive if we
> > could
> > > > eliminate namespacing entirely such that when you reference another
> > key it
> > > > is always very clear what is being referred to -- i.e. always a key in
> > the
> > > > cluster/producer/consumer config.  Otherwise the docs have to spell out
> > > > when it is one versus the other.
> > > >
> > > > That is a good point about being able to provide substitution support
> > to
> > > > SASL/GSSAPI (which relies upon login module code that we do not
> > control) if
> > > > we choose the simple, consistent way of doing things up front.
> > > >
> > > > You asked if there are OAuth use cases that require substitutions to be
> > > > performed in a login module that cannot be done if the substitution is
> > > > performed when the configuration is parsed.  I don't think so, no; the
> > > > timing should not matter.
> > > >
> > > > I hadn't thought about the listener prefix issue.  I don't know that
> > area
> > > > of the code very well, but I have looked enough to guess that the
> > > > underlying "originals" map in AbstractConfig is what we would want to
> > use
> > > > when making a reference to something.  It would eliminate the listener
> > > > prefix namespacing confusion if we always refer to the key as
> > originally
> > > > provided.
> > > >
> > > > I'm willing to go with doing substitution once, up-front, at the
> > > > cluster/producer/consumer config level, and supporting substitution for
> > > > JAAS configs only when provided via sasl.jaas.config.  I'm willing to
> > try
> > > > the coding to introduce it at that level -- tentative given my
> > > > unfamiliarity with the code and its subtleties, but willing to try.
> > Let me
> > > > chew on it for a day or two and see what I can make happen.  In case
> > you
> > > > want to try as well, you can pull the current implementation (which I
> > think
> > > > is in good shape and might only need cosmetic/stylistic code review
> > changes
> > > > as opposed to wholesale API adjustments) from the KAFKA-6664 branch of
> > > > https://github.com/rondagostino/kafka.git.
> > > >
> > > > Ron
> > > >
> > > > On Fri, Apr 13, 2018 at 7:49 AM, Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Ron,
> > > > >
> > > > > Thanks for the notes and KIP update.
> > > > >
> > > > > Handling `sasl.jaas.config` as a special case is fine, but it would
> > be
> > > > > better if we can do any substitutions before we create a
> > `Configuration`
> > > > > object rather than expect the login module to do the substitution.
> > That
> > > > > way, we will have a consistent substitution format for all login
> > modules
> > > > > including built-in ones. And since we have users who already have
> > their
> > > > own
> > > > > login modules (before KIP-86), they will benefit from substitution
> > too
> > > > > without adding additional code to the login module, But you have
> > thought
> > > > > about this more in the context of OAuth, so this is more of a
> > question.
> > > > Are
> > > > > there use cases that require substitutions to be performed in a login
> > > > > module that cannot be done if the substitution is performed when the
> > > > > configuration is parsed?
> > > > >
> > > > > The ability to refer to other keys is generally quite useful. But as
> > you
> > > > > said, "*there is a namespacing of sorts going on*". Even with regular
> > > > > configs, we have listener prefix, which is also a "*namespacing of
> > > > sorts"*.
> > > > > Our current config framework doesn't represent these well. As you
> > already
> > > > > noticed before, there is magic that removes prefixes, flattening the
> > > > > namespace. Perhaps that is not an issue if we want to allow
> > references to
> > > > > keys that are in the global namespace (non-listener-prefixed) as
> > well.
> > > > But
> > > > > we probably want to make sure namespaces are handled consistently
> > for `
> > > > > sasl.jaas.config` and other configs.
> > > > >
> > > > >
> > > > >
> > > > > On Tue, Apr 10, 2018 at 3:41 AM, Ron Dagostino <rndgstn@gmail.com>
> > > > wrote:
> > > > >
> > > > > > Hi folks.  I updated KIP 269 to help clarify some of the issues
> > > > mentioned
> > > > > > previously.  In particular, I added a new single-method
> > > > UnderlyingValues
> > > > > > interface to make it clear how data is to be provided to
> > > > > > SubstitutableValues, and I added information about if/how the
> > > > underlying
> > > > > > values might be re-read in case they can potentially change
> > (basically
> > > > an
> > > > > > instance of SubstitutableValues never re-reads anything, so if the
> > > > > > underlying values are expected to change a new instance of
> > > > > > SubstitutableValues must be allocated in order to have any chance
> > of
> > > > > seeing
> > > > > > those changes).  I kept the KIP focused on the same JAAS use case
> > for
> > > > > now,
> > > > > > but these additions/clarifications should help if we want to
> > expand the
> > > > > > scope to cluster/producer/consumer configs.
> > > > > >
> > > > > > Ron
> > > > > >
> > > > > > On Mon, Apr 9, 2018 at 11:22 AM, Ron Dagostino <rndgstn@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > > Hi folks.  Here is a summary of where I think we stand on this
> > KIP
> > > > and
> > > > > > > what I believe it means for how we move forward.
> > > > > > >
> > > > > > >
> > > > > > >    - There is some desire to use substitution more broadly beyond
> > > > just
> > > > > > >    JAAS module options.  Specifically, cluster/producer/consumer
> > > > config
> > > > > > values
> > > > > > >    such as ssl.keystore.password are places where substitution
> > adds
> > > > > value
> > > > > > >    (dormant KIP 76
> > > > > > >    <https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > 76+Enable+getting+password+from+executable+rather+than+
> > > > > > passing+as+plaintext+in+config+files>
> > > > > > >    was an attempt to add value here in the past).
> > > > > > >    - *More broad review of this KIP is needed given the
> > potential for
> > > > > its
> > > > > > >    expanded scope*
> > > > > > >    - If substitution is applied more broadly, then the
> > > > sasl.jaas.config
> > > > > > >    value should not have substitution performed on it at the same
> > > > times
> > > > > > as
> > > > > > >    other cluster/producer/consumer configs; that value should be
> > > > passed
> > > > > > >    unchanged to the login module where substitution can be
> > applied
> > > > > later.
> > > > > > >    - There are some adjustments to this KIP that should be made
> > to
> > > > > > >    reflect the possibility of more broad use:
> > > > > > >
> > > > > > >
> > > > > > >    1. The use of delimiters that trigger a substitution attempt
> > but
> > > > > that
> > > > > > >       fail to parse should result in the text being passed
> > through
> > > > > > unchanged
> > > > > > >       instead of raising an exception
> > > > > > >       2. The application of substitution should generally be on
> > an
> > > > > opt-in
> > > > > > >       basis
> > > > > > >       3. The implicit fact that substitution was associated with
> > a
> > > > > > >       namespace of sorts (i.e. saying that a default came from a
> > > > > > particular key
> > > > > > >       meant a JAAS module option) needs to be made explicit.  The
> > > > > > namespace is
> > > > > > >       defined by the Map that is passed into the
> > > > SubstitutableValues()
> > > > > > constructor
> > > > > > >       4. It is not clear to me if the Map that is passed into the
> > > > > > >       SubstitutableValues() constructor can be relied upon to
> > contain
> > > > > > only String
> > > > > > >       values in the context of cluster/producer/consumer configs.
> > > > The
> > > > > > >       AbstractConfig's so-called "originals" map seems to support
> > > > > values
> > > > > > of type
> > > > > > >       String, Boolean, Password, Integer, Short, Long, Number,
> > List,
> > > > > and
> > > > > > Class.
> > > > > > >       It is not difficult to support non-String values in the Map
> > > > that
> > > > > > is passed
> > > > > > >       to the SubstitutableValues() constructor, so I can
> > explicitly
> > > > add
> > > > > > support
> > > > > > >       for that.
> > > > > > >
> > > > > > > I don't think these changes impact usage in a JAAS context, so
> > they
> > > > do
> > > > > no
> > > > > > > harm to the original use case while increasing the potential for
> > more
> > > > > > broad
> > > > > > > use of the concept.
> > > > > > >
> > > > > > >
> > > > > > > Ron
> > > > > > >
> > > > > > >
> > > > > > > On Sun, Apr 8, 2018 at 8:46 PM, Ron Dagostino <rndgstn@gmail.com
> > >
> > > > > wrote:
> > > > > > >
> > > > > > >> 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 <
> > > > > rajinisivaram@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 <
> > rndgstn@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 <
> > > > > > >>> rajinisivaram@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.reddy@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 <
> > > > > > >>> > rajinisivaram@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 <
> > > > > > >>> rndgstn@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 <
> > > > > > >>> > > > rajinisivaram@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 <
> > > > > > >>> > rndgstn@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 <
> > > > > > >>> > > rndgstn@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/conf
> > > > > > >>> luence/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
> > > > > > >>> > > > > > > > >
> > > > > > >>> > > > > > > >
> > > > > > >>> > > > > > >
> > > > > > >>> > > > > >
> > > > > > >>> > > > >
> > > > > > >>> > > >
> > > > > > >>> > >
> > > > > > >>> >
> > > > > > >>>
> > > > > > >>
> > > > > > >>
> > > > > > >
> > > > > >
> > > > >
> > > >
> >

Mime
View raw message