Hi Colin,

quick summary up front: I totally agree, and always have! I think we
misunderstood each other a little :)
I was never really opposed the idea of restricting which ACL features can
be used, I was just opposed to doing it specifically for this change, while
not introducing real versioning. But when I suggested a few things around
how to implement versioning back in January [1] and got no replies on those
suggestions I figured no one was really interested in that, so I fell back
to the minimal idea of not doing anything about it. I am absolutely happy
to pick this up.

I've done some more digging through the code and feel plenty stupid at the
moment, because there actually already is an ACL version [2]. Then again,
good news, we can just use this. Currently loading ACLs from Zookeeper
fails if the hard-coded version in the authorizer differs from what is
stored in Zookeeper [3].
Following your idea of using the inter-broker protocol, we probably need to
create a mapping of IBP -> ACL version somewhere, so we would have
something like ACL_2 -> KAFKA_2_3 which defines that this version requires
the IBP to be at least the stated version. When we make a change that
requires a new ACL version, we simply add a new mapping to the appropriate
API version and change the hard-coded version value in the authorizer.
One potential issue that I see with this is that I am unsure if the
Authorizer will actually have access to the configured IBP version when it
is running from the kafka-acls.sh tool. I'll look for a way, but I think
this might actually not be possible, in "legacy" mode, when writing ACLs
directly to Zookeeper.

The biggest take away that I have from this is, that we will probably need
to change the zk path after all. If we do not do this, older brokers using
the ACLAuthorizer will simply fail to start if someone creates an ACL with
a newer version. And I am not sure we can actually prohibit them from doing
that, as mentioned above.

Maybe an alternative would be to keep the current logic as is, i.e. fail on
presence of an ACL version != current version and add a "migrate" command
to the acl commandline tool - at the cost of not being able to downgrade
again, unless we make the migrate tool bi-directional..

I am unsure of the best way to be honest, will do some more looking through
the code and thinking. But thought I'd share this in case you have some
thoughts.

Best regards,
Sönke

[1]
https://lists.apache.org/thread.html/4936d5205852d0db19653d17aa9821d4ba30f077aee528bb90955ce4@%3Cdev.kafka.apache.org%3E
[2]
https://github.com/apache/kafka/blob/b4fa87cc51f0a7d640dc6ae2cc8f89f006aae652/core/src/main/scala/kafka/security/auth/Acl.scala#L37
[3]
https://github.com/apache/kafka/blob/b4fa87cc51f0a7d640dc6ae2cc8f89f006aae652/core/src/main/scala/kafka/security/auth/Acl.scala#L66


On Fri, Apr 5, 2019 at 8:09 AM Colin McCabe <cmcc...@apache.org> wrote:

