On Fri, Sep 9, 2016 at 1:16 PM, Arun M. Krishnakumar < [email protected]> wrote:
> Thanks Sijie for the quick response. Comments in-line. > > On Friday, September 9, 2016, Sijie Guo <[email protected]> wrote: > > > On Fri, Sep 9, 2016 at 10:46 AM, Arun M. Krishnakumar < > > [email protected]> 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? > > > > > > 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. > > 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 > > > > > >
