On Saturday 30 of May 2009 17:06:40 Claes Jakobsson wrote:
> Well, from what I can dig out of the sources NSS calls
> CERT_DestroyCertificate internally on the cert returned from the
> callback (see for example the end of ssl2_HandleRequestCertificate in
> lib/ssl/sslcon.c) which is why we must retain the certificate using
> CERT_DupCertificate if we want to keep it around as it's done in
> curl's nss implementation.

Thanks for digging this. In that case we should let NSS libraries to call
CERT_DestroyCertificate() function on the certificate. Then we actually no
more need to hold the reference within the ssl_connect_data. Calling
of the CERT_DestroyCertificate() function was the only reason to hold the
reference.

> What kind of certificates are leaking memory for you - those loaded
> from a PEM file or ones that are already in a NSS store?

PEM file. I'll play with it a bit more next week to find out why.

> It's the second connection when it finds the certificate in the
> session cache that it seg. faults because the cert has been destroyed
> when we closed the connection last time which is why it's actually
> CERT_DupCertificate that fails and not CERT_DestroyCertificate.

Please try the following patch instead of yours if it helps:

--- nss.c.orig 2009-05-30 17:51:57.857984631 +0200
+++ nss.c 2009-05-30 17:52:26.569499370 +0200
@@ -912,8 +912,6 @@ void Curl_nss_close(struct connectdata *
       free(connssl->client_nickname);
       connssl->client_nickname = NULL;
     }
- if(connssl->client_cert)
- CERT_DestroyCertificate(connssl->client_cert);
     if(connssl->key)
       (void)PK11_DestroyGenericObject(connssl->key);
     if(connssl->cacert[1])

> I've attached the PKCS11 debug log (but with some some cert and token
> names removed)

The debug log is for the not-working variant? Please make sure the C_Finalize
function is called in the working variant. If not, it's possible it leaks
memory in your case as well.

Kamil
Received on 2009-05-30