KAFKA-3221: Pass requesting user information to authorizer to enable …

[SinghAsDev]

…user authorization before modifying acls.

[SinghAsDev]

@ijuma @gwenshap mind taking a look. I have added reasoning behind adding hadoop-core dependency. I am open to suggestion, if that can be avoided somehow. Without changing APIs, which seems not essential for default authorizer, I could not find a better way to pass requesting user's info to authorizer. Suggestions are welcome.

[gwenshap]

I don't think the Hadoop behavior is desirable (we probably want "nobody" if a user is not authenticated with Kerberos, not the OS user), and I'm definitely not a fan of this dependency.

system.username is definitely not desirable since its an environment variable and completely insecure.
Since Hadoop manages to figure out the current principal (and so does kinit), I assume it is doable. Why not take a look at the Hadoop code and see how its done?
Then we can also separate the behavior we need from behavior that is specific to Hadoop (use of OS name, specific format for Kerberos principals, etc)

[ijuma]

Agreed with @gwenshap. In terms of the API changes to the Authorizer, do you mean adding a Session to every method related to ACLs so that the Authorizer can decide whether the principal can view and modify ACLs? If this is important, we should consider ways of doing it in a compatible manner (or, as a last resort, we could consider an incompatible change for 0.10.0.0 if it's easy for users to adapt their Authorizer implementations).

[harshach]

@SinghAsDev I am not sure I follow the intent of the PR. Ideally we should move all these authorizer, create topic etc.. as broker requests(KIP-4) than we can add authorizer functionality for these admin requests.

[Parth-Brahmbhatt]

+1 on moving all authorizer apis to broker side.

[granthenke]

I agree with these requests going through the broker. There are quite a few requests that need to be added as part of KIP-4. I am working on the topic ones now (as those have been agreed to already) and then plan to open a discussion about the other needed protocol messages including authorizer and version (also KIP-35) messages. All of these request/scripts would then use the AdminClient.

Once the first few get reviewed. I should be able to add these fairly quickly. I have an update pull request for the CreateTopic implementation at #626, and will post a one for DeleteTopic shortly.

[SinghAsDev]

Let me try to summarize all the discussion here and on JIRA so far, so that we don't have to go back and forth.

We all agree that there is a problem in the way we allow users to perform CRUD on acls and that problem is the user does not get authenticated. This opens up a security hole in current authorization support of Kafka. Default authorizer, SimpleAclAuthorizer, relies on ZK authentication to address this. However, there is no information provided to custom authorizers (via current APIs) to be able to determine if a particular request from a user should be allowed or not.

To fix this we need to add user authentication, possible options are.

1. Add request/response for CRUD of acls. The requests will be performed on broker.
2. kafka-acls.sh gets the user information and passed it to authorizer, which then can perform authentication, which then can check if user has sufficient privileges for a requested action.

Let's talk about each of these approaches in detail.

1. Add request/response for CRUD of acls. Sure this sounds to be inline with general direction of moving all admin commands over wire. I am in favor of this. However, before we get comfortable with this idea, below are the concerns/ questions that we should think about.
   1. User info still needs to be obtained and passed to broker. Yes, using a common admin client that takes care of this will help.
   2. For user authentication on broker, we have couple of options.
      1. Let broker to the authentication. Currently there is no resource or model for which a user can be authorized to check if a user has privileges to perform CRUD on privileges. Even if we translate a user trying to modify an ACL on existing resource into some required acl on the resource, we need to authenticate the user for that resource via authorizer. Catch 22 situation? Honestly, I do not see a reason for expecting that authorizer should trust kafka for deciding if it should let a user to perform CRUD on privileges, etc. One might say, we should restrict these operations to kafka's admin user only, which will take care of the issue of authorizing users via authorizer before allowing/denying their request of CRUD ops. For that just imagine you have to go to your admin every time you have to perform chmod/chown on topics you have created, even to see perms as part of your ls command.
      2. Pass the user information to authorizer. Ok, so we are again talking about modifying APIs here, the problem this JIRA points at?
   3. The problem exists in 0.9, are we saying wait till 0.10 before we address this issue? Please correct me if I am wrong, but my understanding is that KIP-4 will probably make it to 0.10.
2. kafka-acls.sh gets and passes user info directly to authorizer. This does not require waiting on KIP-4, however this is probably not a long term solution. May serve as a solution till we have KIP-4 is done and incorporated completely. Passing user information can be done in a couple of ways.
   1. Modify APIs in a backwards compatible way, i.e, by default pass null as requesting user info.
   2. Pass this info as part of authorizer properties. As this is supposed to be a stop gap solution, this is not a terrible idea.

Whatever path we decide to take, we should come up with some solution for custom authorizers for 0.9. This issue makes custom authorizers not so useful. Please correct me if any of my aforementioned understanding is flawed.

@gwenshap you are correct if the user is not authenticated, there is no point in passing his/her info.

[harshach]

"This opens up a security hole in current authorization support of Kafka"
Can you explain why you think there is a security hole ?

"User info still needs to be obtained and passed to broker. Yes, using a common admin client that takes care of this will help."
Yes a NetworkClient to send requests to broker

"Let broker to the authentication."
once we move these requests to broker all kafka-acls need to is to grab the user credentials like consumer,producer do and send a request to kafka broker for add/remove acls. whats catch 22 here?

"Honestly, I do not see a reason for expecting that authorizer should trust kafka for deciding if it should let a user to perform CRUD on privileges, etc. One might say, we should restrict these operations to kafka's admin user only, which will take care of the issue of authorizing users via authorizer before allowing/denying their request of CRUD ops. For that just imagine you have to go to your admin every time you have to perform chmod/chown on topics you have created, even to see perms as part of your ls command."

1. What you mean by authorizer trusting kafka for deciding a user can perform admin actions or not.
   1.1 it already does by authorizing kafka authenticated user for all the other requests.
2. Admins can be determined by a config in kafka server.properties like super.users
3. If that's not convenient enough admin can add additional users as "admin" into authorizer store using kafka-acls.sh or to give additional permissions to selected users using the same interface.
4. As @Parth-Brahmbhatt pointed out if you have edit acl on a topic you can today add or remove acls for that topic as well.

The proposal we are making instead of the proposed changes in this PR is lets use the client/broker authentication which we already have to establish a session and authorize that session like we are doing for writes and reads from clients already.

"kafka-acls.sh gets and passes user info directly to authorizer. This does not require waiting on KIP-4, however this is probably not a long term solution. "
Since this is not a long term solution and we already have temporary solution which is to trust zookeeper acls and authentication.

Ideally all the requests and authorization should happen in kafka broker. We shouldn't be adding another authentication layer to authorizer and have another session.
Lets have proper fix which is to move these requests to broker side and if this is getting delayed by KIP-4 we can file another jira to address authorizer specific requests.

[SinghAsDev]

@harshach by security hole I mean, there is no way custom authorizers can restrict acls CRUD to authorized users, while allowing users to use kafka-acls.sh.

I had an offline chat with @granthenke and we might be able to get KIP-4 pieces in soon. I will punt on addressing the issue after KIP-4. Thanks for feedbacks.