Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-17 Thread Colin McCabe
On Mon, Apr 16, 2018, at 08:15, Ron Dagostino wrote:
> Hi Rajini.  I think a good, illustrative OAuth example is the situation
> where a Kafka client (whether non-broker or broker, where the latter occurs
> when OAUTHBEARER is the inter-broker SASL mechanism) needs to authenticate
> to the token endpoint to retrieve an access token.  There are actually 2
> potential authentications that may occur.  First, the client image itself
> may authenticate as per https://tools.ietf.org/html/rfc6749#section-2.3,
> and as the spec says, the mechanism can be anything ("The authorization
> server MAY accept any form of client authentication meeting its security
> requirements").  The most common form is a client_id and a client_secret as
> per https://tools.ietf.org/html/rfc6749#section-2.3.1.  There may also be
> (and almost always is) authentication of the entity (usually a human).  In
> the Kafka case there is no human involved, but there still may be an
> identity associated beyond just the image identity, and that would probably
> (though again, not necessarily) take the form of a username and password as
> per https://tools.ietf.org/html/rfc6749#section-4.3.  If the image identity
> alone is sufficient then the client credentials grant may be used as per
> https://tools.ietf.org/html/rfc6749#section-4.4.  The first general point
> to take from this is that OAuth 2 is very flexible, so the amount and type
> of secret material that the producer/consumer/broker will need will vary
> from installation to installation.
> 
> The second point to address is how the login module is to retrieve the
> secret material.  More and more customers are going to orchestrate their
> images in Kubernetes clusters, and secrets are injected via file paths in
> that environment.  So to get the client_id and client_secret (for example),
> or the username and password, or all 4 if that is what the installation
> requires, a login module running in Kubernetes is going to have to read 4
> files.  One way to do that is for the login module to leverage JAAS module
> options that would look like this:
> 
> client_id="$[file=/path/to/secrets/client_id]"
> client_secret="$[file=/path/to/secrets/client_secret]"
> username="$[file=/path/to/secrets/username]"
> password="$[file=/path/to/secrets/password]"
> 
> The code in the login callback handler would then be as follows:
> 
> String clientId = getModuleOption("client_id");
> String clientSecret = getModuleOption("client_secret");
> String username = getModuleOption("username");
> String password = getModuleOption("password");
> 
> It is certainly possible that the login callback handler could just read
> those files directly as opposed to asking for the value of the module
> option and letting the substitution occur.  The module options and code for
> that approach might look like this:
> 
> pathToSecrets="/path/to/secrets/"
> 
> String pathToSecrets = getModuleOption("pathToSecrets");
> String clientId = readFile(pathToSecrets, "client_id");
> String clientSecret = readFile(pathToSecrets, "client_secret");
> String username = readFile(pathToSecrets, "username");
> String password = readFile(pathToSecrets, "password");
> 
> private String readFile(String dirName, String fileName) throws IOException
> {
> // etc...
> }
> 
> The advantage of the first approach where the code simply asks for the
> value of a module option is that it doesn't force a developer to create 4
> files.  In development situations where standalone/IDE as opposed to full
> PROD (e.g. k8s) infrastructure is a real possibility, it is convenient to
> be able to simply hard-code values in the JAAS config; encapsulating the
> retrieval of the values behind the module option facilitates being able to
> do that.  The same code that runs in production will run in DEV.  Otherwise
> the developer has to create the 4 files -- it wouldn't be the end of the
> world, of course, but it is easier to not have to deal with it.
> 
> Substitution doesn't seem to be a critical issue when we look at it like
> this, but I think there are other considerations.
> 
> 
>- There are secret values that can't be stored in Zookeeper even for
>those who migrate to dynamic configuration (password.encoder.secret and
>password.encoder.old.secret). Storage of those values in configuration
>files is likely to be one of the weaker links in the security setup, and
>the possibility of a stronger approach (i.e. injection at runtime,
>abstracted behind a configuration value that does substitution) is
>interesting.

I agree that some password values probably shouldn't be stored with the rest of 
the configuration.  But we don't need configuration values that do substitution 
to solve this, right?  We can just have a different configuration key that 
specifies a file to read some of the passwords out of.  Presumably this file 
would have highly locked down permissions, similar to ssh private keys.

>  The issue of notification of chang

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-16 Thread Ron Dagostino
Hi Rajini.  I think a good, illustrative OAuth example is the situation
where a Kafka client (whether non-broker or broker, where the latter occurs
when OAUTHBEARER is the inter-broker SASL mechanism) needs to authenticate
to the token endpoint to retrieve an access token.  There are actually 2
potential authentications that may occur.  First, the client image itself
may authenticate as per https://tools.ietf.org/html/rfc6749#section-2.3,
and as the spec says, the mechanism can be anything ("The authorization
server MAY accept any form of client authentication meeting its security
requirements").  The most common form is a client_id and a client_secret as
per https://tools.ietf.org/html/rfc6749#section-2.3.1.  There may also be
(and almost always is) authentication of the entity (usually a human).  In
the Kafka case there is no human involved, but there still may be an
identity associated beyond just the image identity, and that would probably
(though again, not necessarily) take the form of a username and password as
per https://tools.ietf.org/html/rfc6749#section-4.3.  If the image identity
alone is sufficient then the client credentials grant may be used as per
https://tools.ietf.org/html/rfc6749#section-4.4.  The first general point
to take from this is that OAuth 2 is very flexible, so the amount and type
of secret material that the producer/consumer/broker will need will vary
from installation to installation.

The second point to address is how the login module is to retrieve the
secret material.  More and more customers are going to orchestrate their
images in Kubernetes clusters, and secrets are injected via file paths in
that environment.  So to get the client_id and client_secret (for example),
or the username and password, or all 4 if that is what the installation
requires, a login module running in Kubernetes is going to have to read 4
files.  One way to do that is for the login module to leverage JAAS module
options that would look like this:

client_id="$[file=/path/to/secrets/client_id]"
client_secret="$[file=/path/to/secrets/client_secret]"
username="$[file=/path/to/secrets/username]"
password="$[file=/path/to/secrets/password]"

The code in the login callback handler would then be as follows:

String clientId = getModuleOption("client_id");
String clientSecret = getModuleOption("client_secret");
String username = getModuleOption("username");
String password = getModuleOption("password");

It is certainly possible that the login callback handler could just read
those files directly as opposed to asking for the value of the module
option and letting the substitution occur.  The module options and code for
that approach might look like this:

pathToSecrets="/path/to/secrets/"

String pathToSecrets = getModuleOption("pathToSecrets");
String clientId = readFile(pathToSecrets, "client_id");
String clientSecret = readFile(pathToSecrets, "client_secret");
String username = readFile(pathToSecrets, "username");
String password = readFile(pathToSecrets, "password");

private String readFile(String dirName, String fileName) throws IOException
{
// etc...
}

The advantage of the first approach where the code simply asks for the
value of a module option is that it doesn't force a developer to create 4
files.  In development situations where standalone/IDE as opposed to full
PROD (e.g. k8s) infrastructure is a real possibility, it is convenient to
be able to simply hard-code values in the JAAS config; encapsulating the
retrieval of the values behind the module option facilitates being able to
do that.  The same code that runs in production will run in DEV.  Otherwise
the developer has to create the 4 files -- it wouldn't be the end of the
world, of course, but it is easier to not have to deal with it.

Substitution doesn't seem to be a critical issue when we look at it like
this, but I think there are other considerations.


   - There are secret values that can't be stored in Zookeeper even for
   those who migrate to dynamic configuration (password.encoder.secret and
   password.encoder.old.secret). Storage of those values in configuration
   files is likely to be one of the weaker links in the security setup, and
   the possibility of a stronger approach (i.e. injection at runtime,
   abstracted behind a configuration value that does substitution) is
   interesting.  The issue of notification of changes as previously discussed
   does not apply here because these values are only used at broker startup.
   - My guess is that migration to dynamic configuration and/or KIP-86
   callback handler replacement will proceed piecemeal with some customers
   waiting much longer than others -- and some may never migrate to one or
   both.  There are going to be people who will appreciate the ability to
   abstract out a retrieval mechanism behind a configuration value (whether
   that be a JAAS module option or a producer/consumer/broker config).  The
   issue of notification of changes as previously discus

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-16 Thread Rajini Sivaram
Hi Ron,

Thanks for the analysis, this is very useful. Reducing the feature to the
minimum required for the scenarios helps (though I was hoping that the
redact flag would one day help with improving SASL diagnostics, that can be
for another day).

In the past when users requested different password storing mechanisms like
in KIP-76, the main reason was to avoid storing clear passwords in a file.
Now that we can store encrypted passwords in ZK, that is less of a
motivation for SSL keystore passwords (at least on the broker-side). But
integrating with external password vaults is perhaps still useful for SASL.
But an important point is that none of the built-in substitution mechanisms
helps with this security requirement.

Colin had suggested that it would be better to do this type of substitution
outside of Kafka, e.g. using scripts. So if we can store encrypted
passwords in ZooKeeper, users can write scripts to extract passwords from
their favourite password vault and store them encrypted in ZK, do we still
need substitution?

I think there are two use cases for SASL:

   1. Client configuration (Producer/Consumer/AdminClient)
   2. Dynamic password updates with caching (as you mentioned) for both
   clients and brokers

Looking at only SASL, we can do 1) with SASL callback handlers (KIP-86) for
both clients and brokers. Callback handlers can do 2) as well. On the
broker-side, as you mentioned, dynamic configs in ZK would be better for 2)
(we don't currently support changing SASL configs for existing listeners,
but eventually we probably would).

