You're right, we should set the root ACL separately instead of starting the recursion on the root. One other reason for doing this is that there is at least one persistent branch we don't want to secure (ConsumersPath).
I'll create a jira for this and fix it. -Flavio > On 06 Jan 2016, at 14:09, Matthew Bruce <mbr...@blackberry.com> wrote: > > Hi Flavio, > > We're not using a chroot for Kafka itself as the ensemble is (mostly) not > shared. We do have 1 or two extra branches that are used for some custom > programs that use our Kafka infrastructure (each of these branches are used > as a chroot). In hindsight using a separate chroot for Kafka would have been > a good idea, but that's a design decision that was made a long time ago now. > > Wouldn't we be able to set the ACLs for all the Kafka specific branches > (/controller, /brokers, etc) recursively as well as "/" (non-recursively), > thus leaving all the non-kafka related branches alone? > > It seems counter-intuitive to me to loop through > zkUtils.securePersistentZkPaths - logging that we're going to be setting the > ACLs on those specific paths to then just go and perform the operation on > all of / recursively anyway. > > Thanks, > Matt > > -----Original Message----- > From: Flavio Junqueira [mailto:f...@apache.org] > Sent: Wednesday, January 6, 2016 4:35 AM > To: dev@kafka.apache.org > Subject: Re: ZkSecurityMigrator incorrectly applies ACLs to entire ZooKeeper > tree? > > Hi Matthew, > > If you're sharing a ZK ensemble and you have a specific path for the Kafka > znodes, then you need to use a chroot for this. Just pass it along with the > connect string: > > http://zookeeper.apache.org/doc/r3.4.6/zookeeperProgrammers.html#ch_zkSessions > > <http://zookeeper.apache.org/doc/r3.4.6/zookeeperProgrammers.html#ch_zkSessions> > > If we don't protected the root of the Kafka sub-tree, then an unauthorized > user will be able to delete child nodes under the sub-tree root: > > http://zookeeper.apache.org/doc/r3.4.6/zookeeperProgrammers.html#sc_ACLPermissions > > <http://zookeeper.apache.org/doc/r3.4.6/zookeeperProgrammers.html#sc_ACLPermissions> > > -Flavio > >> On 05 Jan 2016, at 21:09, Matthew Bruce <mbr...@blackberry.com> wrote: >> >> Hi, >> >> I'm running through some 0.8.2 to 0.9.0 upgrade testing that involves moving >> to a secured cluster - While running the zookeeper-security-migration.sh >> script, I noticed that it modifies ACLs for non-Kafka specific znodes/trees >> also. >> >> Looking at the code it seems like the intention is to only set the ACLs on >> specific branches, but then it recursively applies them to all of '/' anyway: >> >> private def run(): Unit = { >> try { >> for (path <- zkUtils.securePersistentZkPaths) { >> debug("Going to set ACL for %s".format(path)) >> zkUtils.makeSurePersistentPathExists(path) >> } >> setAclsRecursively("/") >> . >> . >> . >> >> >> Am I missing something here, or should the setAclsRecursively call be moved >> into the loop and be called against each specific path? >> >> Thanks, >> Matthew Bruce >> mbr...@blackbery.com >
