Hi, Before I can vote on this KIP, I have two additional questions / comments on the new configuration:
1. sasl.callback.handler.class - it looks like we want a single class that implements all mechanisms. I think this will make it difficult to extend since the only way I can add a mechanism will be by implementing every single existing mechanism (otherwise customers will need to choose between new and existing when selecting which class to use). If Microsoft releases a proprietary "AD Mechanism" and Oracle releases "OID mechanism", there will be no class that implements both. Can we make it a list of classes instead? I realize this complicates the code a bit (some kind of factory will be required to choose the right class to use), but important IMO. 2. similar for sasl.login.class - if I have a class for Kerberos (with refresh thread) and a class for "plain", we need to be able to load both. Gwen On Wed, Mar 2, 2016 at 12:30 AM, Rajini Sivaram <rajinisiva...@googlemail.com> wrote: > Jun, > > Thanks, I have added a note to the KIP. I will add a comment in the > implementation and also add a unit test to ensure that conflicts are > avoided when version number is modified. > > On Tue, Mar 1, 2016 at 5:43 PM, Jun Rao <j...@confluent.io> wrote: > >> Rajini, >> >> Thanks for the explanation. For 1, this implies that we have to be careful >> with changing the 2-byte version in the future to avoid conflict. Could you >> document this in the KIP and also in the implementation? >> >> Jun >> >> On Tue, Mar 1, 2016 at 2:47 AM, Rajini Sivaram < >> rajinisiva...@googlemail.com >> > wrote: >> >> > Jun, >> > >> > Thank you for the review. >> > >> > >> > 1. With GSSAPI, the first context establishment packet starts with the >> > byte 0x60 (tag for APPLICATION-0) followed by a variable-length >> encoded >> > size, followed by various tags and contents. And the packet also >> > contains a >> > checksum. This is completely different from the mechanism packet from >> > Kafka >> > clients which start with a two-byte version set to zero currently, >> > followed >> > by just a String mechanism. >> > 2. Agreed, I have removed the version from the server response in the >> > KIP. Thanks. >> > >> > >> > On Tue, Mar 1, 2016 at 2:33 AM, Jun Rao <j...@confluent.io> wrote: >> > >> > > Rajini, >> > > >> > > Thanks for the updates. Just a couple of minor comments. >> > > >> > > 1. With the default GSSAPI, what's the first packet that the client >> sends >> > > to the server? Is that completely different from the packet format that >> > we >> > > will use for non-GSSAPI mechanisms? >> > > >> > > 2. In the server response, it doesn't seem that we need to include the >> > > version since the client knows the version of the request that it >> sends. >> > > >> > > Jun >> > > >> > > On Mon, Feb 29, 2016 at 10:14 AM, Rajini Sivaram < >> > > rajinisiva...@googlemail.com> wrote: >> > > >> > > > Harsha, >> > > > >> > > > Thank you for the review. I will wait another day to see if there is >> > more >> > > > feedback and then start a voting thread. >> > > > >> > > > Rajini >> > > > >> > > > On Mon, Feb 29, 2016 at 2:51 PM, Harsha <ka...@harsha.io> wrote: >> > > > >> > > > > Rajini, >> > > > > Thanks for the changes to the KIP. It looks good to >> > me. I >> > > > > think we can move to voting. >> > > > > Thanks, >> > > > > Harsha >> > > > > >> > > > > On Mon, Feb 29, 2016, at 12:43 AM, Rajini Sivaram wrote: >> > > > > > I have added some more detail to the KIP based on the discussion >> in >> > > the >> > > > > > last KIP meeting to simplify support for multiple mechanisms. >> Have >> > > also >> > > > > > changed the property names to reflect this. >> > > > > > >> > > > > > Also updated the PR in >> > > > https://issues.apache.org/jira/browse/KAFKA-3149 >> > > > > > to >> > > > > > reflect the KIP. >> > > > > > >> > > > > > Any feedback is appreciated. >> > > > > > >> > > > > > >> > > > > > On Tue, Feb 23, 2016 at 9:36 PM, Rajini Sivaram < >> > > > > > rajinisiva...@googlemail.com> wrote: >> > > > > > >> > > > > > > I have updated the KIP based on the discussion in the KIP >> meeting >> > > > > today. >> > > > > > > >> > > > > > > Comments and feedback are welcome. >> > > > > > > >> > > > > > > On Wed, Feb 3, 2016 at 7:20 PM, Rajini Sivaram < >> > > > > > > rajinisiva...@googlemail.com> wrote: >> > > > > > > >> > > > > > >> Hi Harsha, >> > > > > > >> >> > > > > > >> Thank you for the review. Can you clarify - I think you are >> > saying >> > > > > that >> > > > > > >> the client should send its mechanism over the wire to the >> > server. >> > > Is >> > > > > that >> > > > > > >> correct? The exchange is slightly different in the KIP (the PR >> > > > > matches the >> > > > > > >> KIP) from the one you described to enable interoperability >> with >> > > > > 0.9.0.0. >> > > > > > >> >> > > > > > >> >> > > > > > >> On Wed, Feb 3, 2016 at 1:56 PM, Harsha <m...@harsha.io> >> wrote: >> > > > > > >> >> > > > > > >>> Rajini, >> > > > > > >>> I looked at the PR you have. I think its better >> with >> > > > your >> > > > > > >>> earlier approach rather than extending the >> protocol. >> > > > > > >>> What I was thinking initially is, Broker has a config option >> of >> > > say >> > > > > > >>> sasl.mechanism = GSSAPI, PLAIN >> > > > > > >>> and the client can have similar config of >> sasl.mechanism=PLAIN. >> > > > > Client >> > > > > > >>> can send its sasl mechanism before the handshake starts and >> if >> > > the >> > > > > > >>> broker accepts that particular mechanism than it can go ahead >> > > with >> > > > > > >>> handshake otherwise return a error saying that the mechanism >> > not >> > > > > > >>> allowed. >> > > > > > >>> >> > > > > > >>> Thanks, >> > > > > > >>> Harsha >> > > > > > >>> >> > > > > > >>> On Wed, Feb 3, 2016, at 04:58 AM, Rajini Sivaram wrote: >> > > > > > >>> > A slightly different approach for supporting different SASL >> > > > > mechanisms >> > > > > > >>> > within a broker is to allow the same "*security protocol*" >> to >> > > be >> > > > > used >> > > > > > >>> on >> > > > > > >>> > different ports with different configuration options. An >> > > > advantage >> > > > > of >> > > > > > >>> > this >> > > > > > >>> > approach is that it extends the configurability of not just >> > > SASL, >> > > > > but >> > > > > > >>> any >> > > > > > >>> > protocol. For instance, it would enable the use of SSL with >> > > > mutual >> > > > > > >>> client >> > > > > > >>> > authentication on one port or different certificate chains >> on >> > > > > another. >> > > > > > >>> > And >> > > > > > >>> > it avoids the need for SASL mechanism negotiation. >> > > > > > >>> > >> > > > > > >>> > Kafka would have the same "*security protocols" *defined as >> > > > today, >> > > > > but >> > > > > > >>> > with >> > > > > > >>> > (a single) configurable SASL mechanism. To have different >> > > > > > >>> configurations >> > > > > > >>> > of >> > > > > > >>> > a protocol within a broker, users can define new protocol >> > names >> > > > > which >> > > > > > >>> are >> > > > > > >>> > configured versions of existing protocols, perhaps using >> just >> > > > > > >>> > configuration >> > > > > > >>> > entries and no additional code. >> > > > > > >>> > >> > > > > > >>> > For example: >> > > > > > >>> > >> > > > > > >>> > A single mechanism broker would be configured as: >> > > > > > >>> > >> > > > > > >>> > listeners=SASL_SSL://:9092 >> > > > > > >>> > sasl.mechanism=GSSAPI >> > > > > > >>> > sasl.kerberos.class.name=kafka >> > > > > > >>> > ... >> > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > And a multi-mechanism broker would be configured as: >> > > > > > >>> > >> > > > > > >>> > listeners=gssapi://:9092,plain://:9093,custom://:9094 >> > > > > > >>> > gssapi.security.protocol=SASL_SSL >> > > > > > >>> > gssapi.sasl.mechanism=GSSAPI >> > > > > > >>> > gssapi.sasl.kerberos.class.name=kafka >> > > > > > >>> > ... >> > > > > > >>> > plain.security.protocol=SASL_SSL >> > > > > > >>> > plain.sasl.mechanism=PLAIN >> > > > > > >>> > .. >> > > > > > >>> > custom.security.protocol=SASL_PLAINTEXT >> > > > > > >>> > custom.sasl.mechanism=CUSTOM >> > > > > > >>> > >> > > custom.sasl.callback.handler.class=example.CustomCallbackHandler >> > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > This is still a big change because it affects the currently >> > > fixed >> > > > > > >>> > enumeration of security protocol definitions, but one that >> is >> > > > > perhaps >> > > > > > >>> > more >> > > > > > >>> > flexible than defining every new SASL mechanism as a new >> > > security >> > > > > > >>> > protocol. >> > > > > > >>> > >> > > > > > >>> > Thoughts? >> > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > On Tue, Feb 2, 2016 at 12:20 PM, Rajini Sivaram < >> > > > > > >>> > rajinisiva...@googlemail.com> wrote: >> > > > > > >>> > >> > > > > > >>> > > As Ismael has said, we do not have a requirement to >> support >> > > > > multiple >> > > > > > >>> > > protocols in a broker. But I agree with Jun's observation >> > > that >> > > > > some >> > > > > > >>> > > companies might want to support a different >> authentication >> > > > > mechanism >> > > > > > >>> for >> > > > > > >>> > > internal users or partners. For instance, we do use two >> > > > different >> > > > > > >>> > > authentication mechanisms, it just so happens that we are >> > > able >> > > > > to use >> > > > > > >>> > > certificate-based authentication for internal users, and >> > > hence >> > > > > don't >> > > > > > >>> > > require multiple SASL mechanisms in a broker. >> > > > > > >>> > > >> > > > > > >>> > > As Tao has pointed out, mechanism negotiation is a common >> > > usage >> > > > > > >>> pattern. >> > > > > > >>> > > Many existing protocols that support SASL do already use >> > this >> > > > > > >>> pattern. AMQP >> > > > > > >>> > > ( >> > > > > > >>> > > >> > > > > > >>> >> > > > > >> > > > >> > > >> > >> http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-security-v1.0-os.html#type-sasl-mechanisms >> > > > > > >>> ), >> > > > > > >>> > > which, as a messaging protocol maybe closer to Kafka in >> use >> > > > cases >> > > > > > >>> than >> > > > > > >>> > > Zookeeper, is an example. Other examples where the client >> > > > > negotiates >> > > > > > >>> or >> > > > > > >>> > > sends SASL mechanism to server include ACAP that is used >> as >> > > an >> > > > > > >>> example in >> > > > > > >>> > > the SASL RFCs, POP3, LDAP, SMTP etc. This is not to say >> > that >> > > > > Kafka >> > > > > > >>> > > shouldn't use a different type of mechanism selection >> that >> > > fits >> > > > > > >>> better with >> > > > > > >>> > > the existing Kafka design. Just that negotiation is a >> > common >> > > > > pattern >> > > > > > >>> and >> > > > > > >>> > > since we typically turn on javax.net.debug to debug TLS >> > > > > negotiation >> > > > > > >>> issues, >> > > > > > >>> > > having to use Kafka logging to debug SASL negotiation >> > issues >> > > is >> > > > > not >> > > > > > >>> that >> > > > > > >>> > > dissimilar. >> > > > > > >>> > > >> > > > > > >>> > > >> > > > > > >>> > > On Tue, Feb 2, 2016 at 6:12 AM, tao xiao < >> > > xiaotao...@gmail.com >> > > > > >> > > > > > >>> wrote: >> > > > > > >>> > > >> > > > > > >>> > >> I am the author of KIP-44. I hope my use case will add >> > some >> > > > > values >> > > > > > >>> to this >> > > > > > >>> > >> discussion. The reason I raised KIP44 is that I want to >> be >> > > > able >> > > > > to >> > > > > > >>> > >> implement a custom security protocol that can fulfill >> the >> > > need >> > > > > of my >> > > > > > >>> > >> company. As pointed out by Ismael KIP-43 now supports a >> > > > > pluggable >> > > > > > >>> way to >> > > > > > >>> > >> inject custom security provider to SASL I think it is >> > enough >> > > > to >> > > > > > >>> cover the >> > > > > > >>> > >> use case I have and address the concerns raised in >> KIP-44. >> > > > > > >>> > >> >> > > > > > >>> > >> For multiple security protocols support simultaneously >> it >> > is >> > > > not >> > > > > > >>> needed in >> > > > > > >>> > >> my use case and I don't foresee it is needed in the >> future >> > > but >> > > > > as i >> > > > > > >>> said >> > > > > > >>> > >> this is my use case only there may be other use cases >> that >> > > > need >> > > > > it. >> > > > > > >>> But if >> > > > > > >>> > >> we want to support it in the future I prefer to get it >> > right >> > > > at >> > > > > the >> > > > > > >>> first >> > > > > > >>> > >> place given the fact that security protocol is an ENUM >> and >> > > if >> > > > we >> > > > > > >>> stick to >> > > > > > >>> > >> that implementation it is very hard to extend in the >> > future >> > > > > when we >> > > > > > >>> decide >> > > > > > >>> > >> multiple security protocols is needed. >> > > > > > >>> > >> >> > > > > > >>> > >> Protocol negotiation is a very common usage pattern in >> > > > security >> > > > > > >>> domain. As >> > > > > > >>> > >> suggested in Java SASL doc >> > > > > > >>> > >> >> > > > > > >>> > >> >> > > > > > >>> >> > > > > >> > > > >> > > >> > >> http://docs.oracle.com/javase/7/docs/technotes/guides/security/sasl/sasl-refguide.html >> > > > > > >>> > >> client >> > > > > > >>> > >> first sends out a packet to server and server responds >> > with >> > > a >> > > > > list >> > > > > > >>> of >> > > > > > >>> > >> mechanisms it supports. This is very similar to SSL/TLS >> > > > > negotiation. >> > > > > > >>> > >> >> > > > > > >>> > >> On Tue, 2 Feb 2016 at 06:39 Ismael Juma < >> > ism...@juma.me.uk> >> > > > > wrote: >> > > > > > >>> > >> >> > > > > > >>> > >> > On Mon, Feb 1, 2016 at 7:04 PM, Gwen Shapira < >> > > > > g...@confluent.io> >> > > > > > >>> wrote: >> > > > > > >>> > >> > >> > > > > > >>> > >> > > Looking at "existing solutions", it looks like >> > Zookeeper >> > > > > allows >> > > > > > >>> > >> plugging >> > > > > > >>> > >> > in >> > > > > > >>> > >> > > any SASL mechanism, but the server will only support >> > one >> > > > > > >>> mechanism at >> > > > > > >>> > >> a >> > > > > > >>> > >> > > time. >> > > > > > >>> > >> > > >> > > > > > >>> > >> > >> > > > > > >>> > >> > This was the original proposal from Rajini as that is >> > > enough >> > > > > for >> > > > > > >>> their >> > > > > > >>> > >> > needs. >> > > > > > >>> > >> > >> > > > > > >>> > >> > >> > > > > > >>> > >> > > If this is good enough for our use-case (do we >> > actually >> > > > > need to >> > > > > > >>> > >> support >> > > > > > >>> > >> > > multiple mechanisms at once?), it will simplify >> life a >> > > lot >> > > > > for >> > > > > > >>> us ( >> > > > > > >>> > >> > > >> > > > > > >>> > >> >> > > > > > >>> >> > > > > >> > > >> https://cwiki.apache.org/confluence/display/ZOOKEEPER/Zookeeper+and+SASL >> > > > > > >>> > >> > ) >> > > > > > >>> > >> > > >> > > > > > >>> > >> > >> > > > > > >>> > >> > The current thinking is that it would be useful to >> > support >> > > > > > >>> multiple SASL >> > > > > > >>> > >> > mechanisms simultaneously. In the KIP meeting, Jun >> > > mentioned >> > > > > that >> > > > > > >>> > >> companies >> > > > > > >>> > >> > sometimes support additional authentication mechanisms >> > for >> > > > > > >>> partners, for >> > > > > > >>> > >> > example. It does make things more complex, as you say, >> > so >> > > we >> > > > > need >> > > > > > >>> to be >> > > > > > >>> > >> > sure the complexity is worth it. >> > > > > > >>> > >> > >> > > > > > >>> > >> > Two more points: >> > > > > > >>> > >> > >> > > > > > >>> > >> > 1. It has been suggested that custom security protocol >> > > > > support is >> > > > > > >>> > >> needed by >> > > > > > >>> > >> > some (KIP-44). Rajini enhanced KIP-43 so that a SASL >> > > > mechanism >> > > > > > >>> with a >> > > > > > >>> > >> > custom provider can be used for this purpose instead. >> > > Given >> > > > > this, >> > > > > > >>> it >> > > > > > >>> > >> seems >> > > > > > >>> > >> > a bit inconsistent and restrictive not to allow >> multiple >> > > > SASL >> > > > > > >>> mechanisms >> > > > > > >>> > >> > simultaneously (we do allow SSL and SASL >> authentication >> > > > > > >>> simultaneously, >> > > > > > >>> > >> > after all). >> > > > > > >>> > >> > >> > > > > > >>> > >> > 2. The other option would be to support a single SASL >> > > > > mechanism >> > > > > > >>> > >> > simultaneously to start with and then extend this to >> > > > multiple >> > > > > > >>> mechanisms >> > > > > > >>> > >> > simultaneously later (if and when needed). It seems >> like >> > > it >> > > > > would >> > > > > > >>> be >> > > > > > >>> > >> harder >> > > > > > >>> > >> > to support the latter in the future if we go down this >> > > > route, >> > > > > but >> > > > > > >>> maybe >> > > > > > >>> > >> > there are ways around this. >> > > > > > >>> > >> > >> > > > > > >>> > >> > Thoughts? >> > > > > > >>> > >> > >> > > > > > >>> > >> > Ismael >> > > > > > >>> > >> > >> > > > > > >>> > >> >> > > > > > >>> > > >> > > > > > >>> > > >> > > > > > >>> > > >> > > > > > >>> > > -- >> > > > > > >>> > > Regards, >> > > > > > >>> > > >> > > > > > >>> > > Rajini >> > > > > > >>> > > >> > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > -- >> > > > > > >>> > Regards, >> > > > > > >>> > >> > > > > > >>> > Rajini >> > > > > > >>> >> > > > > > >> >> > > > > > >> >> > > > > > >> >> > > > > > >> -- >> > > > > > >> Regards, >> > > > > > >> >> > > > > > >> Rajini >> > > > > > >> >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > -- >> > > > > > > Regards, >> > > > > > > >> > > > > > > Rajini >> > > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > -- >> > > > > > Regards, >> > > > > > >> > > > > > Rajini >> > > > > >> > > > >> > > > >> > > > >> > > > -- >> > > > Regards, >> > > > >> > > > Rajini >> > > > >> > > >> > >> > >> > >> > -- >> > Regards, >> > >> > Rajini >> > >> > > > > -- > Regards, > > Rajini
