1. *Support for running potentially long-running delegated tasks outside the network thread*: It is recommended that delegated tasks indicated by a handshake status of NEED_TASK are run on a separate thread since they may block ( http://docs.oracle.com/javase/7/docs/api/javax/net/ssl/SSLEngine.html). It is easier to encapsulate this in SSLChannel without any changes to common code if selection keys are managed within the Channel.
This makes sense I can change code to not do it on the network thread. Right now we are doing the handshake as part of the processor ( it shouldn’t be in acceptor) and we have multiple processors thread. Do we still see this as an issue if it happens on the same thread as processor? . -- Harsha Sent with Airmail On April 22, 2015 at 7:18:17 AM, Sriharsha Chintalapani (harsh...@fastmail.fm) wrote: Hi Rajini, Thanks for the details. I did go through your code . There was a discussion before about not having selector related code into the channel or extending the selector it self. 1. *Support for running potentially long-running delegated tasks outside the network thread*: It is recommended that delegated tasks indicated by a handshake status of NEED_TASK are run on a separate thread since they may block ( http://docs.oracle.com/javase/7/docs/api/javax/net/ssl/SSLEngine.html). It is easier to encapsulate this in SSLChannel without any changes to common code if selection keys are managed within the Channel. This makes sense I can change code to not do it on the network thread. 2. *Renegotiation handshake*: During a read operation, handshake status may indicate that renegotiation is required. It will be good to encapsulate this state change (and any knowledge of these SSL-specific state transitions) within SSLChannel. Our experience was that managing keys and state within the SSLChannel rather than in Selector made this code neater. Do we even want to support renegotiation. This is a case where user/client handshakes with server anonymously but later want to change and present their identity and establish a new SSL session. In our producer or consumers either present their identity ( two -way auth) or not. Since these are long running processes I don’t see that there might be a case where they initially establish the session and later present their identity. *Graceful shutdown of the SSL connection*s: Our experience was that we could encapsulate all of the logic for shutting down SSLEngine gracefully within SSLChannel when the selection key and state are owned and managed by SSLChannel. Can’t this be done when channel.close() is called any reason to own the selection key. 4. *And finally a minor point:* We found that by managing selection key and selection interests within SSLChannel, protocol-independent Selector didn't need the concept of handshake at all and all channel state management and handshake related code could be held in protocol-specific classes. This may be worth taking into consideration since it makes it easier for common network layer code to be maintained without any understanding of the details of individual security protocols. The only thing network code( SocketServer) is aware of channel isHandshakeComplete if its not do the handshake or go about read/write from channel. Yes socketServer need to be aware of channel is ready to read or not. But on the other hand there isn’t too many details of handshake leaked into socketServer. Either we let server know that a channel needs handshake or we keep the selectionKey state into channel which means we are adding selector related code into channel. Thanks, Harsha On April 22, 2015 at 3:56:04 AM, Rajini Sivaram (rajinisiva...@googlemail.com) wrote: When we were working on the client-side SSL implementation for Kafka, we found that returning selection interest from handshake() method wasn't sufficient to handle some of the SSL sequences. We resorted to managing the selection key and interest state within SSLChannel to avoid SSL-specific knowledge escaping out of SSL classes into protocol-independent network code. The current server-side SSL patch doesn't address these scenarios yet, but we may want to take these into account while designing the common Channel class/interface. 1. *Support for running potentially long-running delegated tasks outside the network thread*: It is recommended that delegated tasks indicated by a handshake status of NEED_TASK are run on a separate thread since they may block ( http://docs.oracle.com/javase/7/docs/api/javax/net/ssl/SSLEngine.html). It is easier to encapsulate this in SSLChannel without any changes to common code if selection keys are managed within the Channel. 2. *Renegotiation handshake*: During a read operation, handshake status may indicate that renegotiation is required. It will be good to encapsulate this state change (and any knowledge of these SSL-specific state transitions) within SSLChannel. Our experience was that managing keys and state within the SSLChannel rather than in Selector made this code neater. 3. *Graceful shutdown of the SSL connection*s: Our experience was that we could encapsulate all of the logic for shutting down SSLEngine gracefully within SSLChannel when the selection key and state are owned and managed by SSLChannel. 4. *And finally a minor point:* We found that by managing selection key and selection interests within SSLChannel, protocol-independent Selector didn't need the concept of handshake at all and all channel state management and handshake related code could be held in protocol-specific classes. This may be worth taking into consideration since it makes it easier for common network layer code to be maintained without any understanding of the details of individual security protocols. The channel classes we used are included in the patch in https://issues.apache.org/jira/browse/KAFKA-1690. The patch contains unit tests to validate these scenarios as well as other buffer overflow conditions which may be useful for server-side code when the scenarios described above are implemented. Regards, Rajini On Tue, Apr 21, 2015 at 11:13 PM, Sriharsha Chintalapani < harsh...@fastmail.fm> wrote: > Hi Jay, > Thanks for the review. > > 1. Isn't the blocking handshake going to be a performance concern? Can > we > do the handshake non-blocking instead? If anything that causes connections > to drop can incur blocking network roundtrips won't that eat up all the > network threads immediately? I guess I would have to look at that code to > know... > I’ve non-blocking handshake on the server side as well as for new > producer client. Blocking handshake is only done for BlockingChannel.scala > and it just loops over the non-blocking hand shake until the context is > established. So on the server side (SocketServer.scala) as it goes through > the steps and returns “READ or WRITE” signal for next step. For > BlockingChannel the worst case I look at is the connection timeout but most > times this handshake will finish up much quicker . I am cleaning up the > code will send up a patch in next few days . > > 2. Do we need to support blocking channel at all? That is just for the old > clients, and I think we should probably just leave those be to reduce > scope > here. > So blocking channel used not only by simple consumer but also > ControllerChannelManager and controlled shutdown also. Are we planning on > deprecating it. I think at least for ControllerChannelManager it makes > sense to have a blocking channel. If the users want to lock down the > cluster i.e no PLAINTEXT channels are allowed than all the communication > has to go through either SSL and KERBEROS so in this case we need add this > capability to BlockingChannel. > > > > 3. Can we change the APIs to drop the getters when that is not required by > the API being implemented. In general we don't use setters and getters as > a > naming convention. > > My bad on adding getters and setters :). I’ll work on removing it and > change the KIP accordingly. I still need some accessor methods though . > > Thanks, > > Harsha > > > > On April 21, 2015 at 2:51:15 PM, Jay Kreps (jay.kr...@gmail.com) wrote: > > Hey Sriharsha, > > Thanks for the excellent write-up. > > Couple of minor questions: > > 1. Isn't the blocking handshake going to be a performance concern? Can we > do the handshake non-blocking instead? If anything that causes connections > to drop can incur blocking network roundtrips won't that eat up all the > network threads immediately? I guess I would have to look at that code to > know... > > 2. Do we need to support blocking channel at all? That is just for the old > clients, and I think we should probably just leave those be to reduce scope > here. > > 3. Can we change the APIs to drop the getters when that is not required by > the API being implemented. In general we don't use setters and getters as a > naming convention. > > The long explanation on that is that setters/getters kind of imply a style > of java programming where you have simple structs with getters and setters > for each field. In general we try to have access methods only when > necessary, and rather than setters model the full change or action being > carried out, and if possible disallow change entirely. This is more in line > with modern java style I think. We aren't perfect in following this, but > once you start with getters and setters people start just adding them > everywhere and then using them. > > -Jay > > > On Mon, Apr 20, 2015 at 10:42 AM, Sriharsha Chintalapani <ka...@harsha.io> > wrote: > > > Hi, > > I updated the KIP-12 with more details. Please take a look > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51809888 > > > > Thanks, > > Harsha > > > > > > On February 11, 2015 at 10:02:43 AM, Harsha (ka...@harsha.io) wrote: > > > > Thanks Joe. It will be part of KafkaServer and will run on its own > > thread. Since each kafka server will run with a keytab we should make > > sure they are all getting renewed. > > > > On Wed, Feb 11, 2015, at 10:00 AM, Joe Stein wrote: > > > Thanks Harsha, looks good so far. How were you thinking of running > > > the KerberosTicketManager as a standalone process or like controller or > > > is > > > it a layer of code that does the plumbing pieces everywhere? > > > > > > ~ Joestein > > > > > > On Wed, Feb 11, 2015 at 12:18 PM, Harsha <ka...@harsha.io> wrote: > > > > > > > Hi, > > > > Here is the initial proposal for sasl/kerberos implementation for > > > > kafka https://cwiki.apache.org/confluence/x/YI4WAw > > > > and JIRA https://issues.apache.org/jira/browse/KAFKA-1686. I am > > > > currently working on prototype which will add more details to the > KIP. > > > > Just opening the thread to say the work is in progress. I'll update > the > > > > thread with a initial prototype patch. > > > > Thanks, > > > > Harsha > > > > > > >
