Hi Gwen, Thanks a lot for taking the time to review this. I have tried to address all your questions below.
Thanks Parth On 3/28/15, 8:08 PM, "Gwen Shapira" <gshap...@cloudera.com<mailto:gshap...@cloudera.com>> wrote: Preparing for Tuesday meeting, I went over the KIP :) First, Parth did an amazing job, the KIP is fantastic - detailed and readable. Thank you! Second, I have a loooong list of questions :) No objections, just some things I'm unclear on and random minor comments. In general, I like the design, I just feel I'm missing parts of the picture. 1. "Yes, Create topic will have an optional acls, the output of describe will display owner and acls and alter topic will allow to modify the acls.” - will be nice to see what the CLI will look like. * I will modify the KIP but I was going to add “—acl <acl-file.json>” to create-topic and alter-topic. 2. I like the addition of Topic owner. We made the mistake of forgetting about it when adding authorization to Sqoop2. We probably want to add “chown” command to the topic commands. * Again we can add “—owner <user-name>” to alter topic. 3. "Kafka server will read "authorizer.class” config value at startup time, create an instance of the specified class and call initialize method." We’ll need to validate that users specify only one of those. * The config type will be string so type validation should take care of it. 4. "One added assumption is that on non-secure connections the session will have principal set to an object whose name() method will return "Anonymous”." Can we keep DrWho? :) * Sure, its up to you actually as you are the owner of the jira that introduces session concept. 5. "For cluster actions that do not apply to a specific topic like CREATE we have 2 options. We can either add a broker config called broker.acls which will point to a json file. This file will be available on all broker hosts and authorizer will read the acls on initialization and keep refreshing it every X minutes. Any changes will require re-distribution of the acl json file. Alternatively we can add a zookeeper path /brokers/acls and store the acl json as data. Authorizer can refresh the acl from json every X minutes. In absence of broker acls the authorizer will fail open, in other words it will allow all users from all hosts to perform all cluster actions” I prefer a file to ZK - since thats where we store all use-defined configurations for now. Everyone knows how to secure a file system :) * I will let everyone vote, file system is fine by me. 6. "When an Acl is missing , this implementation will always fail open for backward compatibility. “ <- agree, but we need to document that this makes the default authorizer non-secure * Sure. 7. "If the value of authorizer.class.name is null, in secure mode the cluster will fail with ConfigException. In non secure mode in absence of config value forauthorizer.class.name the server will allow all requests to all topics that , even if the topic has configured acls” <- I don’t think Kafka has “secure mode” - it can support SSL and plaintext (un-authenticated) on two different ports simultaneously. I don’t object to adding such configuration, but we need to decide exactly what it means. * This is one area of confusion so I will add an open question. 8. "Currently all topic creation/modification/deletion actions are performed using KafkaAdminUtil which mostly interacts directly with zookeeper instead of forwarding requests to a broker host. Given all the code is executed on client side there is no easy way to perform authorization. “ - since we are adding the admin protocol requests in KIP-4, perhaps addressing those makes sense. * Yes, We will have to wait for KIP-4 to be delivered. 9. I didn’t see a specification of what is “resource”, I assume its an enum with things like Topic and… ? * Resource is not an enum, Right now its just name of the topic. Currently we will not be able to support cluster actions like CREATE in which case the resource can be some constant string like “Kafka-Cluster”. 10. I’m also unclear on where and how “PermissionType” will be used and what does “DENY takes precedence” mean here. * I originally did not have the “PermissionType” but someone suggested in DISCUSS thread that we should add support for ALLOW and DENY acls. The use case is to support acls like “Allow user U to Perform READ from all Hosts except from Host1 and Host2”. Deny take precedence only means if you have define 2 ACLs for the same user/resource/operation. DENY acl will take effect I.e. “Allow user X to read from *” and “Deny User X to Read from host1 and host2” will satisfy the use case described in the previous statement. 11. "What does acls on zookeeper node look like given all our admin APIs are currently performed directly from client?” <- “secure mode” Kafka will need to set ACLs on ZK (using ZK’s model of ACLs) and both Kafka and everyone else will need to use them (this is limited to Kerberos authentication AFAIK.) * Yes, again I think this can be a separate issue for now and can only be worked on after KIP-4 is delivered. 12. "Do we want to support group acls as part of this authorizer? Do we want to support principal to local user mapping? If yes we need to add plugins for UserToGroupMapper and PrincipalToUserMapper.” <- Sentry uses Groups for authorizing, so we need to support that. I figured that as long as the API specifies Principal, it typically contains both user and group, so nothing else is needed. Did I miss anything? * Once we support group acls we will need someway to indicate if a principal is of type group or user(as part of acl) or we can have group acls and user acls stored separately with topic config. We will also need a way to map an authenticated user to list of groups the user belongs to. 13. It looks like the Authorizer stores the ACLs and not Kafka. So we need an API for Kafka to notify Authorizer when a topic is added and when ACLs are modified, right? I didn’t see that. * ACLs will be stored under /topic/config at time of topic creation in json format. Authorizer will get these acls using newly introduced TopicConfigCache which gets updated anytime topic config changes. 14. Are we going to have any API for Kafka to give out the ACLs on a topic? Or we leave this to the Authorizer? * I think it is better to leave this out side of Kafka. Mainly because most 3rd party authorizers will have their own ACL stores and configuration Uis like (Ranger-Argus not sure what are they calling it now). On Wed, Mar 25, 2015 at 9:26 PM, Neha Narkhede <n...@confluent.io<mailto:n...@confluent.io>> wrote: Parth, We can make some 15 mins or so to discuss this at the next KIP hangout. Thanks, Neha On Wed, Mar 25, 2015 at 1:07 PM, Parth Brahmbhatt < pbrahmbh...@hortonworks.com<mailto:pbrahmbh...@hortonworks.com>> wrote: Hi all, I have modified the KIP to reflect the recent change request from the reviewers. I have been working on the code and I have the server side code for authorization ready. I am now modifying the command line utilities. I would really appreciate if some of the committers can spend sometime to review the KIP so we can make progress on this. Thanks Parth On 3/18/15, 2:20 PM, "Michael Herstine" <mherst...@linkedin.com.INVALID<mailto:mherst...@linkedin.com.INVALID>> wrote: >Hi Parth, > >Thanks! A few questions: > >1. Do you want to permit rules in your ACLs that DENY access as well as >ALLOW? This can be handy setting up rules that have exceptions. E.g. >“Allow principal P to READ resource R from all hosts” with “Deny principal >P READ access to resource R from host H1” in combination would allow P to >READ R from all hosts *except* H1. > >2. When a topic is newly created, will there be an ACL created for it? If >not, would that not deny subsequent access to it? > >(nit) Maybe use Principal instead of String to represent principals? > > >On 3/9/15, 11:48 AM, "Don Bosco Durai" ><bo...@apache.org<mailto:bo...@apache.org>> wrote: > >>Parth >> >>Overall it is looking good. Couple of questionsŠ >> >>- Can you give an example how the policies will look like in the default >>implementation? >>- In the operations, can we support ³CONNECT² also? This can be used >>during Session connection >>- Regarding access control for ³Topic Creation², since we can¹t do it on >>the server side, can we de-scope it for? And plan it as a future feature >>request? >> >>Thanks >> >>Bosco >> >> >> >>On 3/6/15, 8:10 AM, "Harsha" <ka...@harsha.io<mailto:ka...@harsha.io>> wrote: >> >>>Hi Parth, >>> Thanks for putting this together. Overall it looks good to >>> me. Although AdminUtils is a concern KIP-4 can probably fix >>> that part. >>>Thanks, >>>Harsha >>> >>>On Thu, Mar 5, 2015, at 10:39 AM, Parth Brahmbhatt wrote: >>>> Forgot to add links to wiki and jira. >>>> >>>> Link to wiki: >>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorizatio >>>>n >>>>+ >>>>Interface >>>> Link to Jira: https://issues.apache.org/jira/browse/KAFKA-1688 >>>> >>>> Thanks >>>> Parth >>>> >>>> From: Parth Brahmbhatt >>>> <pbrahmbh...@hortonworks.com<mailto:pbrahmbh...@hortonworks.com><mailto:pbrahmbh...@hortonworks.com>> >>>> Date: Thursday, March 5, 2015 at 10:33 AM >>>> To: >>>> "dev@kafka.apache.org<mailto:dev@kafka.apache.org><mailto:dev@kafka.apache.org>" >>>> <dev@kafka.apache.org<mailto:dev@kafka.apache.org><mailto:dev@kafka.apache.org>> >>>> Subject: [DISCUSS] KIP-11- Authorization design for kafka security >>>> >>>> Hi, >>>> >>>> KIP-11 is open for discussion , I have updated the wiki with the >>>>design >>>> and open questions. >>>> >>>> Thanks >>>> Parth >> >> > -- Thanks, Neha