> On Thu, Apr 4, 2019, at 01:52, Sönke Liebau wrote:
> > Hi Colin,
> >
> > I agree, we need to have a way of failing incorrect ranges server-side,
> > I'll amend the KIP and look into that. I think INVALID_REQUEST should fit
> > fine, afaik we can send a message along with that code, so that could
> > explain the actual reason.
>
> Hi Sonke,
>
> Sounds good.
>
> >
> > Regarding prohibiting these ACLs from being created before the
> inter-broker
> > protocol is updated, I am a bit hesitant about this for two reasons.
> >
> > 1. I can't help but feel that we are mixing things in with each other
> here
> > that don't really belong together. The broker protocol and ACL versions
> and
> > potential ACL versioning are to my mind only loosely linked, because
> broker
> > and authorizer are usually updated at the same time. But if we look at
> > Sentry, Ranger or the Confluent LDAP authorizer I am not sure that this
> > will hold true forever.
>
> I strongly doubt that anyone actually wants to update the authorizer
> plugins on a separate schedule than the brokers. I worked on Hadoop for a
> while, so I have some perspective on this. Even in an upgrade scenario, you
> have to bounce the broker process at some point, and at the point, it's not
> clear that you gain anything except a headache by bringing it up again with
> the old authorizer code and the new broker code.
>
> > Also, this doesn't address the issue of potential future ACL changes that
> > might actually be incompatible with existing ACLs. If we go down this
> road
> > I think we should go all the way and come up with a full ACL
> > versioning-scheme. I've written down a few thoughts/suggestions/questions
> > on that in this thread.
>
> I think an ACL versioning scheme is a good idea, but mainly just so that
> we can provide clean error messages when someone with old software tries to
> access ACLs in the new format. Right now, we have no way to evolve the
> format of the data in ZK at all, which is why we used the hack of creating
> a new path under ZK to implement prefix ACLs.
>
> I think it makes sense to "gate" using the new ACL versions on having a
> specific value of the inter-broker-protocol configured. The thing to keep
> in mind is that the IBP should not be bumped until all brokers have been
> upgraded. Once all brokers have been upgraded, using the new ACL version
> should be OK.
>
> > 2. I agree that not preventing this creates a security risk, however in a
> > very limited scenario.
> > The same scenario applied when we introduced prefixed-ACLs in version 2
> and
> > back then it was not addressed as far as I can tell. The only reference I
> > can find is one sentence in the KIP that states the risk.
> > For people using the Confluent LDAP authorizer (or any other authorizer
> > that is based on the SimpleACLAuthorizer) there is not much we can do, as
> > once the cluster is updated these ACLs could be created, but the
> > Authorizers would only honor them after they themselves are upgraded
> (which
> > will probably be a rolling restart as well, hence have the same issue).
>
> That is a fair point-- in both scenarios, DENY ACLs could cause problems.
>
> However, it seems much more likely that people will set up DENY ACLs on a
> range of IPs than on a topic prefix. Prefix-based security is mainly used
> to grant permissions, not to revoke them. For example, you might grant
> someone permission to create topics under "foo_" so that their KSQL jobs
> can run smoothly, etc. On the other hand, it is natural to think in terms
> of excluding certain IPs -- iptables is often used this way.
>
> Prefix ACLs also exist under a separate ZK path, which at least avoids
> brokers crashing or getting exceptions if someone creates a prefix ACL and
> the cluster is partially upgraded.
>
> Finally, there was a lot of discussion during the prefix ACLs KIP about
> the need to fix the versioning issue.
>
> >
> > I am really not trying to duck the work here, but just addressing this
> > specific change feels like it misses the larger issue to me.
>
> I think you make some reasonable points. But we really should not keep
> kicking the can down the road.
>
> Let's just create a JSON structure for ACLs in ZK, similar to the other
> JSON structures we have there. Put it under a new ZK root if we must. Call
> what exists today version 0, and make it so that you can't use version 1
> (the first JSON version) until upgrading the IBP. What do you think?
>
> best,
> Colin
>
> >
> > Best regards,
> > Sönke
> >
> > On Wed, Apr 3, 2019 at 4:36 PM Colin McCabe <cmcc...@apache.org> wrote:
> >
> > > Hi Sönke,
> > >
> > > Maybe a reasonable design here would be to not allow creating ACLs
> based
> > > on ip ranges and subnets unless the inter-broker protocol setting has
> been
> > > upgraded. If an upgrade is done correctly, the IBP should not be
> upgraded
> > > until all the brokers have been upgraded, so there shouldn't be older
> > > brokers in the cluster erroneously giving access to things they
> shouldn't.
> > > In that case, perhaps we can hold off on introducing an ACL versioning
> > > scheme for now.
> > >
> > > Another thing that is important here is having some way of rejecting
> > > malformed ip address ranges in the CreateAcls call. This is probably
> not
> > > too difficult, but it should be spelled out. We could use
> INVALID_REQUEST
> > > as the error code for this situation, or maybe create a new one to be
> more
> > > specific.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Wed, Apr 3, 2019, at 04:58, Sönke Liebau wrote:
> > > > All,
> > > >
> > > > as this thread has now been dormant for about three months again
> I'll am
> > > > willing to consider the attempt at looking at a larger versioning
> scheme
> > > > for ACLs as failed.
> > > >
> > > > I am away for a long weekend tomorrow and will start a [VOTE] thread
> on
> > > > implementing this as is on Monday, as I personally consider the
> security
> > > > implications of these ACLs in a mixed version cluster quite minimal
> and
> > > > addressable via the release notes.
> > > >
> > > > Best,
> > > > Sönke
> > > >
> > > > On Sat, Mar 16, 2019 at 1:32 PM Sönke Liebau <
> soenke.lie...@opencore.com
> > > >
> > > > wrote:
> > > >
> > > > > Just a quick bump, as this has been quiet for a while again.
> > > > >
> > > > > On Tue, Jan 8, 2019 at 12:44 PM Sönke Liebau <
> > > soenke.lie...@opencore.com>
> > > > > wrote:
> > > > >
> > > > >> Hi Colin,
> > > > >>
> > > > >> thanks for your response!
> > > > >>
> > > > >> in theory we could get away without any additional path changes I
> > > > >> think.. I am still somewhat unsure about the best way of
> addressing
> > > > >> this. I'll outline my current idea and concerns that I still have,
> > > > >> maybe you have some thoughts on it.
> > > > >>
> > > > >> ACLs are currently stored in two places in ZK: /kafka-acl and
> > > > >> /kafka-acl-extended based on whether they make use of prefixes or
> not.
> > > > >> The reasoning[1] for this is not fundamentally changed by
> anything we
> > > > >> are discussing here, so I think that split will need to remain.
> > > > >>
> > > > >> ACLs are then stored in the form of a json array:
> > > > >> [zk: 127.0.0.1:2181(CONNECTED) 9] get /kafka-acl/Topic/*
> > > > >>
> > > > >>
> > >
> {"version":1,"acls":[{"principal":"User:sliebau","permissionType":"Allow","operation":"Read","host":"*"},{"principal":"User:sliebau","permissionType":"Allow","operation":"Describe","host":"*"},{"principal":"User:sliebau2","permissionType":"Allow","operation":"Describe","host":"*"},{"principal":"User:sliebau2","permissionType":"Allow","operation":"Read","host":"*"}]}
> > > > >>
> > > > >> What we could do is add a version property to the individual ACL
> > > > >> elements like so:
> > > > >> [
> > > > >> {
> > > > >> "principal": "User:sliebau",
> > > > >> "permissionType": "Allow",
> > > > >> "operation": "Read",
> > > > >> "host": "*",
> > > > >> "acl_version": "1"
> > > > >> }
> > > > >> ]
> > > > >>
> > > > >> We define the current state of ACLs as version 0 and the
> Authorizer
> > > > >> will default a missing "acl_version" element to this value for
> > > > >> backwards compatibility. So there should hopefully be no need to
> > > > >> migrate existing ACLs (concerns notwithstanding, see later).
> > > > >>
> > > > >> Additionally the authorizer will get a max_supported_acl_version
> > > > >> setting which will cause it to ignore any ACLs larger than what
> is set
> > > > >> here, hence allowing for controlled upgrading similar to the
> process
> > > > >> using inter broker protocol version. If this happens we should
> > > > >> probably log a warning in case this was unintentional. Maybe even
> have
> > > > >> a setting that controls whether startup is even possible when not
> all
> > > > >> ACLs are in effect.
> > > > >>
> > > > >> As I mentioned I have a few concerns, question marks still
> > > outstanding on
> > > > >> this:
> > > > >> - This approach would necessitate being backwards compatible with
> all
> > > > >> earlier versions of ACLs unless we also add a min_acl_version
> setting
> > > > >> - which would put the topic of ACL migrations back on the agenda.
> > > > >> - Do we need to touch the wire protocol for the admin client for
> this?
> > > > >> In theory I think not, as the authorizer would write ACLs in the
> most
> > > > >> current (unless forced down by max_acl_version) version it knows,
> but
> > > > >> this takes any control over this away from the user.
> > > > >> - This adds json parsing logic to the Authorizer, as it would
> have to
> > > > >> check the version first, look up the proper ACL schema for that
> > > > >> version and then re-parse the ACL string with that schema -
> should not
> > > > >> be a real issue if the initial parsing is robust, but strictly
> > > > >> speaking we are parsing something that we don't know the schema
> for
> > > > >> which might create issues with updates down the line.
> > > > >>
> > > > >> Beyond the practical concerns outlined above there are also some
> > > > >> broader things maybe worth thinking about. The long term goal is
> to
> > > > >> move away from Zookeeper and other data like consumer group
> offsets
> > > > >> has already been moved into Kafka topics - is that something that
> we'd
> > > > >> want to consider for ACLs as well? With the current storage model
> we'd
> > > > >> need more than one topic for this to cleanly separate resources
> and
> > > > >> prefixed ACLs - if we consider pursuing this option it might be a
> > > > >> chance for a "larger" change to the format which introduces
> versioning
> > > > >> and allows storing everything in one compacted topic.
> > > > >>
> > > > >> Any thoughts on this?
> > > > >>
> > > > >> Best regards,
> > > > >> Sönke
> > > > >>
> > > > >>
> > > > >>
> > > > >> [1]
> > > > >>
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+Prefixed+ACLs
> > > > >>
> > > > >>
> > > > >> On Sat, Dec 22, 2018 at 5:51 AM Colin McCabe <cmcc...@apache.org>
> > > wrote:
> > > > >> >
> > > > >> > Hi Sönke,
> > > > >> >
> > > > >> > One path forward would be to forbid the new ACL types from being
> > > > >> created until the inter-broker protocol had been upgraded. We'd
> also
> > > have
> > > > >> to figure out how the new ACLs were stored in ZooKeeper. There
> are a
> > > bunch
> > > > >> of proposals in this thread that could work for that-- I really
> hope
> > > we
> > > > >> don't keep changing the ZK path each time there is a version bump.
> > > > >> >
> > > > >> > best,
> > > > >> > Colin
> > > > >> >
> > > > >> >
> > > > >> > On Thu, Nov 29, 2018, at 14:25, Sönke Liebau wrote:
> > > > >> > > This has been dormant for a while now, can I interest anybody
> in
> > > > >> chiming in
> > > > >> > > here?
> > > > >> > >
> > > > >> > > I think we need to come up with an idea of how to handle
> changes
> > > to
> > > > >> ACLs
> > > > >> > > going forward, i.e. some sort of versioning scheme. Not
> > > necessarily
> > > > >> what I
> > > > >> > > proposed in my previous mail, but something.
> > > > >> > > Currently this fairly simple change is stuck due to this being
> > > > >> unsolved.
> > > > >> > >
> > > > >> > > I am happy to move forward without addressing the larger
> issue (I
> > > > >> think the
> > > > >> > > issue raised by Colin is valid but could be mitigated in the
> > > release
> > > > >> > > notes), but that would mean that the next KIP to touch ACLs
> would
> > > > >> inherit
> > > > >> > > the issue, which somehow doesn't seem right.
> > > > >> > >
> > > > >> > > Looking forward to your input :)
> > > > >> > >
> > > > >> > > Best regards,
> > > > >> > > Sönke
> > > > >> > >
> > > > >> > > On Tue, Jun 19, 2018 at 5:32 PM Sönke Liebau <
> > > > >> soenke.lie...@opencore.com>
> > > > >> > > wrote:
> > > > >> > >
> > > > >> > > > Picking this back up, now that KIP-290 has been merged..
> > > > >> > > >
> > > > >> > > > As Colin mentioned in an earlier mail this change could
> create a
> > > > >> > > > potential security issue if not all brokers are upgraded
> and a
> > > DENY
> > > > >> > > > Acl based on an IP range is created, as old brokers won't
> match
> > > this
> > > > >> > > > rule and still allow requests. As I stated earlier I am not
> sure
> > > > >> > > > whether for this specific change this couldn't be handled
> via
> > > the
> > > > >> > > > release notes (see also this comment [1] from Jun Rao on a
> > > similar
> > > > >> > > > topic), but in principle I think some sort of versioning
> system
> > > > >> around
> > > > >> > > > ACLs would be useful. As seen in KIP-290 there were a few
> > > > >> > > > complications around where to store ACLs. To avoid adding
> ever
> > > new
> > > > >> > > > Zookeeper paths for future ACL changes a versioning system
> is
> > > > >> probably
> > > > >> > > > useful.
> > > > >> > > >
> > > > >> > > > @Andy: I've copied you directly in this mail, since you did
> a
> > > bulk
> > > > >> of
> > > > >> > > > the work around KIP-290 and mentioned potentially picking
> up the
> > > > >> > > > follow up work, so I think your input would be very valuable
> > > here.
> > > > >> Not
> > > > >> > > > trying to shove extra work your way, I'm happy to
> contribute,
> > > but
> > > > >> we'd
> > > > >> > > > be touching a lot of the same areas I think.
> > > > >> > > >
> > > > >> > > > If we want to implement a versioning system for ACLs I see
> the
> > > > >> > > > following todos (probably incomplete & missing something at
> the
> > > same
> > > > >> > > > time):
> > > > >> > > > 1. ensure that the current Authorizer doesn't pick up newer
> ACLs
> > > > >> > > > 2. add a version marker to new ACLs
> > > > >> > > > 3. change SimpleACLAuthorizer to know what version of ACLs
> it is
> > > > >> > > > compatible with and only load ACLs of this / smaller version
> > > > >> > > > 4. Decide how to handle if incompatible (newer version)
> ACLs are
> > > > >> > > > present: log warning, fail broker startup, ...
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > Post-KIP-290 ACLs are stored in two places in Zookeeper:
> > > > >> > > > /kafka-acl-extended - for ACLs with wildcards in the
> resource
> > > > >> > > > /kafka-acl - for literal ACLs without wildcards (i.e. *
> > > means *
> > > > >> not
> > > > >> > > > any character)
> > > > >> > > >
> > > > >> > > > To ensure 1 we probably need to move to a new directory once
> > > more,
> > > > >> > > > call it /kafka-acl-extended-new for arguments sake. Any ACL
> > > stored
> > > > >> > > > here would get a version number stored with it, and only
> > > > >> > > > SimpleAuthorizers that actually know to look here would find
> > > these
> > > > >> > > > ACLs and also know to check for a version number. I think
> Andy
> > > > >> > > > mentioned moving the resource definition in the new ACL
> format
> > > to
> > > > >> JSON
> > > > >> > > > instead of simple string in a follow up PR, maybe these
> pieces
> > > of
> > > > >> work
> > > > >> > > > are best tackled together - and if a new znode can be
> avoided
> > > even
> > > > >> > > > better.
> > > > >> > > >
> > > > >> > > > This would allow us to recognize situations where ACLs are
> > > defined
> > > > >> > > > that not all Authorizers can understand, as those
> Authorizers
> > > would
> > > > >> > > > notice that there are ACLs with a larger version than the
> one
> > > they
> > > > >> > > > support (not applicable to legacy ACLs up until now). How we
> > > want to
> > > > >> > > > treat this scenario is up for discussion, I think make it
> > > > >> > > > configurable, as customers have different requirements
> around
> > > > >> > > > security. Some would probably want to fail a broker that
> > > encounters
> > > > >> > > > unknown ACLs so as to not create potential security risks t
> > > others
> > > > >> > > > might be happy with just a warning in the logs. This should
> > > never
> > > > >> > > > happen, if users fully upgrade their clusters before
> creating
> > > new
> > > > >> ACLs
> > > > >> > > > - but to counteract the situation that Colin described it
> would
> > > be
> > > > >> > > > useful.
> > > > >> > > >
> > > > >> > > > Looking forward, a migration option might be added to the
> > > kafka-acl
> > > > >> > > > tool to migrate all legacy ACLs once into the new structure
> > > once the
> > > > >> > > > user is certain that no old brokers will come online again.
> > > > >> > > >
> > > > >> > > > If you think this sounds like a convoluted way to go about
> > > things
> > > > >> ...
> > > > >> > > > I agree :) But I couldn't come up with a better way yet.
> > > > >> > > >
> > > > >> > > > Any thoughts?
> > > > >> > > >
> > > > >> > > > Best regards,
> > > > >> > > > Sönke
> > > > >> > > >
> > > > >> > > > [1]
> > > > >>
> https://github.com/apache/kafka/pull/5079#pullrequestreview-124512689
> > > > >> > > >
> > > > >> > > > On Thu, May 3, 2018 at 10:57 PM, Sönke Liebau
> > > > >> > > > <soenke.lie...@opencore.com> wrote:
> > > > >> > > > > Technically I absolutely agree with you, this would indeed
> > > create
> > > > >> > > > > issues. If we were just talking about this KIP I think I'd
> > > argue
> > > > >> that
> > > > >> > > > > it is not too harsh of a requirement for users to refrain
> from
> > > > >> using
> > > > >> > > > > new features until they have fully upgraded their entire
> > > cluster.
> > > > >> I
> > > > >> > > > > think in that case it could have been solved in the
> release
> > > notes
> > > > >> -
> > > > >> > > > > similarly to the way a binary protocol change is handled.
> > > > >> > > > > However looking at the discussion on KIP-290 and thinking
> > > ahead to
> > > > >> > > > > potential other changes on ACLs it would really just mean
> > > putting
> > > > >> off
> > > > >> > > > > a proper solution which is a versioning system for ACLs
> makes
> > > > >> sense.
> > > > >> > > > >
> > > > >> > > > > At least from the point of view of this KIP versioning
> should
> > > be a
> > > > >> > > > > separate KIP as otherwise we don't solve the issue you
> > > mentioned
> > > > >> above
> > > > >> > > > > - not sure about 290..
> > > > >> > > > >
> > > > >> > > > > I thought about this for a little while, would something
> like
> > > the
> > > > >> > > > > following make sense?
> > > > >> > > > >
> > > > >> > > > > ACLs are either stored in a separate Zookeeper node or
> get a
> > > > >> version
> > > > >> > > > > stored with them (separate node is probably easier). So
> > > current
> > > > >> ACLs
> > > > >> > > > > would default to v0 and post-KIP252 would be an explicit
> v1
> > > for
> > > > >> > > > > example.
> > > > >> > > > > Authorizers declare which versions they are compatible
> with
> > > > >> (though
> > > > >> > > > > I'd say i backwards compatibility is what we shoud shoot
> > > for) and
> > > > >> > > > > load ACLs of those versions.
> > > > >> > > > > Introduce a new parameter authorizer.acl.maxversion which
> > > controls
> > > > >> > > > > which ACLs are loaded by the authorizer - nothing with a
> > > version
> > > > >> > > > > higher than specified here gets loaded, even if the
> Authorizer
> > > > >> would
> > > > >> > > > > be able to.
> > > > >> > > > >
> > > > >> > > > > So the process for a cluster update would be similar to a
> > > binary
> > > > >> > > > > protocol change, set authorizer.acl.maxversion to
> new_version
> > > - 1.
> > > > >> > > > > Upgrade brokers one by one. Once you are done,
> change/remove
> > > > >> parameter
> > > > >> > > > > and restart cluster.
> > > > >> > > > >
> > > > >> > > > > I'm sure I missed something, but sound good in principle?
> > > > >> > > > >
> > > > >> > > > > Best regards,
> > > > >> > > > > Sönke
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > On Thu, May 3, 2018 at 8:15 PM, Colin McCabe <
> > > co...@cmccabe.xyz>
> > > > >> wrote:
> > > > >> > > > >> There are still some problems with compatibility here,
> right?
> > > > >> > > > >>
> > > > >> > > > >> One example is if we construct a DENY ACL with an IP
> range
> > > and
> > > > >> then
> > > > >> > > > install it. If all of our brokers have been upgraded, it
> will
> > > > >> work. But
> > > > >> > > > if there are some that still haven't been upgraded, they
> will
> > > not
> > > > >> honor the
> > > > >> > > > DENY ACL, possibly causing a security issue.
> > > > >> > > > >>
> > > > >> > > > >> In general, it seems like we need some kind of versioning
> > > system
> > > > >> in
> > > > >> > > > ACLs to handle these cases.
> > > > >> > > > >>
> > > > >> > > > >> best,
> > > > >> > > > >> Colin
> > > > >> > > > >>
> > > > >> > > > >> On Thu, May 3, 2018, at 08:11, Sönke Liebau wrote:
> > > > >> > > > >>> Hi all,
> > > > >> > > > >>>
> > > > >> > > > >>> I'd like to readopt this KIP, I got a bit sidetracked by
> > > other
> > > > >> stuff
> > > > >> > > > >>> after posting the initial version and discussion, sorry
> for
> > > > >> that.
> > > > >> > > > >>>
> > > > >> > > > >>> I've added IPv6 to the KIP, but decided to forego the
> other
> > > > >> scope
> > > > >> > > > >>> extensions that I mentioned in my previous mail, as
> there
> > > are
> > > > >> other
> > > > >> > > > >>> efforts underway in KIP-290 that cover most of the
> > > suggestions
> > > > >> > > > >>> already.
> > > > >> > > > >>>
> > > > >> > > > >>> Does anybody have any other objections to starting a
> vote on
> > > > >> this KIP?
> > > > >> > > > >>>
> > > > >> > > > >>> Regards,
> > > > >> > > > >>> Sönke
> > > > >> > > > >>>
> > > > >> > > > >>> On Fri, Feb 2, 2018 at 5:11 PM, Sönke Liebau <
> > > > >> > > > soenke.lie...@opencore.com> wrote:
> > > > >> > > > >>> > Hi Manikumar,
> > > > >> > > > >>> >
> > > > >> > > > >>> > you are right, 5713 is a bit ambiguous about which
> fields
> > > are
> > > > >> > > > considered in
> > > > >> > > > >>> > scope, but I agree that wildcards for Ips are not
> > > necessary
> > > > >> when we
> > > > >> > > > have
> > > > >> > > > >>> > ranges.
> > > > >> > > > >>> >
> > > > >> > > > >>> > I am wondering though, if we might want to extend the
> > > scope
> > > > >> of this
> > > > >> > > > KIP a
> > > > >> > > > >>> > bit while we are changing acl and authorizer classes
> > > anyway.
> > > > >> > > > >>> >
> > > > >> > > > >>> > After considering this a bit on a flihht with no wifi
> > > > >> yesterday I
> > > > >> > > > came up
> > > > >> > > > >>> > with the following:
> > > > >> > > > >>> >
> > > > >> > > > >>> > * wildcards or regular expressions for principals,
> groups
> > > and
> > > > >> topics
> > > > >> > > > >>> > * extend the KafkaPrincipal object to allow adding
> custom
> > > > >> key-value
> > > > >> > > > pairs in
> > > > >> > > > >>> > principalbuilder implementations
> > > > >> > > > >>> > * extend SimpleAclAuthorizer and the ACL tools to
> > > authorize
> > > > >> on these
> > > > >> > > > >>> > key/value pairs
> > > > >> > > > >>> >
> > > > >> > > > >>> > The second and third bullet points would allow easy
> > > creation
> > > > >> of for
> > > > >> > > > example
> > > > >> > > > >>> > a principalbuilder that adds groups the user belongs
> to in
> > > > >> the active
> > > > >> > > > >>> > directory to its principal, without requiring the
> user to
> > > also
> > > > >> > > > extend the
> > > > >> > > > >>> > authorizer and create custom ACL storage. This would
> > > > >> significantly
> > > > >> > > > lower the
> > > > >> > > > >>> > technical debt incurred by custom authorizer
> mechanisms I
> > > > >> think.
> > > > >> > > > >>> >
> > > > >> > > > >>> > There are a few issues to hash out of course, but I'd
> > > think in
> > > > >> > > > general this
> > > > >> > > > >>> > should work work nicely and be a step towards meeting
> > > > >> corporate
> > > > >> > > > >>> > authorization requirements.
> > > > >> > > > >>> >
> > > > >> > > > >>> > Best regards,
> > > > >> > > > >>> > Sönke
> > > > >> > > > >>> >
> > > > >> > > > >>> > Am 01.02.2018 18:46 schrieb "Manikumar" <
> > > > >> manikumar.re...@gmail.com>:
> > > > >> > > > >>> >
> > > > >> > > > >>> > Hi,
> > > > >> > > > >>> >
> > > > >> > > > >>> > They are few deployments using IPv6. It is good to
> > > support
> > > > >> IPv6
> > > > >> > > > also.
> > > > >> > > > >>> >
> > > > >> > > > >>> > I think KAFKA-5713 is about adding regular expression
> > > support
> > > > >> to
> > > > >> > > > resource
> > > > >> > > > >>> > names (topic. consumer etc..).
> > > > >> > > > >>> > Yes, wildcards (*) in hostname doesn't makes sense.
> Range
> > > and
> > > > >> subnet
> > > > >> > > > >>> > support will give us the flexibility.
> > > > >> > > > >>> >
> > > > >> > > > >>> > On Thu, Feb 1, 2018 at 5:56 PM, Sönke Liebau <
> > > > >> > > > >>> > soenke.lie...@opencore.com.invalid> wrote:
> > > > >> > > > >>> >
> > > > >> > > > >>> >> Hi Manikumar,
> > > > >> > > > >>> >>
> > > > >> > > > >>> >> the current proposal indeed leaves out IPv6
> addresses,
> > > as I
> > > > >> was
> > > > >> > > > unsure
> > > > >> > > > >>> >> whether Kafka fully supports that yet to be honest.
> But
> > > it
> > > > >> would be
> > > > >> > > > >>> >> fairly easy to add these to the proposal - I'll
> update it
> > > > >> over the
> > > > >> > > > >>> >> weekend.
> > > > >> > > > >>> >>
> > > > >> > > > >>> >> Regarding KAFKA-5713, I simply listed it as related,
> > > since
> > > > >> it is
> > > > >> > > > >>> >> similar in spirit, if not exact wording. Parts of
> that
> > > issue
> > > > >> > > > >>> >> (wildcards in hosts) would be covered by this kip -
> just
> > > in a
> > > > >> > > > slightly
> > > > >> > > > >>> >> different way. Do we really need wildcard support in
> IP
> > > > >> addresses if
> > > > >> > > > >>> >> we can specify ranges and subnets? I considered it,
> but
> > > only
> > > > >> came up
> > > > >> > > > >>> >> with scenarios that seemed fairly academic to me,
> like
> > > > >> allowing the
> > > > >> > > > >>> >> same host from multiple subnets (10.0.*.1) for
> example.
> > > > >> > > > >>> >>
> > > > >> > > > >>> >> Allowing wildcards has the potential to make the code
> > > more
> > > > >> complex,
> > > > >> > > > >>> >> depending on how we decide to implement this feature,
> > > hance I
> > > > >> > > > decided
> > > > >> > > > >>> >> to leave wildcards out for now.
> > > > >> > > > >>> >>
> > > > >> > > > >>> >> What do you think?
> > > > >> > > > >>> >>
> > > > >> > > > >>> >> Best regards,
> > > > >> > > > >>> >> Sönke
> > > > >> > > > >>> >>
> > > > >> > > > >>> >> On Thu, Feb 1, 2018 at 10:14 AM, Manikumar <
> > > > >> > > > manikumar.re...@gmail.com>
> > > > >> > > > >>> >> wrote:
> > > > >> > > > >>> >> > Hi,
> > > > >> > > > >>> >> >
> > > > >> > > > >>> >> > 1. Do we support IPv6 CIDR/ranges?
> > > > >> > > > >>> >> >
> > > > >> > > > >>> >> > 2. KAFKA-5713 is mentioned in Related JIRAs
> section.
> > > But
> > > > >> there is
> > > > >> > > > no
> > > > >> > > > >>> >> > mention of wildcard support in the KIP.
> > > > >> > > > >>> >> >
> > > > >> > > > >>> >> >
> > > > >> > > > >>> >> > Thanks,
> > > > >> > > > >>> >> >
> > > > >> > > > >>> >> > On Thu, Feb 1, 2018 at 4:05 AM, Sönke Liebau <
> > > > >> > > > >>> >> > soenke.lie...@opencore.com.invalid> wrote:
> > > > >> > > > >>> >> >
> > > > >> > > > >>> >> >> Hey everybody,
> > > > >> > > > >>> >> >>
> > > > >> > > > >>> >> >> following a brief inital discussion a couple of
> days
> > > ago
> > > > >> on this
> > > > >> > > > list
> > > > >> > > > >>> >> >> I'd like to get a discussion going on KIP-252
> which
> > > would
> > > > >> allow
> > > > >> > > > >>> >> >> specifying ip ranges and subnets for the
> -allow-host
> > > and
> > > > >> > > > --deny-host
> > > > >> > > > >>> >> >> parameters of the acl tool.
> > > > >> > > > >>> >> >>
> > > > >> > > > >>> >> >> The KIP can be found at
> > > > >> > > > >>> >> >>
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > >> > > > >>> >> >>
> > > > >> > > >
> > > 252+-+Extend+ACLs+to+allow+filtering+based+on+ip+ranges+and+subnets
> > > > >> > > > >>> >> >>
> > > > >> > > > >>> >> >> Best regards,
> > > > >> > > > >>> >> >> Sönke
> > > > >> > > > >>> >> >>
> > > > >> > > > >>> >>
> > > > >> > > > >>> >>
> > > > >> > > > >>> >>
> > > > >> > > > >>> >> --
> > > > >> > > > >>> >> Sönke Liebau
> > > > >> > > > >>> >> Partner
> > > > >> > > > >>> >> Tel. +49 179 7940878
> > > > >> > > > >>> >> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880
> > > Wedel -
> > > > >> > > > Germany
> > > > >> > > > >>> >>
> > > > >> > > > >>> >
> > > > >> > > > >>> >
> > > > >> > > > >>>
> > > > >> > > > >>>
> > > > >> > > > >>>
> > > > >> > > > >>> --
> > > > >> > > > >>> Sönke Liebau
> > > > >> > > > >>> Partner
> > > > >> > > > >>> Tel. +49 179 7940878
> > > > >> > > > >>> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880
> Wedel
> > > -
> > > > >> Germany
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > --
> > > > >> > > > > Sönke Liebau
> > > > >> > > > > Partner
> > > > >> > > > > Tel. +49 179 7940878
> > > > >> > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880
> Wedel -
> > > > >> Germany
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > Sönke Liebau
> > > > >> > > > Partner
> > > > >> > > > Tel. +49 179 7940878
> > > > >> > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel
> -
> > > > >> Germany
> > > > >> > > >
> > > > >> > >
> > > > >> > >
> > > > >> > > --
> > > > >> > > Sönke Liebau
> > > > >> > > Partner
> > > > >> > > Tel. +49 179 7940878
> > > > >> > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel -
> > > Germany
> > > > >>
> > > > >>
> > > > >>
> > > > >> --
> > > > >> Sönke Liebau
> > > > >> Partner
> > > > >> Tel. +49 179 7940878
> > > > >> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel -
> Germany
> > > > >>
> > > > >
> > > > >
> > > > > --
> > > > > Sönke Liebau
> > > > > Partner
> > > > > Tel. +49 179 7940878
> > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel -
> Germany
> > > > >
> > > >
> > > >
> > > > --
> > > > Sönke Liebau
> > > > Partner
> > > > Tel. +49 179 7940878
> > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany
> > > >
> > >
> >
> >
> > --
> > Sönke Liebau
> > Partner
> > Tel. +49 179 7940878
> > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany
> >



-- 
Sönke Liebau
Partner
Tel. +49 179 7940878
OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany

Reply via email to