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 >