So if we assume that SSL passwords can be handled by encrypted passwords in
ZK, SASL/PLAIN and SCRAM can be handled by callback handlers, can we narrow
down the OAuth scenarios that really benefit from substitution? Do we need
specific built-in substitution mechanisms for these scenarios given that
any built-in substitution mechanism would be inherently insecure? Do we
have to support custom substitution mechanisms for these? Perhaps an
example would help since most of us are not that familiar with the
different OAuth options and you have already thought this through in the
context of OAuth?

Once we identify what we need for OAuth, we could go back and look at
whether this would also help other SASL mechanisms, SSL passwords, other
configs etc.

Thank you,

Rajini



On Sun, Apr 15, 2018 at 3:54 AM, Ron Dagostino  wrote:

> I am unsure if substitution should be supported for just JAAS configs or if
> we should allow it for cluster/broker/consumer configs.  What I think would
> be helpful would be to boil down this proposal to its most essential
> requirement, and I think the discussion has helped us arrive at what that
> looks like: an ability to inject secrets from external sources.
> SASL/OAUTHBEARER (KIP 255) requires this, and there is value for other SASL
> mechanisms at the JAAS config level as described in this KIP.  If we focus
> on just this functionality, and keep in mind the feedback about
> configuration getting out of hand, then we likely arrive at the following
> conclusions:
>
>- *If* we allow substitution in producer/consumer/broker configs (and
>again, I am not sure if this is a good idea yet) then we would only
> allow
>it for configs of type Password (which includes sasl.jaas.config); since
>these are by definition sensitive values, it is redundant to specify
>redact, and therefore redact is no longer needed as an explicit modifier
>-- it is always in effect.
>- We can and should eliminate dependencies by removing the ability to
>refer to another configuration value; this means defaultKey and
>fromValueOfKey are no longer needed as modifiers, and the keyValue
>substitution type goes away.
>- It doesn't seem reasonable to allow an empty or blank secret to be
>injected, which means notBlank and notEmpty are redundant and
>unnecessary modifiers.
>- A default value wouldn't seem to make sense with secret injection, so
>we don't need the defaultValue modifier.
>
> There are still some outstanding issues.
>
> Are substituted values cached (and if so, for how long?) or are they
> recalculated each time they are requested?  For example, if a password
> comes from some external source (file, password vault, etc.), is that
> external source queried every time the password is required, or is queried
> once and then always reused, or is it queried once and then reused for some
> amount of time before the external source is queried again?  Compare this
> to dynamic configuration as implemented via KIP 226, which is a
> push/event-driven mechanism via kafka-configs.sh, Zookeeper, and the
> Reconfigurable interface.  Changes to substituted values as proposed in
> this KIP are not push/event-driven; as proposed it will be up to Kafka to
> somehow check for changes, or to set a cache timeout as described above.
> This is clearly inferior to the push/event-driven mechanism provided by
> dynamic configur

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-14 Thread Ron Dagostino
I am unsure if substitution should be supported for just JAAS configs or if
we should allow it for cluster/broker/consumer configs.  What I think would
be helpful would be to boil down this proposal to its most essential
requirement, and I think the discussion has helped us arrive at what that
looks like: an ability to inject secrets from external sources.
SASL/OAUTHBEARER (KIP 255) requires this, and there is value for other SASL
mechanisms at the JAAS config level as described in this KIP.  If we focus
on just this functionality, and keep in mind the feedback about
configuration getting out of hand, then we likely arrive at the following
conclusions:

   - *If* we allow substitution in producer/consumer/broker configs (and
   again, I am not sure if this is a good idea yet) then we would only allow
   it for configs of type Password (which includes sasl.jaas.config); since
   these are by definition sensitive values, it is redundant to specify
   redact, and therefore redact is no longer needed as an explicit modifier
   -- it is always in effect.
   - We can and should eliminate dependencies by removing the ability to
   refer to another configuration value; this means defaultKey and
   fromValueOfKey are no longer needed as modifiers, and the keyValue
   substitution type goes away.
   - It doesn't seem reasonable to allow an empty or blank secret to be
   injected, which means notBlank and notEmpty are redundant and
   unnecessary modifiers.
   - A default value wouldn't seem to make sense with secret injection, so
   we don't need the defaultValue modifier.

