Hi Viktor and Colin, Thanks for taking a look.
The intent behind sasl.scram.password.change.enabled was to give the admins the choice about whether they're willing for passwords to be sent to the broker in the clear (after all they know their deployment and network best). I'm fine with removing it if people don't think that's going to be an issue. I guess alter on the cluster is fine with me. As you say we can always make it finer grained later if there's demand for users to be able to set their own passwords. The reason behind HashType is that currently a user can have both SCRAM-512 and SCRAM-256 credentials, but they don't have to use the same password (and logically it should be possible to set both or either in a given request). So it's the username+hash pair that identifies the credential. I guess using a Map (with keys for each hash type) would work. Since SCRAM stores salted and hashed passwords it's not possible to support retrieving the password because once it's stored the broker won't know it any more. Overall the KIP was written to replace the existing functionality for setting passwords from the script while still storing the information in ZK for now. I can certainly abstract an interface for storing SCRAM credentials, but really the store is tightly coupled with the supported mechanisms. For example, supposing for a minute that Kafka supported storing passwords for the SASL-PLAIN mechanism that credential store would not be usable for SCRAM, and vice versa. I'm not sure how useful it would be to have pluggable stores for SCRAM passwords, as opposed to delegating the authentication itself to some remote service. Finally wrt KIP-500, Jun's comment pertained specifically to the bootstrapping problem where the only broker listener is SCRAM -- how can we start the cluster in this case? One way to address that would be to provide the information to the broker at start up (from a file, for example). Each broker could use that file as its initial credential store and once the cluster is up and running those credentials would be added to the real store (the metadata log, or a topic) and the file deleted. I think this is orthogonal to KIP-506 as it stands right now. Kind regards, Tom On Mon, Oct 21, 2019 at 6:21 PM Colin McCabe <cmcc...@apache.org> wrote: > On Mon, Oct 21, 2019, at 03:59, Viktor Somogyi-Vass wrote: > > Hey Tom, > > > > Bringing over the conversation from KIP-500 and also adding my > > observations. Let me capture them in points. > > > > 1. Perhaps I'm a bit aggressive in introducing new features but I'm not > > sure if we need sasl.scram.password.change.enabled. As I understand it > > controls whether your can use the AdminClient API or not. I think we > > should > > enable it by default and control it with a strict ACL. > > Yeah, I agree. Please, let's not have a lot of extra configs. > > > 2. On this note I think we should include ACLs too. In KIP-373 ( > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-373%3A+Allow+users+to+create+delegation+tokens+for+other+users > ) > > I'll introduce User resource types so perhaps we can take that as an > > example. > > For changing a password, it's probably fine just to require ALTER on > CLUSTER. That's what most other administrative operations require. If we > want fine grained control of who can change whose password, we can add that > in a follow-on (I bet the demand will be scarce). > > > 3. To recap KIP-500 I think it would be useful to decouple Zookeeper > > entirely in this case from the server side. So we would just think about > > Zookeeper as a pluggable key management system, just as we do it with > > authorization. That way we solve the zookeeper connection problem. I > > think > > it would be useful to think of it this way as opposed to having only the > > controller(s) control this information too. The reasoning for this is > > that > > this way we can then outsource typical problems to the provider (like > > caching, securely distributing and storing keys in memory). So every > > broker > > would be connected to this key provider and they interact with via an > > interface, just as the authorizer does. At some point when KIP-500 is > > in a > > later phase we can extract this "Zookeeper implementation" to a > > separated > > module so it will be entirely decoupled or we can replace it with an > > actual > > key management system and authorizer. > > The other alternative is that we can write a protocol which distributes > > the > > SCRAM info among brokers. In this case though you'd have to think about > > the > > problems mentioned earlier which can be a bigger development and > > maintenance overhead. > > > > What do you think? > > Yeah, I agree. Actually, I would go a step further and say that we should > remove "scram" from the names of these functions. These are really generic > user/password management APIs and not specific to SCRAM. > > I think we should have a pluggable interface to support these APIs. The > default implementation can be to throw an exception indicating that the API > is not implemented. After all, not all user/group management systems will > support these operations. However, the ones that do will know how to > implement them. > > Rules about passwords, and so on, can be handled inside these pluggable > interfaces. They really have to be-- if we're changing the password in > some external system, it may have its own rules about passwords. We can > have an InvalidPasswordException for this case. The text can explain the > issue. > > Similarly, I would replace "HashType" with something like a generic map of > strings to strings named "options". > > For the response, can we just have an array of error codes and error > strings? I'm not sure why we need to repeat things like the hash type, > username, etc. since the caller already knows what they passed for those in > the request. Not repeating yourself is the most efficient encoding. > > I guess the other question is, do we want an API to show what the password > is for an existing user? It seems undeniably useful. And if we provide > the ability to set the password, there is little point in not providing the > ability to show what it is set to. > > best, > Colin > > > > > Viktor > > > > On Mon, Aug 12, 2019 at 11:22 AM Tom Bentley <tbent...@redhat.com> > wrote: > > > > > Hi All, > > > > > > I've written KIP-506 proposing an RPC and Admin interface for setting > SCRAM > > > user passwords. I think there could be an interesting discussion over > the > > > relative merits of hashing on the broker or client. In any case I'd be > > > grateful for any comments you may have: > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-506%3A+Allow+setting+SCRAM+password+via+Admin+interface > > > > > > Kind regards, > > > > > > Tom > > > > > >