[openssl.org #311] Memory leak in session caching?
I've committed a fix for this that the requestor has tested, so I'm closing the ticket as well. -- __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: Memory leak in session caching?
On Mon, Oct 21, 2002 at 12:44:01PM -0400, Geoff Thorpe wrote: Thanks for the detailed report - Lutz has already created an RT ticket for this. I replied to the RT mail from him privately before noticing these followups (from you both) on modssl-users and openssl-dev, so I'll repeat here a suggestion I proposed in my mail to him, though I'll try to elaborate a bit. I think perhaps the correct solution would be to have things as follows; - SSL_SESS_CACHE_NO_INTERNAL_LOOKUP Lookups are not performed using the internal/local cache but solely through (any) external cache-lookup callback. Make sure that cache stores are nonetheless cached internally as well as through any external callback - as it was before and perhaps is still the case in some inconsistent buggy fashion, as you have suggested might be the case. :-) - SSL_SESS_CACHE_NO_INTERNAL_STORE (new flag) Cache stores are not performed against the internal/local cache but solely through (any) external cache-store callback. Lookups are nonetheless performed internally before using any external callback. - SSL_SESS_CACHE_NO_INTERNAL (new flag) This is (SSL_SESS_CACHE_NO_INTERNAL_LOOKUP|SSL_SESS_CACHE_NO_INTERNAL_STORE) Combination of the both the above, and the appropriate replacement for use in mod_ssl, Apache2, and other circumstances where fork() can introduce security problems with cache state. The reason I don't suggest changing the behaviour SSL_SESS_CACHE_NO_INTERNAL_LOOKUP to not store sessions is two-fold; (i) behavioural change of a flag that applications may be relying on and will introduce an unforeseen bug. (ii) it may actually be advantageous to have fine-grained control as I've suggested with the 3 flags (well, the third being an abbreviation for the OR of the first two). W.r.t. (ii) I can invisage NO_INTERNAL_LOOKUP being used on its own with the current behaviour - we may want a cache of sessions for manual inspection by the application without the application having to implement its own cache. With this flag, the application can be sure that the SSL/TLS code won't automatically resume them without going via its external callback - ie. the application itself will control those sessions it is prepared to accept or not. NO_INTERNAL_STORE may be similarly useful because the application can know the SSL/TLS implementation will automatically resume any resumable session in its cache, but the contents of that cache are only those sessions that the application has manually inserted into the cache of its own will - the SSL/TLS implementation won't automatically populate the cache. Obviously the combination will be useful too and in fact is what is desired in Apache2/mod_ssl. Any thoughts? I'm thinking out loud here - but will continue to think a little more carefully (and quietly) about it. The current behaviour is, that NO_INTERNAL_LOOKUP does not longer cache sessions internally since 0.9.6d. Nobody ever complained about this behavioural change. (Well it seems that the change I made at that time was incomplete and is to be fixed...) With respect to your discussion: if the extension to the API you proposed here is required/useful for at least one major application (Apache2/mod_ssl) I have no problems to: * revert the change in 0.9.6, such that NO_INTERNAL_LOOKUP does again store sessions into the cache, * fix the reported bug for the 0.9.7 (and later) tree, * add the proposed API extension. I would propose to make the API change already to the 0.9.7 tree, as I think that the change is small enough and making the behaviour consistent seems like a bugfix to me. Comments? Lutz -- Lutz Jaenicke [EMAIL PROTECTED] http://www.aet.TU-Cottbus.DE/personen/jaenicke/ BTU Cottbus, Allgemeine Elektrotechnik Universitaetsplatz 3-4, D-03044 Cottbus __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: Memory leak in session caching?
Hey Lutz (et al), On Tuesday 22 Oct 2002 3:11 am, Lutz Jaenicke wrote: The current behaviour is, that NO_INTERNAL_LOOKUP does not longer cache sessions internally since 0.9.6d. Nobody ever complained about this behavioural change. (Well it seems that the change I made at that time was incomplete and is to be fixed...) Yeah - I'm not surprised, in actuality I suspect that everyone using NO_INTERNAL_LOOKUP is using it because they have their own cache implementation (and callbacks) - ie. they don't notice or care whether builtin caching was being used as well, they just care that lookups aren't made in the builtin cache. Well, that is, nobody noticed or cared until Nadav started looking into it :-) With respect to your discussion: if the extension to the API you proposed here is required/useful for at least one major application (Apache2/mod_ssl) I have no problems to: * revert the change in 0.9.6, such that NO_INTERNAL_LOOKUP does again store sessions into the cache, * fix the reported bug for the 0.9.7 (and later) tree, * add the proposed API extension. I would propose to make the API change already to the 0.9.7 tree, as I think that the change is small enough and making the behaviour consistent seems like a bugfix to me. aolme too/aol I think there are a couple of facts arguing for this; - nobody currently notices (it seems) that NO_INTERNAL_LOOKUP used to store sessions, and now it *doesn't* store sessions except when it does (ie. it's gone from contradicting the man page to inconsistently contradicting itself). - NO_INTERNAL_LOOKUP and NO_INTERNAL_STORE would be logical and complimentary flags to implement, and would AFAICS provide applications with all the logically useful possibilities of builtin caching. - Apache2/mod_ssl requires the bitwise OR of NO_INTERNAL_LOOKUP and NO_INTERNAL_STORE (I propose NO_INTERNAL) and currently the appropriate flag has never existed (NO_INTERNAL_LOOKUP used to implement what I suggest it should, and now implements a buggy mix of what it used to implement and what Apache2/mod_ssl needs). I'll try and put a patch together later for review (and perhaps Nadav you would be able to grind it through your existing tests?). Cheers, Geoff -- Geoff Thorpe [EMAIL PROTECTED] http://www.geoffthorpe.net/ __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: Memory leak in session caching?
Hi there, I'm not cross-posting this to modssl-users as the problem and solution are mostly openssl-specific. Perhaps we can post a summary to modssl-users once this is over with. On Monday 21 Oct 2002 8:40 am, Nadav Har'El wrote: I ran the test again with the correct library, and OpenSSL 0.9.6g indeed does not cache new sessions in the internal cache (when NO_INTERNAL_LOOKUP), like I already noticed in the source code. But it does cache resumed sessions in the internal cache, again like I noticed in the source code (ssl_get_prev_session()) - so I think this is the bug. Thanks for the detailed report - Lutz has already created an RT ticket for this. I replied to the RT mail from him privately before noticing these followups (from you both) on modssl-users and openssl-dev, so I'll repeat here a suggestion I proposed in my mail to him, though I'll try to elaborate a bit. I think perhaps the correct solution would be to have things as follows; - SSL_SESS_CACHE_NO_INTERNAL_LOOKUP Lookups are not performed using the internal/local cache but solely through (any) external cache-lookup callback. Make sure that cache stores are nonetheless cached internally as well as through any external callback - as it was before and perhaps is still the case in some inconsistent buggy fashion, as you have suggested might be the case. :-) - SSL_SESS_CACHE_NO_INTERNAL_STORE (new flag) Cache stores are not performed against the internal/local cache but solely through (any) external cache-store callback. Lookups are nonetheless performed internally before using any external callback. - SSL_SESS_CACHE_NO_INTERNAL (new flag) This is (SSL_SESS_CACHE_NO_INTERNAL_LOOKUP|SSL_SESS_CACHE_NO_INTERNAL_STORE) Combination of the both the above, and the appropriate replacement for use in mod_ssl, Apache2, and other circumstances where fork() can introduce security problems with cache state. The reason I don't suggest changing the behaviour SSL_SESS_CACHE_NO_INTERNAL_LOOKUP to not store sessions is two-fold; (i) behavioural change of a flag that applications may be relying on and will introduce an unforeseen bug. (ii) it may actually be advantageous to have fine-grained control as I've suggested with the 3 flags (well, the third being an abbreviation for the OR of the first two). W.r.t. (ii) I can invisage NO_INTERNAL_LOOKUP being used on its own with the current behaviour - we may want a cache of sessions for manual inspection by the application without the application having to implement its own cache. With this flag, the application can be sure that the SSL/TLS code won't automatically resume them without going via its external callback - ie. the application itself will control those sessions it is prepared to accept or not. NO_INTERNAL_STORE may be similarly useful because the application can know the SSL/TLS implementation will automatically resume any resumable session in its cache, but the contents of that cache are only those sessions that the application has manually inserted into the cache of its own will - the SSL/TLS implementation won't automatically populate the cache. Obviously the combination will be useful too and in fact is what is desired in Apache2/mod_ssl. Any thoughts? I'm thinking out loud here - but will continue to think a little more carefully (and quietly) about it. Cheers, Geoff -- Geoff Thorpe [EMAIL PROTECTED] http://www.geoffthorpe.net/ __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