There are still some outstanding issues.

Are substituted values cached (and if so, for how long?) or are they
recalculated each time they are requested?  For example, if a password
comes from some external source (file, password vault, etc.), is that
external source queried every time the password is required, or is queried
once and then always reused, or is it queried once and then reused for some
amount of time before the external source is queried again?  Compare this
to dynamic configuration as implemented via KIP 226, which is a
push/event-driven mechanism via kafka-configs.sh, Zookeeper, and the
Reconfigurable interface.  Changes to substituted values as proposed in
this KIP are not push/event-driven; as proposed it will be up to Kafka to
somehow check for changes, or to set a cache timeout as described above.
This is clearly inferior to the push/event-driven mechanism provided by
dynamic configuration, and I am not sure what to do with that statement,
but there it is.  SASL/OAUTHBEARER requires flexibility due to the flexible
nature of the OAuth 2 framework, and I can't imagine forcing the use of
dynamic configuration for secret injection on SASL/OAUTHBEARER
implementations (or for the SASL/PLAIN password use case taken as
motivation for this KIP).  But should we provide a mechanism at the
producer/consumer/broker configuration level that is not push/event-driven
given that dynamic configuration is push/event-driven?  Maybe there is
value in retrieving the password.encoder.secret and
password.encoder.old.secret values via substitution.  Maybe there is also
value in supporting substitution for producer/consumer/broker configs that
are of type Password despite the fact that they can be managed
dynamically.  I don't know.

