Memory Leak in CertificateChain.GetCertificates |
|
|
by B. Ogatiy posted on 2005/05/23 |
|
This funtion leaks memory on Windows 2000 and greater systems.
Every call consumes 4 system handles and about 160,000 bytes of memory that cannot be released.
|
by Alfonso Ferrandez [alfonso dot ferrandez at ntlworld dot com] posted on 2005/05/24 |
|
Thanks for confirming this for me. I thought I was going mad. Have you found a resolution for this? In my case this is a big showstopper, but the full solution has now being developed and it's ready to deploy, and the huge memory leak is causing us to stop everything else!
AL |
by Neil posted on 2005/05/25 |
|
According to the MSDN help on the function CertGetIssuerCertificateFromStore the return value is only freed when passed as the pPrevIssuerContext (parameter 3). Otherwise the parameter must be explicitly freed by calling CertFreeCertificateContext. The GetCertificates call to this function does not pass the returned value (cert) as parameter 3 so must a call to SspiProvider.CertFreeCertificateContext must be added to free the memory. |
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/05/25 |
|
Thanks for reporting this bug. At first sight, it looks looks like the line
ret.Add(new Certificate(cert, true));
should be changed to
ret.Add(new Certificate(cert, false));
This should solve your problems. I'll take a closer look at it later this day and update the SecLib package accordingly. |
by Alfonso Ferrandez [alfonso dot ferrandez at ntlworld dot com] posted on 2005/05/26 |
|
My prject is basically a Secure FTP Proxy, and the one symptom we've observed is that memory grows with the amount of data transferred. That is, the larger a file transfer, the more memory is leaked. So I have a feeling the certificate leak is not the oly thing going on here. Perhaps a leak with the actual encryption/decryption of the stream of data as it flows through the Record Layer?
AL |
by Alfonso Ferrandez posted on 2005/05/27 |
|
Hi Pieter,
any luck with fixing the memory leaks?
AL |
by Dmytro posted on 2005/05/27 |
|
Yes i noticed that too the RAM grows huge and never gets flushed! anyone found solutions? |
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/05/28 |
|
A new version of the library has been uploaded that should fix the CertificateChain memory leak.
As for other possible leaks, we can only fix them if someone can tell us how to reliably reproduce them (sample code would be greatly appreciated).
|
by Dmytro [ecode at programmer dot net] posted on 2005/05/30 |
|
well i noticed it runs faster but when i leave my server for a few days, my RAM goes up to 500MB and then i have to reset it again, (once I terminate that process it goes back to like 200MB) so the socket class still not perfect, it does not drop whats already been sent...
(I THINK THIS IS MAJOUR BUG, AND IF THIS GETS FIXED THEN, man this socket rocks) |
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/05/30 |
|
I'm curious whether a call to GC.Collect has an impact on the used RAM. If not, it's definitely an unmanaged resource that isn't released properly, but I don't think we use that many unmanaged resources in an SSL session (only the certificates, I can't really think of anything else).
Can you also describe the type of your server a little bit? I.e. does your server typically have many short connections or less but longer connections? What cipher suites are enabled and are typically used? Etc. |
by Alfonso Ferrandez [alfonso dot ferrandez at ntlworld dot com] posted on 2005/05/31 |
|
Hi Pieter,
whilst I was trying to find the memory leak a few days ago I made use of a few memory profilers that report only on Managed Memory and that can trigger GC.Collect on demand. The managed memory was always correctly released to its base level, which made me think the leak was indeed of unmanaged resources. I guess the main problem is that most classes in the Security Library rely on the FInalizer rather than on the IDisposable pattern, which means that both performance and resource release are delayed.
Still, even changing the library to use Dispose() made not much difference with the bulk of the memory being released only after shutting down the process.
I agree that this is a MAJOR ISSUE that in our case makes the service consume 1.5GB of RAM (from a starting base of 32Mb!) in just a few humndred connections. This is related to both, the number of connections establishe, and the amount of data transferred. This is what makes me think there's a leak in the encryption/decryption layer which scales up with the amount of data encrypted/decrypted.
I should say that in my case this is running on Windows 2003 Server.
By the way, is there anywhere we can see the changes made to this new version?
Thanks for your help, and I really hope you help us all find the problem, because this is too good a library to be rendered useless but a nasty bug like this...
Regards,
AL
|
by Alfonso Ferrandez [alfonso dot ferrandez at ntlworld dot com] posted on 2005/05/31 |
|
Hi again,
sorry to be a pain, but I've gone through the new release's code and the only change I can identify is the version number in the AssemblyInfo file, but no actual code changes! Is this an oversight? Or have I missed something?
AL |
by Dmytro [ecode at programmer dot net] posted on 2005/05/31 |
|
<h2>Hi, I created a simple HTTPS server, as an example of what we are talking about, in VB.NET</h2> http://www.iroot.ca/mentalis/
Please dont pay much attention to the server itself, but to the Socket Library memory leak, <b>You must install a sample certificate</b> I included it in .pfx file the password is: test
YOU NOTICE WHEN REFRESHING THE PAGE THE RAM JUST KEEPS BUILDING UP AND NEVER GETS FLUSHED!
If some one can take time to fix this, I can contribute, but i have limited skills in C#
|
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/05/31 |
|
Alfonso, the only change that was made in this new version was in the CertificateChain::GetCertificates method. The line
ret.Add(new Certificate(cert, true));
was replaced with the line
ret.Add(new Certificate(cert, false));
As for the memory leak in the SSL code, I'll see what I can do about it. |
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/06/01 |
|
I've modified the WebClient example in our sample section to open hundreds of connections to the specified server instead of one, and then I've used this modified example project to connect to the WebServer example project and Dmytro's web server application.
In both cases, I wasn't able to reproduce the memory leak. As expected, the memory of the server application fluctuated between 40 and 60mb, but it didn't substantially increase even after several thousand connections.
If one of you can modify the WebClient/WebServer example projects in such a ways that I can reproduce the bug, I'll do my best to solve it. However, I can't fix it if I can't reproduce it... |
by Alfonso Ferrandez [alfonso dot ferrandez at ntlworld dot com] posted on 2005/06/01 |
|
Hi Pieter,
according to your first posting, the change should be changed from
ret.Add(new Certificate(cert, true));
to
ret.Add(new Certificate(cert, false));
but the new package still contains the old code (in fact the file is dated Jan 6th 2005). So I just thought I'd let you know that the code hasn't changed in the latest distributed version.
As for the Web Server example, I'll try to run it on our machine and see. We are running on a 4-CPU Windows 2003 Server which might have something to do with it. Have you tried the code on a Win2003 machine?
Thanks!
AL
|
by Dmytro [ecode at programmer dot net] posted on 2005/06/01 |
|
OK here is more detail, I do not know how you managed not to reproduce this type of memory leak, may be you have updated library, without realizing(upload it to my FTP: ftp.iroot.ca), i dono. TAKE A LOOK AT THIS-> http://www.iroot.ca/mentalis/memory.jpg
_________________________
the HTTPS server can be found here: http://www.iroot.ca/mentalis/
|
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/06/01 |
|
Thanks for notifying us about that, Alfonso! I've updated the ZIP file to include the bugfix. You can download it from:
http://www.mentalis.org/soft/projects/seclib/download.qpx
When I said I couldn't reproduce the memory leak, I was already using the updated code. The method that was fixed is used during the SSL/TLS handshake, so it certainly fixes at least a part of the memory problems you're seeing. The question now is whether it fixes all your memory problems or not. It would be great if you could test that for me. |
by Alfonso Ferrandez [alfonso dot ferrandez at ntlworld dot com] posted on 2005/06/02 |
|
Hi Pieter,
unfortunately the memory leak is still there. I can see the memory growing consistently when a file is transferred and it never gets released after this.
So the only thing I can think of is the fact that we are working on Windows 2003 (with and without SP1) and the memory leak is still there!
Anybody else seeing the memory leak also running on Windows 2003?
Alfonso
|
by Dmytro [ecode at programmer dot net] posted on 2005/06/02 |
|
MAN, IT MADE IT EVEN WORTH, I GET THIS ERROR WHEN MORE THEN ONE CONNECTION CONNECTS -> www.iroot.ca/mentalis/update713.jpg
Just debug it useing my HTTPS server example (The server is not a prolem everything is bulletproof, because when I replace "SecureSocket" with "Socket" and use it with normal .NET sockets, it does not give me any memory leaks, so the problem is SSL Library!)
once again take a look at the screen shot and get my server at http://www.iroot.ca/mentalis/HTTS_test.zip to try to find out what the problem is
Thank you, Dmytro |
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/06/02 |
|
Alfonso, the reason why I can't find the leak is because after the handshake we don't use the unmanaged API's anymore (unless the connection negotiates one of the AES cipher suites, but IE doesn't support this suite and we don't allocate unmanaged memory in the RijndaelCryptoServiceProider class, soI doubt this could be the reason).
If you could give me more information about your server, that would be helpful. For instance, are you using SSL3, TLS1 or both (and which protocol actually gets used, because by default IE doesn't allow TLS1). Also, which cipher suite is used between the clients and the server (you can get this info by calling SecureSocket.ActiveEncryption)?
Dmytro, I downloaded your ZIP file and ran the precompiled executable. It worked perfectly... The only thing I had to do was to import the PFX file into the 'MY' certificate store. |
by Alfonso Ferrandez [alfonso dot ferrandez at ntlworld dot com] posted on 2005/06/03 |
|
Hi Pieter,
Firstly, it looks like the encryption used is RSA_AES_128_SHA. How can I force it to avoid AES and go for a managed one?
Also, I noticed than with the fix introduced in this new version (certificate duplication set to false) the code will fail after a while (exactly as Dmytro indicated) because for some strange reason, the Private key of the certificate disappears and therefore the whole creating a securesocket process fails bacause there is no private key anymore!).
I'm getting really desperate now, and I need help (stress levels through the roof, since this project is way over its deadline!)
Thanks!
Alfonso
|
by Dmytro [ecode at programmer dot net] posted on 2005/06/03 |
|
OK PARTIAL SOLUTION!
After I did this, it fixed a lot of memory leaks, strange but i been running my ssl for a few days the RAM keeps going up and down, probably some leak in SEND. But much better then before!!!
ok the code fix:
_______________________________________
public CertificateStore(StoreLocation location, string store)
{
if (store == null)
throw new ArgumentNullException("The name of the store cannot be a null reference.");
// GH - memory leak here!!!
// IntPtr intPtr = new IntPtr(SecurityConstants.CERT_STORE_PROV_SYSTEM_A);
// m_Handle = SspiProvider.CertOpenStore(intPtr, 0, 0, (int)location, store);
this.m_Handle = GetStoreByLocation_StoreName(location, store);
if (m_Handle == IntPtr.Zero)
throw new CertificateException("An error occurs while opening the specified store.");
}
/// <summary>
/// GH - this fixed a major leak in CertificateStore's constructor.
/// </summary>
/// <param name="location"></param>
/// <param name="store"></param>
/// <returns></returns>
private static IntPtr GetStoreByLocation_StoreName(StoreLocation location, string store)
{
if ( !storesByLocation_StoreName.ContainsKey(location) )
storesByLocation_StoreName[location] = new Hashtable();
Hashtable h = storesByLocation_StoreName[location] as Hashtable;
if (!h.ContainsKey(store))
{
IntPtr intPtr = new IntPtr(SecurityConstants.CERT_STORE_PROV_SYSTEM_A);
IntPtr handle = SspiProvider.CertOpenStore(intPtr, 0, 0, (int)location, store);
h[store] = handle;
}
return (IntPtr)h[store];
}
private static Hashtable storesByLocation_StoreName = new Hashtable();
_______________________________________
and you MUST set this back to-> "true"
ret.Add(new Certificate(cert, true)); |
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/06/03 |
|
Ok guys, I just updated the code and I think (hope) this is going to solve most of the problems you're seeing.
Alfonso, I took another look at the GetCertificates method after you reported that the private key sometimes disappeared. There was indeed another bug in the method that was originally already there, but was made more visible by the fix. This bug caused two certificate objects to point to the same handle; if one of the certificates got out of scope and was collected by the GC, it would kill the certificate handle and thus also killing the other Certificate object (hence the 'disappearing' key). I never noticed this because I didn't test these methods under a heavy load, so the GC never actually collected one of the certificates. I assume that this bug also caused the NullReferenceException that Dmytro is seeing with his server.
Dmytro, the code you posted doesn't fix a memory leak, but it's rather a memory optimization (and a very good one, I might add!). The old code created new certificate stores for every call to GetCertificates and would eventually let the GC reclaim the memory. The code you posted caches the most important certificate stores and hence reduces the memory footprint of the application significantly. I've added a modified version of your code to the library.
The GetCertificates method seems bug-free now. Calling it thousands of times in a loop doesn't impact the memory at all.
Alfonso, as for the AES encryption, I've switched from the unmanaged RijndaelCryptoServiceProvider to the RijndaelManaged class (just to be sure). If you still want to exclude the AES cipher suites, you can do this by appropriately filling in the AllowedAlgorithms field of the SecurityOptions structure. For instance, you can use the following code:
SecurityOptions so = new SecurityOptions(..);
so.AllowedAlgorithms = RSA_RC4_128_SHA | RSA_RC4_128_MD5
| RSA_3DES_168_SHA | NULL_COMPRESSION;
|
by Dmytro posted on 2005/06/04 |
|
I didnt look at the code changes, just for the sake of time, I compiled it, and run my SSL server, seems to be GREAT! much more improved. I will test it in production for a few days, then will see... Just wanted to say Great job Pete, and everyone! The Socket Library is amayzing... Thankx |
by Alfonso Ferrandez posted on 2005/06/08 |
|
Hi Pieter,
I've recompiled the solution with the new libraries and so far I haven't had any issues other than the multiprocessor ones I already had. Since these are proving to be a show stopper, the memory leak investigation has taken a bit of a background position. It looks like the memory consumption is less than before, but I'm not sure whether it's totally controlled yet.
One question: Any reason why the BeginSend/Send part of the libraries does not use lock() to protect against racing consitions?
Thanks!
Alfonso |
by Alfonso Ferrandez posted on 2005/06/08 |
|
Hi again!
I can confirm that the memory leak has been plugged! So now I only have the multiprocessor issue to contend with...
If you have any ideas about this please do let me know, because it is proving a pain to work out (with so little time left in the project!)
Thanks!
AL |
by Pieter Philippaerts [Pieter at mentalis dot org] posted on 2005/06/09 |
|
> Any reason why the BeginSend/Send part of the
> libraries does not use lock() to protect against
> racing conditions
The lock methods are called in the functions of the SocketController (a layer beneath the SecureSocket). We'll take the discussion of this multi-processor issue to the other forum thread... |