Hi Rajini, Thanks for the KIP. I am in favour of introducing SCRAM as an additional SASL mechanism. A few comments:
1. Magnus raised the point that cyrus-sasl currently only implements SCRAM-SHA-1, so having a larger number of variants will involve more work for non-Java clients. Do we really need all of SCRAM-SHA-224, SCRAM-SHA-256, SCRAM-SHA-384 and SCRAM-SHA-512? 2. Like Radai mentioned, it seems a bit weird to use single letter keys. The space savings don't seem particularly useful given the size of the values. You mentioned that it would not be useful for humans to interpret them, but we do expose them via the configs tool. Since we have to add more keys, I am also not convinced about the benefit of reusing the same letters as for SCRAM messages. 3. Have we given any thought on whether the znodes storing the authentication information should have additional access restrictions (even though they are hashed and salted)? Is the assumption that people would enable ZK authentication? Ismael On Tue, Nov 15, 2016 at 7:25 PM, Rajini Sivaram < rajinisiva...@googlemail.com> wrote: > Radai, > > I don't have a strong objection to using a more verbose format. But the > reasons for choosing the cryptic s=<salt>,t=<storedKey>,... format: > > 1. Unlike other properties like quotas stored in Zookeeper which need to > be human readable in order to query the values, these values only need > to > be parsed by code. The base64-encoded array of bytes don't really mean > anything to a human. You would only ever want to check if the user has > credentials for a mechanism, you can't really tell what the credentials > are > from the value stored in ZK. > 2. Single letter keys save space. Agree, it is not much, but since a > more verbose format doesn't add much value, it feels like wasted space > in > ZK to store long key names for each property for each user for each > mechanism. > 3. SCRAM authentication messages defined in RFC 5802 > <https://tools.ietf.org/html/rfc5802> use comma-separated key=value > pairs with single letter keys. s=<salt> and i=<iterations> appear > exactly > like that in SCRAM messages. Server key and stored key are not > exchanged, > so I chose two unused letters. The same parser used for SCRAM messages > is > used to parse this persisted value as well since the format is the same. > > > On Tue, Nov 15, 2016 at 5:02 PM, radai <radai.rosenbl...@gmail.com> wrote: > > > small nitpick - given that s,t,k and i are used as part of a rather large > > CSV format, what is the gain in having them be single letter aliases? > > in other words - why not salt=... , serverKey=... , storedKey=... , > > iterations=... ? > > > > On Tue, Nov 15, 2016 at 7:26 AM, Mickael Maison < > mickael.mai...@gmail.com> > > wrote: > > > > > +1 > > > > > > On Tue, Nov 15, 2016 at 10:57 AM, Rajini Sivaram > > > <rajinisiva...@googlemail.com> wrote: > > > > Jun, > > > > > > > > Thank you, I have made the updates to the KIP. > > > > > > > > On Tue, Nov 15, 2016 at 12:34 AM, Jun Rao <j...@confluent.io> wrote: > > > > > > > >> Hi, Rajini, > > > >> > > > >> Thanks for the proposal. +1. A few minor comments. > > > >> > > > >> 30. Could you add that the broker config sasl.enabled.mechanisms can > > now > > > >> take more values? > > > >> > > > >> 31. Could you document the meaning of s,t,k,i used in > > > /config/users/alice > > > >> in ZK? > > > >> > > > >> 32. In the rejected section, could you document why we decided not > to > > > bump > > > >> up the version of SaslHandshakeRequest? > > > >> > > > >> Jun > > > >> > > > >> > > > >> On Mon, Nov 14, 2016 at 5:57 AM, Rajini Sivaram < > > > >> rajinisiva...@googlemail.com> wrote: > > > >> > > > >> > Hi all, > > > >> > > > > >> > I would like to initiate the voting process for *KIP-84: Support > > > >> SASL/SCRAM > > > >> > mechanisms*: > > > >> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > >> > 84%3A+Support+SASL+SCRAM+mechanisms > > > >> > > > > >> > This KIP adds support for four SCRAM mechanisms (SHA-224, SHA-256, > > > >> SHA-384 > > > >> > and SHA-512) for SASL authentication, giving more choice for users > > to > > > >> > configure security. When delegation token support is added to > Kafka, > > > >> SCRAM > > > >> > will also support secure authentication using delegation tokens. > > > >> > > > > >> > Thank you... > > > >> > > > > >> > Regards, > > > >> > > > > >> > Rajini > > > >> > > > > >> > > > > > > > > > > > > > > > > -- > > > > Regards, > > > > > > > > Rajini > > > > > > > > > -- > Regards, > > Rajini >
