My gut says a building block that takes a configuration as input and
exposes a representation of the potential issues found would be a good
starting point. Then this could be leveraged as part of a command line
tool that emits the data to stdout, or it could be used during broker
startup such that any potential issues found are logged (or, more
aggressively, cause the broker to not start if that is desired). The
return could be something as simple as an
EnumSet<PotentialConfigSecurityIssue>:
enum PotentialConfigSecurityIssue {
OAUTHBEARER_UNSECURE_TOKENS_ALLOWED, // when OAUTHBEARER is in
sasl.enabled.mechanisms but server callback handler is the unsecured
validator
PLAINTEXT_LISTENER, // if there is a PLAINTEXT listener
SSL_CLIENT_AUTH_NOT_REQUIRED, // when SSL listener is enabled but
ssl.client.auth is not required
ETC
}
Or it could be a Set of instances where each instance implements an
interface that returns one of these enum values via a getType() method;
this allows each instance to potentially be a different class and hold
additional information (like the PLAINTEXT listener's address in case that
is something that requires additional validation, though how the validation
would be plugged in is a separate issue that I can't figure out at the
moment).
It feels to me that the EnumSet<> approach is the simplest place to start,
and it might be best to allow anything more complicated to fall out as
experience with the simple starting point builds. Making the EnumSet<>
part of the public API would then allow anyone to build upon it as they see
fit.
Ron
On Thu, Feb 21, 2019 at 12:57 PM Stanislav Kozlovski <stanis...@confluent.io>
wrote:
> I think that is a solid idea. The closest thing I can think of is David's
> PR about duplicate config key logging -
> https://github.com/apache/kafka/pull/6104
>
> We could either continue the pattern of checking on broker startup and
> logging a warning or create a separate tool that analyzes the configs.
>
> On Thu, Feb 21, 2019 at 3:16 PM Rajini Sivaram <rajinisiva...@gmail.com>
> wrote:
>
> > Hi Ron,
> >
> > Yes, a security sanity check tool could be quite useful. Let's see what
> > others think.
> >
> > Thanks,
> >
> > Rajini
> >
> >
> > On Thu, Feb 21, 2019 at 1:49 PM Ron Dagostino <rndg...@gmail.com> wrote:
> >
> > > HI Rajini and Stan. Thanks for the feedback.
> > >
> > > Stan, regarding the proposed config name, I couldn't think of anything
> > so I
> > > just threw in something outrageous in the hopes that it would give a
> > sense
> > > of what I was talking about while perhaps making folks chuckle a bit.
> > >
> > > Rajini, I definitely see your point. It probably doesn't make sense to
> > > address this one particular issue (if we can even consider it an issue)
> > > when in fact it is part of a pattern that has been explicitly agreed
> upon
> > > as being appropriate.
> > >
> > > Maybe a security sanity check tool that scans the config and flags any
> of
> > > these items you mentioned, plus the OAUTHBEARER one and any others we
> can
> > > think of, would be useful? That way the out-of-the-box experience can
> > > remain straightforward while some of the security risk that comes as a
> > > byproduct can be mitigated.
> > >
> > > Ron
> > >
> > > Ron
> > >
> > > On Thu, Feb 21, 2019 at 8:02 AM Rajini Sivaram <
> rajinisiva...@gmail.com>
> > > wrote:
> > >
> > > > Hi Ron,
> > > >
> > > > Thanks for the KIP. How is this different from other scenarios:
> > > >
> > > > 1. Our default is to use a PLAINTEXT listener. If you forget to
> > change
> > > > that, anyone has access to your cluster
> > > > 2. You may add a PLAINTEXT listener to the list of listeners in
> > > > production. May be you intended it for an interface that was
> > protected
> > > > using network segmentation, but entered the wrong address.
> > > > 3. You are very security conscious and add an SSL listener. You
> must
> > > be
> > > > secure now right? Our default is `ssl.client.auth=none`, which
> means
> > > any
> > > > one can connect.
> > > > 4. You use the built-in insecure PLAIN callback that stores
> > cleartext
> > > > passwords on the file system. Or enable SASL/PLAIN without SSL.
> > > >
> > > > At the moment, our defaults are intended to make it easy to get
> started
> > > > quickly. If we want to make brokers secure by default, we need an
> > > approach
> > > > that works across the board. I am not sure we have a specific issue
> > with
> > > > OAUTHBEARER apart from the fact that we don't provide a secure
> > > alternative.
> > > >
> > > >
> > > >
> > > > On Thu, Feb 21, 2019 at 12:05 PM Stanislav Kozlovski <
> > > > stanis...@confluent.io>
> > > > wrote:
> > > >
> > > > > Hey Ron, thanks for the KIP.
> > > > >
> > > > > I believe the proposed configuration setting
> > > > > `yes.virginia.i.really.do
> > > > >
> > > >
> > >
> >
> .want.to.allow.unsecured.oauthbearer.tokens.because.this.is.not.a.production.cluster`
> > > > > might be too verbose. I acknowledge that we do not want to enable
> > this
> > > in
> > > > > production but we could maybe compromise on a more normal name.
> > > > >
> > > > > I am wondering whether it would be more worth it to replace the
> > default
> > > > > implementation with a secure one. Disabling it by default can be
> seen
> > > as
> > > > > just kicking the can down the road
> > > > >
> > > > > Best,
> > > > > Stanislav
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Feb 20, 2019 at 5:31 PM Ron Dagostino <rndg...@gmail.com>
> > > wrote:
> > > > >
> > > > > > Hi everyone. I created KIP-432: Additional Broker-Side Opt-In for
> > > > > Default,
> > > > > > Unsecure SASL/OAUTHBEARER Implementation
> > > > > > <
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=103091238
> > > > > > >
> > > > > > (
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=103091238
> > > > > > ).
> > > > > > The motivation for this KIPis as follows:
> > > > > >
> > > > > > The default implementation of SASL/OAUTHBEARER, as per KIP-255
> > > > > > <
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=75968876
> > > > > > >,
> > > > > > is unsecured. This is useful for development and testing
> purposes,
> > > and
> > > > > it
> > > > > > provides a great out-of-the-box experience, but it must not be
> used
> > > in
> > > > > > production because it allows the client to authenticate with any
> > > > > principal
> > > > > > name it wishes. To enable the default unsecured SASL/OAUTHBEARER
> > > > > > implementation on the broker side simply requires the addition of
> > > > > > OAUTHBEARER to the sasl.enabled.mechanisms configuration value
> (for
> > > > > > example:
> > > > > > sasl.enabled.mechanisms=GSSAPI,OAUTHBEARER instead of simply
> > > > > > sasl.enabled.mechanisms=GSSAPI). To secure the implementation
> > > requires
> > > > > the
> > > > > > explicit setting of the
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> listener.name.{sasl_plaintext|sasl_ssl}.oauthbearer.sasl.{login,server}.callback.handler.class
> > > > > > properties on the broker. The question then arises: what if
> > someone
> > > > > > either accidentally or maliciously appended OAUTHBEARER to the
> > > > > > sasl.enabled.mechanisms configuration value? Doing so would
> enable
> > > the
> > > > > > unsecured implementation on the broker, and clients could then
> > > > > authenticate
> > > > > > with any principal name they desired.
> > > > > >
> > > > > > This KIP proposes to add an additional opt-in configuration
> > property
> > > on
> > > > > the
> > > > > > broker side for the default, unsecured SASL/OAUTHBEARER
> > > implementation
> > > > > such
> > > > > > that simply adding OAUTHBEARER to the sasl.enabled.mechanisms
> > > > > configuration
> > > > > > value would be insufficient to enable the feature. This
> additional
> > > > > opt-in
> > > > > > broker configuration property would have to be explicitly set to
> > true
> > > > > > before the default unsecured implementation would successfully
> > > > > authenticate
> > > > > > users, and the name of this configuration property would
> explicitly
> > > > > > indicate that the feature is not secure and must not be used in
> > > > > > production. Adding this explicit opt-in is a breaking change;
> > > existing
> > > > > > uses of the unsecured implementation would have to update their
> > > > > > configuration to include this explicit opt-in property before
> their
> > > > > cluster
> > > > > > would accept unsecure tokens again. Note that this would only
> > result
> > > > in
> > > > > a
> > > > > > breaking change in production if the unsecured feature is either
> > > > > > accidentally or maliciously enabled there; it is assumed that 1)
> > this
> > > > > will
> > > > > > probably not happen to anyone; and 2) if it does happen to
> someone
> > it
> > > > > > almost certainly would not impact sanctioned clients but would
> > > instead
> > > > > > impact malicious clients only (if there were any).
> > > > > >
> > > > > >
> > > > > > Ron
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best,
> > > > > Stanislav
> > > > >
> > > >
> > >
> >
>
>
> --
> Best,
> Stanislav
>
