Hi Christopher, sorry for the delay, I spent the whole day in meetings :-/
On Fri, Oct 16, 2015 at 11:42:38AM +0200, Christopher Faulet wrote: > Le 16/10/2015 10:38, Willy Tarreau a écrit : > >Thus this sparks a new question : when the cache is disabled, are we sure > >to always free the ssl_ctx on all error paths after it's generated ? Or > >are we certain that we always pass through ssl_sock_close() ? > > > > That's a good question. By greping on SSL_free, it should be good. OK. > >The other problem I'm having is related to how we manage the LRU cache. > >Is there a risk that we kill some objects in this cache while they're > >still in use ? > > The SSL_CTX and SSL objects are reference-counted objects, so there is > no problem. > > When a SSL_CTX object is created, its refcount is set to 1. When a SSL > connection use it, it is incremented and when the connection is closed, > it is decremented. Of course, it is also decremented when SSL_CTX_free > is called. > During a call to SSL_free or SSL_CTX_free, its reference count reaches > 0, the SSL_CTX object is freed. Note that SSL_free and SSL_CTX_free can > be called in any order. OK so the unused objects in the tree have a refcount of 1 while the used ones have 2 or more, thus the refcount is always valid. Good that also means we must not test if the tree is null or not in ssl_sock_close(), we must always free the ssl_ctx as long as it was dynamically created, so that its refcount decreases, otherwise it keeps increasing upon every reuse. > So, if a call to SSL_CTX_free is called whilst a SSL connection uses the > corresponding SSL_CTX object, there is no problem. Actually, this > happens when a SSL_CTX object is evicted from the cache. There is no > need to check if it is used by a connection or not. Not only it is not needed, but we must not. > We do not track any reference count on SSL_CTX, it is done internally by > openssl. The only thing we must do, is to know if it a generated > certificate I totally agree. > and to track if it is in the cache or not. And here I disagree for the reason explained above since this is already covered by the refcount. > >>finally, we can of course discuss the design of this feature. There is > >>no problem. I will be happy to find a more elegant way to handle it, if > >>it is possible. > > > >Ideally we'd have the info in the ssl_ctx itself, but I remember that > >Emeric > >told me a while ago that we couldn't store anything in an ssl_ctx. Thus I > >can understand that we can't easily "tag" the ssl_ctx as being statically > >or dynamically allocated, which is why I understand the need for the flag > >on the connection as an alternative. > > > > Well, I'm not an openssl guru. It is possible to store and retrieve data > on a SSL_CTX object using SSL_CTX_set_ex_data/SSL_CTX_get_ex_data > functions. But I don't know if this a good practice to use it. And I > don't know if this is expensive or not. That's also what Rémi suggested. I don't know how it's used, I'm seeing an index with it and that's already used for DH data, so I don't know how it mixes (if at all) with this. I'm not much concerned by the access cost in fact since we're supposed to access it once at session creation and once during the release. It's just that I don't understand how this works. Maybe the connection flag is simpler for now. Willy