Anyway, I think this feature can be boiled down to its essentials as
described above, and the last steps are to decide if the feature should be
used at the producer/consumer/broker config level or not; what supported
mechanisms should exist out-of-the-box (file?  environment variable?
system property?); and whether we should allow the addition of
custom-written substitution types.

Ron


On Sat, Apr 14, 2018 at 12:18 AM, Manikumar 
wrote:

> 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  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 

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Manikumar
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  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 
> 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

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Colin McCabe
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  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.

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Ron Dagostino
Thanks, Colin, for your comments.  I appreciate the point that substitution
can be over-applied.  I did not have a specific requirement for the
defaultKey= and fromValueOfKey modifiers; I included them because they
intuitively felt like they would be useful and made the feature more
powerful.  Given a lack of a hard requirement and your well-taken point
about dependency graphs potentially causing problems, I wonder if
eliminating the ability to refer to other keys by removing the
defaultKey= and fromValueOfKey modifiers would be wise.  It would keep
things simple while still providing the ability to inject configuration.

How this feature coexists with dynamic configuration is worth more
discussion.  I recently specified that values are not read twice:

"Once an instance of SubstitutableValues retrieves an underlying value and
its calculated value -- whether different from the raw underlying value due
to a substitution or not -- is determined, the instance of
SubstitutableValues will not retrieve the underlying value again; the
calculated value will be used if it is referred to. This means if the
underlying values are expected to change then to see those changes a
new instance of SubstitutableValues must be allocated."

I stated this because of the dependency graph issue you asked about:

"If one configuration key changes because it is a dynamic configuration
key, should all the configuration keys that included that one change as
well?"

My solution was that if anything changed, and you wanted to see those
changes, you had to invalidate everything and start from the beginning
again because the implementation was not going to track the dependencies.

If we eliminate dependencies then I think we have more flexibility in
dealing with the possibility that underlying configuration might change.
It becomes allowable to re-calculate a value, for example.  There are still
some subtleties -- when is a value to be recalculated?  Should it be done
explicitly, so we have to ask for it to be recalculated?  Should we allow a
value to be tagged with a "lifetime" value so a calculated value can be
discarded after a certain amount of time?  I don't know what the right
answer is, but eliminating dependencies does give us more options here.

Ron





On Fri, Apr 13, 2018 at 1:30 PM, 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?
>
>
> On Fri, Apr 13, 2018 at 5:56 PM, Colin McCabe  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 

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Rajini Sivaram
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?


