We can limit substitution mechanism only for password config types and JAAS config. We may not want to use to for all config properties.
On Sat, Apr 14, 2018 at 9:21 AM, Colin McCabe <cmcc...@apache.org> wrote: > 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 <cmcc...@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 <rndg...@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 < > > > rajinisiva...@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 < > rndg...@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 < > rndg...@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 < > rndg...@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 < > > > > > > 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/confl > uence/display/KAFKA/KIP- > > > > > > > >>> > > > 76+Enable+getting+password+fro > m+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+Config > uration+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 > > > > > > > >>> > > > > > > > > > > > > > > > >>> > > > > > > > > > > > > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > >>> > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
