Le 09/10/2015 12:19, Willy Tarreau a écrit :
On Fri, Oct 09, 2015 at 11:59:00AM +0200, Christopher Faulet wrote:
Le 09/10/2015 10:27, Willy Tarreau a écrit :
Hi Christopher,
I applied the first two ones, but the last one seems to be doing
a lot of stuff at the same time. It's not even clear to me whether
it fixes something or improves something or does both, but the
review is quite hard. Is it possible to cut it into functional
parts ? In practice we always want to do one patch per feature or
per bug fix. If you don't think it can be easily cut by now, we
can still blindly apply it but I still don't feel at ease given
the description which seems to cover multiple aspects :-/
Hi Willy,
Thanks for your work and your feedback. I have split my patch in 3 parts.
Thanks for the quick response. I've applied them. During a build attempt
I noticed that your previous patch "BUG/MINOR: ssl: fix management..."
broke the build here due to some missing #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
that I had to reintroduce (openssl 0.9.8 here).
I thought that an alternative could simply be to declare ssl_ctx_lru_tree
oustide of the ifdef, but I noticed there is a new test in ssl_sock_close()
based on ssl_ctx_lru_tree being NULL, so I'm having doubts now about doing
this since I don't understand why this SSL_CTX_free() call which depends
on the ability to build a cert on the fly only has to be called if the
cert tree is not initialized :-/
I would appreciate it if you could double-check. Thanks!
You're right. I didn't checked it on old versions of openssl. My bad.
ssl_ctx_lru_tree could be defined outside the ifdef, but it is only used
when SNI extension is available. So there is no reason to initialize it
if there is no SNI.
Then, when SNI is available, the tree can be NULL if the cache of
generated certificates is disabled (tune.ssl.ssl-ctx-cache-size == 0).
So, in this situation, we need to free the certificate when the SSL
connection is closed to avoid memory leak. We could want to generate
dynamically SSL certificates without any cache.
--
Christopher Faulet