On Fri, Apr 13, 2018 at 5:56 PM, Colin McCabe  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 
> 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 suppo

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Colin McCabe
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  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;

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Rajini Sivaram
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  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 
> 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 
> wrote:
> >
> > > Hi folks.  I updated KIP 269 to help clarify some of the issues
> mentioned
> > > previously.  In particular, I added a new single-method
> 

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Ron Dagostino
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 
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  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/

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-13 Thread Rajini Sivaram
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  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  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
> > 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  wrote:
> >
> >> Hi Rajini.  I've also been thi

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-09 Thread Ron Dagostino
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  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
>
> 
>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  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= 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

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-09 Thread Ron Dagostino
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
   

   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  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= 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 h

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-08 Thread Ron Dagostino
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= 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 
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  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

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-08 Thread Rajini Sivaram
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  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 

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-06 Thread Ron Dagostino
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 
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 
> 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 
> > 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

Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-06 Thread Rajini Sivaram
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  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 
> 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 
> 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 
> > > 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 
> > > > wrote:
> > > > >
> > > > > > Hi everyone.
> > > > > >
> > > > > > I created KIP-269: Substitution Within Configuration Values
> > > > > >  > > > > 269+Substitution+Within+Configuration+Values>
> > > > > > (https://cwiki.apache.org/confluence/display/KAFKA/KIP+269+
> > > > > > Substitution+Within+Configuration+Values
> > > > > >  > > > > 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
> > > > > >  > > > > action?pageId=75968876>
> > > > > > based on discussion of that KIP.
> > > > > >
> > > > > > Ron
> > > > > >
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-05 Thread Manikumar
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 
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  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 
> > 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 
> > 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 
> > > wrote:
> > > >
> > > > > Hi everyone.
> > > > >
> > > > > I created KIP-269: Substitution Within Configuration Values
> > > > >  > > > 269+Substitution+Within+Configuration+Values>
> > > > > (https://cwiki.apache.org/confluence/display/KAFKA/KIP+269+
> > > > > Substitution+Within+Configuration+Values
> > > > >  > > > 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
> > > > >  > > > action?pageId=75968876>
> > > > > based on discussion of that KIP.
> > > > >
> > > > > Ron
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-05 Thread Rajini Sivaram
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  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 
> 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 
> 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 
> > wrote:
> > >
> > > > Hi everyone.
> > > >
> > > > I created KIP-269: Substitution Within Configuration Values
> > > >  > > 269+Substitution+Within+Configuration+Values>
> > > > (https://cwiki.apache.org/confluence/display/KAFKA/KIP+269+
> > > > Substitution+Within+Configuration+Values
> > > >  > > 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
> > > >  > > action?pageId=75968876>
> > > > based on discussion of that KIP.
> > > >
> > > > Ron
> > > >
> > >
> >
>


Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-05 Thread Ron Dagostino
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 
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  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 
> wrote:
> >
> > > Hi everyone.
> > >
> > > I created KIP-269: Substitution Within Configuration Values
> > >  > 269+Substitution+Within+Configuration+Values>
> > > (https://cwiki.apache.org/confluence/display/KAFKA/KIP+269+
> > > Substitution+Within+Configuration+Values
> > >  > 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
> > >  > action?pageId=75968876>
> > > based on discussion of that KIP.
> > >
> > > Ron
> > >
> >
>


Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-04-05 Thread Rajini Sivaram
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  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  wrote:
>
> > Hi everyone.
> >
> > I created KIP-269: Substitution Within Configuration Values
> >  269+Substitution+Within+Configuration+Values>
> > (https://cwiki.apache.org/confluence/display/KAFKA/KIP+269+
> > Substitution+Within+Configuration+Values
> >  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
> >  action?pageId=75968876>
> > based on discussion of that KIP.
> >
> > Ron
> >
>


Re: [DISCUSS] KIP-269: Substitution Within Configuration Values

2018-03-28 Thread Ron Dagostino
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  wrote:

> Hi everyone.
>
> I created KIP-269: Substitution Within Configuration Values
> 
> (https://cwiki.apache.org/confluence/display/KAFKA/KIP+269+
> Substitution+Within+Configuration+Values
> 
> ).
>
> 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
> 
> based on discussion of that KIP.
>
> Ron
>


[DISCUSS] KIP-269: Substitution Within Configuration Values

2018-03-15 Thread Ron Dagostino
Hi everyone.

I created KIP-269: Substitution Within Configuration Values

(https://cwiki.apache.org/confluence/display/KAFKA/KIP+
269+Substitution+Within+Configuration+Values

).

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

based on discussion of that KIP.

Ron