On Friday, September 9, 2016, Sijie Guo <[email protected]> wrote: > On Fri, Sep 9, 2016 at 1:16 PM, Arun M. Krishnakumar < > [email protected] <javascript:;>> wrote: > > > Thanks Sijie for the quick response. Comments in-line. > > > > On Friday, September 9, 2016, Sijie Guo <[email protected] <javascript:;>> > wrote: > > > > > On Fri, Sep 9, 2016 at 10:46 AM, Arun M. Krishnakumar < > > > [email protected] <javascript:;>> wrote: > > > > > > > Hi, > > > > > > > > I am implementing Mutual-TLS in our environment here. I looked at > > > > Bookkeeper-588 but we cannot support StartTLS, > > > > > > > > > Can you explain why you cannot support StartTLS? > > > > > > [AK]: IIRC StartTLS starts off with an insecure channel and then > switches > > to a secure one after a handshake. We want to have the channels secure > from > > the beginning. > > > > Gotcha. Any idea on contributing your change back? > > [AK]: I hope to do so. Need to clean it up a bit. That is the primary reason I want to ensure that these changes are acceptable.
> > > > > > > > > > so have an implementation > > > > that does plain SSL. I get details of the SSL setup using the java > > > > environment variables, I have followed the zookeeper model for this. > > > > > > > > > > > > > > > > > > > > > > > > We have a system with a high frequency of client certificate expiry. > > New > > > > certificates will be available quite a while before the old ones > > expire. > > > > Consider the following case: > > > > 1. Bookkeeper Client sends a request for AddEntry over SSL > > > > 2. Bookie adds the entry > > > > > > 3. Certificate expires and the communication channel becomes untrusted > > > > 4. The Bookie client is not able to receive a response from the > Bookie, > > > and > > > > it marks the Bookie as being in an invalid state. > > > > (PerChannelBookieClient::messageReceived processes a failure). > > > > > > > > > > > > > I am assuming here, you are talking about the request timeout as no > > > response coming back. Or the client received a failure? > > > > > > > > [AK]: I have not exactly simulated an expired certificate but yes, I > think > > we would receive a timeout. > > > > > > > > > > > > > > > Is the explanation above correct ? > > > > > > > > > > To avoid the above, I have a basic implementation of reconnect in the > > > > DefaultPerChannelBookieClientPool::obtain function, where I can > detect > > > > that > > > > the old certs are about to expire and new ones are available, and > > provide > > > > connections from a pool with channels initialized from the new pool. > > > > > > > > > > I am not very clear about this part. Are you saying you will pre-create > > > connection with new cert and replace the connection with old cert? > > > > > > > > [AK]: The main problem is that we do not want to interrupt existing > > messages until their completion is processed. Hence we need to wait for > > message completion. There are two ways to do this: > > 1. In the DefaultPerChannelBookieClientPool::obtain, if the current > > channel > > has a cert that will expire soon, and has a new cert somewhere in the > > system: > > a. Create a new channel with the new cert and return that channel > > b. Mark the old channel as frozen > > c. In a separate thread wait for the completionObjects of the channel > > to become empty and then disconnect the channel > > OR > > 2. Detect that the certificate is going to expire in another thread. If > so: > > a. Create a new pool of channels with the new cert > > b. Swap the new channels with the old channels > > c. In a separate thread wait for completionObjects of each channel to > > become empty, or wait for 2*(read | write timeouts). > > > > Gotcha. LGTM. It seems to me swapping the whole pool is a much > straightforward change. [AK]: that's perfect. I'll clean up my changes and send out a patch. Thanks, Arun > > > > > > In either case the main idea is to let the old channels not accept new > > requests, let them complete the current requests, and then to disconnect > + > > close them. > > > > Thanks, > > Arun > > > > > > > > > > > > > > > Could you comment if the above makes sense ? > > > > > > > > Thanks, > > > > Arun > > > > > > > > > >
