Ok, thanks for spotting it. -Flavio
> On 22 Oct 2015, at 05:54, Jun Rao <j...@confluent.io> wrote: > > 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 >> >>
