Can you explain the process for a adding a new mechanism based on current KIP?
My thought is that if it requires modifying Apache Kafka code, it is not pluggable enough. On Mon, Mar 7, 2016 at 4:04 PM, Rajini Sivaram <rajinisiva...@googlemail.com> wrote: > Gwen, > > In cases where you want completely different callbacks for different > mechanisms, I was thinking that the choice would be between a map of > classes (mechanism -> callbackHandler class) or a delegator class that > chooses the appropriate callback handler class based on mechanism. I chose > the latter since it makes it easier to configure in Kafka. Since we create > a callback handler for each channel and configure it with the > client-selected mechanism, it is straightforward to have one wrapper class > that delegates to the right mechanism-specific class to handle callbacks. > In many cases, a single class may be sufficient (the PR uses a single > callback class for Kerberos and PLAIN). I do see your point about the > flexibility that multiple classes would provide, but since you need to be > able to associate the callback with a mechanism for this to be useful, I am > not sure if just a list would add value. > > Login class is slightly different since the proposal is to use a single > login context with multiple login modules to handle multiple mechanisms. In > this case, you want to perform login for all the mechanisms that are > enabled. And you want to call loginContext.login() only once. Again, you > can delegate to multiple classes if you wish to add some complex > mechanism-specific logic, but a single login class makes the mapping to a > single login context and the login cache more obvious (the PR has a test > that includes Kerberos and PLAIN). > > Thoughts? > > On Mon, Mar 7, 2016 at 9:57 PM, Gwen Shapira <g...@confluent.io> wrote: > >> 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 >> > > > > -- > Regards, > > Rajini
