cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] Correct refcount issues when using client certs in NSS

From: Claes Jakobsson <claes_at_versed.se>
Date: Sun, 31 May 2009 15:43:30 +0200

Hi

On 30 maj 2009, at 18.01, Kamil Dudka wrote:
> 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.

My pleasure =)

I've attached a new patch against CVS that removes the client_cert
member from ssl_connect_data and all its uses. All tests pass as does
my script that uses smartcard certs. Could you test to see that it
doesn't break anything for you?

>> 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.

Ok, I'll do some memory leak checking as well.

>> 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])

Yes, it would help. See attached patch.

> 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.

The debug log was for the working variant. The reason it didn't close
the sessions was a bug in the perl bindings that I've fixed so that
curl is properly shutdown when exit - a patch for this has been sent
to Balint.

/Claes

Received on 2009-05-31