It seems that in the secure -> unsecure plan, step 3 needs to be done before step 2.
Thanks, Jun On Wed, Oct 21, 2015 at 3:59 PM, Flavio Junqueira <f...@apache.org> wrote: > Ok, thanks for the feedback, Todd. I have updated the KIP with some of the > points discussed here. There is more to add based on these last comments, > though. > > -Flavio > > > On 21 Oct 2015, at 23:43, Todd Palino <tpal...@gmail.com> wrote: > > > > On Wed, Oct 21, 2015 at 3:38 PM, Flavio Junqueira <f...@apache.org > <mailto:f...@apache.org>> wrote: > > > >> > >>> On 21 Oct 2015, at 21:54, Todd Palino <tpal...@gmail.com> wrote: > >>> > >>> Thanks for the clarification on that, Jun. Obviously, we haven't been > >> doing > >>> much with ZK authentication around here yet. There is still a small > >> concern > >>> there, mostly in that you should not share credentials any more than is > >>> necessary, which would argue for being able to use a different ACL than > >> the > >>> default. I don't really like the idea of having to use the exact same > >>> credentials for executing the admin tools as we do for running the > >> brokers. > >>> Given that we don't need to share the credentials with all consumers, I > >>> think we can work around it. > >>> > >> > >> Let me add that a feature to separate the sub-trees of users sharing an > >> ensemble is chroot. > >> > >> On different credentials for admin tools, this sounds doable by setting > >> the ACLs of znodes. For example, there could be an admin id and a broker > >> id, both with the ability of changing znodes, but different credentials. > >> Would something like that work for you? > >> > > > > It would be a nice option to have, as the credentials can be protected > > differently. I would consider this a nice to have, and not an "absolutely > > must have" feature at this point. > > > > > >> This does bring up another good question, however. What will be the > >> process > >>> for having to rotate the credentials? That is, if the credentials are > >>> compromised and need to be changed, how can that be accomplished with > the > >>> cluster online. I'm guessing some combination of using skipAcl on the > >>> Zookeeper ensemble and config changes to the brokers will be required, > >> but > >>> this is an important enough operation that we should make sure it's > >>> reasonable to perform and that it is documented. > >> > >> Right now there is no kafka support in the plan for this. But this is > >> doable directly through the zk api. Would it be sufficient to write down > >> how to perform such an operation via the zk api or do we need a tool to > do > >> it? > >> > > > > I think as long as there is a documented procedure for how to do it, that > > will be good enough. It's mostly about making sure that we can, and that > we > > don't put something in place that would require downtime to a cluster in > > order to change credentials. We can always develop a tool later if it is > a > > requested item. > > > > Thanks! > > > > -Todd > > > > > > > >> > >> -Flavio > >> > >>> > >>> > >>> On Wed, Oct 21, 2015 at 1:23 PM, Jun Rao <j...@confluent.io> wrote: > >>> > >>>> Parth, > >>>> > >>>> For 2), in your approach, the broker/controller will then always have > >> the > >>>> overhead of resetting the ACL on startup after zookeeper.set.acl is > set > >> to > >>>> true. The benefit of using a separate migration tool is that you paid > >> the > >>>> cost only once during upgrade. It is an extra step during the upgrade. > >>>> However, given the other things that you need to do to upgrade to > 0.9.0 > >>>> (e.g. two rounds of rolling upgrades on all brokers, etc), I am not > >> sure if > >>>> it's worth to optimize away of this step. We probably just need to > >> document > >>>> this clearly. > >>>> > >>>> Todd, > >>>> > >>>> Just to be clear about the shared ZK usage. Once you set > >> CREATOR_ALL_ACL + > >>>> READ_ACL_UNSAFE on a path, only ZK clients with the same user as the > >>>> creator can modify the path. Other ZK clients authenticated with a > >>>> different user can read, but not modify the path. Are you concerned > >> about > >>>> the reads or the writes to ZK? > >>>> > >>>> Thanks, > >>>> > >>>> Jun > >>>> > >>>> > >>>> > >>>> On Wed, Oct 21, 2015 at 10:46 AM, Flavio Junqueira <f...@apache.org> > >> wrote: > >>>> > >>>>> > >>>>>> On 21 Oct 2015, at 18:07, Parth Brahmbhatt < > >>>> pbrahmbh...@hortonworks.com> > >>>>> wrote: > >>>>>> > >>>>>> I have 2 suggestions: > >>>>>> > >>>>>> 1) We need to document how does one move from secure to non secure > >>>>>> environment: > >>>>>> 1) change the config on all brokers to zookeeper.set.acl = false > >>>>> and do a > >>>>>> rolling upgrade. > >>>>>> 2) Run the migration script with the jass config file so it is > >>>> sasl > >>>>>> authenticated with zookeeper and change the acls on all subtrees > back > >>>> to > >>>>>> World modifiable. > >>>>>> 3) remove the jaas config / or only the zookeeper section from > >>>> the > >>>>> jaas, > >>>>>> and restart all brokers. > >>>>>> > >>>>> > >>>>> Thanks for bringing it up, it makes sense to have a downgrade path > and > >>>>> document it. > >>>>> > >>>>> > >>>>>> 2) I am not sure if we should force users trying to move from > unsecure > >>>> to > >>>>>> secure environment to execute the migration script. In the second > step > >>>>>> once the zookeeper.set.acl is set to true, we can secure all the > >>>> subtrees > >>>>>> by calling ensureCorrectAcls as part of broker initialization (just > >>>> after > >>>>>> makesurePersistentPathExists). Not sure why we want to add one more > >>>>>> manual/admin step when it can be automated. This also has the added > >>>>>> advantage that migration script will not have to take a flag as > input > >>>> to > >>>>>> figure out if it should set the acls to secure or unsecure given it > >>>> will > >>>>>> always be used to move from secure to unsecure. > >>>>>> > >>>>> > >>>>> The advantage of the third step is to make a single traversal to > change > >>>>> any remaining znodes with the open ACL. As you suggest, each broker > >> would > >>>>> do it, so the overhead is much higher. I do agree that eliminating a > >> step > >>>>> is an advantage, though. > >>>>> > >>>>>> Given we are assuming all the information in zookeeper is world > >>>> readable > >>>>> , > >>>>>> I don¹t see SSL support as a must have or a blocker for this KIP. > >>>>> > >>>>> OK, but keep in mind that SSL is only available in the 3.5 branch of > >> ZK. > >>>>> > >>>>> -Flavio > >>>>> > >>>>>> > >>>>>> Thanks > >>>>>> Parth > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 10/21/15, 9:56 AM, "Flavio Junqueira" <f...@apache.org> wrote: > >>>>>> > >>>>>>> > >>>>>>>> On 21 Oct 2015, at 17:47, Todd Palino <tpal...@gmail.com> wrote: > >>>>>>>> > >>>>>>>> There seems to be a bit of detail lacking in the KIP. > Specifically, > >>>> I'd > >>>>>>>> like to understand: > >>>>>>>> > >>>>>>>> 1) What znodes are the brokers going to secure? Is this > >> configurable? > >>>>>>>> How? > >>>>>>> > >>>>>>> Currently it is securing all paths here except the consumers one: > >>>>>>> > >>>>>>> > >>>>> > >>>> > >> > https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/utils > >>>>>>> /ZkUtils.scala#L56 > >>>>>>> < > >>>>> > >>>> > >> > https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/util > >>>>>>> s/ZkUtils.scala#L56> > >>>>>>> > >>>>>>> This isn't configurable at the moment. > >>>>>>> > >>>>>>>> 2) What ACL is the broker going to apply? Is this configurable? > >>>>>>> > >>>>>>> The default is CREATOR_ALL_ACL + READ_ACL_UNSAFE, which means that > an > >>>>>>> authenticated client can manipulate secured znodes and everyone can > >>>> read > >>>>>>> znodes. The API of ZkUtils accommodates other ACLs, but the current > >>>> code > >>>>>>> is using the default. > >>>>>>> > >>>>>>>> 3) How will the admin tools (such as preferred replica election > and > >>>>>>>> partition reassignment) interact with this? > >>>>>>>> > >>>>>>> > >>>>>>> Currently, you need to set a system property passing the login > config > >>>>>>> file to be able to authenticate the client and perform writes to > ZK. > >>>>>>> > >>>>>>> -Flavio > >>>>>>> > >>>>>>>> -Todd > >>>>>>>> > >>>>>>>> > >>>>>>>> On Wed, Oct 21, 2015 at 9:16 AM, Ismael Juma <ism...@juma.me.uk> > >>>>> wrote: > >>>>>>>> > >>>>>>>>> On Wed, Oct 21, 2015 at 5:04 PM, Flavio Junqueira < > f...@apache.org> > >>>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> Bringing the points Grant brought to this thread: > >>>>>>>>>> > >>>>>>>>>>> Is it worth mentioning the follow up steps that were discussed > in > >>>>> the > >>>>>>>>> KIP > >>>>>>>>>>> call in this KIP document? Some of them were: > >>>>>>>>>>> > >>>>>>>>>>> - Adding SSL support for Zookeeper > >>>>>>>>>>> - Removing the "world readable" assumption > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> Grant, how would you do it? I see three options: > >>>>>>>>>> > >>>>>>>>>> 1- Add to the existing KIP, but then the functionality we should > >> be > >>>>>>>>>> checking in soon won't include it, so the KIP will remain > >>>> incomplete > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> A "Future work" section would make sense to me, but I don't know > >> how > >>>>>>>>> this > >>>>>>>>> is normally handled. > >>>>>>>>> > >>>>>>>>> Ismael > >