Hi Mayuresh, 

See comments below, please:

> On 22 Oct 2015, at 18:17, Mayuresh Gharat <gharatmayures...@gmail.com> wrote:
> 
> This might have been explained before. I had a question :
> In the KIP it says :
> "One ZooKeeper setting of interest on the server side is
> zookeeper.allowSaslFailedClients. If this is false, then clients trying to
> authenticate with an incorrect configuration will have their connections
> dropped. Otherwise, such clients will be able to connect successfully, but
> will not have the right credentials set. Setting it to false prevents
> clients with an incorrect configuration from making progress.ZooKeeper also
> allows users to disable authentication on the client side even in the
> presence of a JAAS login file with the property zookeeper.sasl.client.
> Setting it to false disables client authentication."
> 
> 
> *1) So if I set "zookeeper.sasl.client" to false and
> "zookeeper.allowSaslFailedClients" to true, will the clients be
> authenticated? *

"zookeeper.sasl.client" is a config parameter on the client side, while 
"allowSaslFailedClients" is a configuration parameter for servers. If you set 
"zookeeper.sasl.client" to false, then the client won't be authenticated 
independent of the value of "allowSaslFailedClients" because you'll be 
disabling the SASL client code. 

> *2) Also what is meant by clients with incorrect configuration?*
> 

Say you make a mistake in your JAAS login file. If "allowSaslFailedClients" is 
set to true, then your client would still connect (not authenticated) silently, 
and you may prefer to spot that problem right up front by having the server 
reject the connection. 

-Flavio

> 
> On Thu, Oct 22, 2015 at 8:11 AM, Flavio Junqueira <f...@apache.org> wrote:
> 
>> 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
>>>> 
>>>> 
>> 
>> 
> 
> 
> -- 
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125

Reply via email to