nss: fix a memory leak when CURLOPT_CRLFILE is used

This commit is contained in:
Kamil Dudka 2014-07-04 00:39:23 +02:00
parent caa4db8a51
commit 52cd5ac21c
3 changed files with 35 additions and 5 deletions

View File

@ -37,6 +37,7 @@ This release includes the following bugfixes:
o nss: do not abort on connection failure (failing tests 305 and 404) o nss: do not abort on connection failure (failing tests 305 and 404)
o nss: make the fallback to SSLv3 work again o nss: make the fallback to SSLv3 work again
o tool: prevent valgrind from reporting possibly lost memory (nss only) o tool: prevent valgrind from reporting possibly lost memory (nss only)
o nss: fix a memory leak when CURLOPT_CRLFILE is used
o o
This release includes the following known bugs: This release includes the following known bugs:

View File

@ -324,6 +324,7 @@ struct ssl_connect_data {
PRFileDesc *handle; PRFileDesc *handle;
char *client_nickname; char *client_nickname;
struct SessionHandle *data; struct SessionHandle *data;
struct curl_llist *crl_list;
struct curl_llist *obj_list; struct curl_llist *obj_list;
PK11GenericObject *obj_clicert; PK11GenericObject *obj_clicert;
ssl_connect_state connecting_state; ssl_connect_state connecting_state;

View File

@ -393,6 +393,14 @@ static void nss_destroy_object(void *user, void *ptr)
PK11_DestroyGenericObject(obj); PK11_DestroyGenericObject(obj);
} }
/* same as nss_destroy_object() but for CRL items */
static void nss_destroy_crl_item(void *user, void *ptr)
{
SECItem *crl_der = (SECItem *)ptr;
(void) user;
SECITEM_FreeItem(crl_der, PR_TRUE);
}
static CURLcode nss_load_cert(struct ssl_connect_data *ssl, static CURLcode nss_load_cert(struct ssl_connect_data *ssl,
const char *filename, PRBool cacert) const char *filename, PRBool cacert)
{ {
@ -431,7 +439,7 @@ static CURLcode nss_load_cert(struct ssl_connect_data *ssl,
} }
/* add given CRL to cache if it is not already there */ /* add given CRL to cache if it is not already there */
static CURLcode nss_cache_crl(SECItem *crl_der) static CURLcode nss_cache_crl(struct ssl_connect_data *ssl, SECItem *crl_der)
{ {
CERTCertDBHandle *db = CERT_GetDefaultCertDB(); CERTCertDBHandle *db = CERT_GetDefaultCertDB();
CERTSignedCrl *crl = SEC_FindCrlByDERCert(db, crl_der, 0); CERTSignedCrl *crl = SEC_FindCrlByDERCert(db, crl_der, 0);
@ -442,12 +450,17 @@ static CURLcode nss_cache_crl(SECItem *crl_der)
return CURLE_SSL_CRL_BADFILE; return CURLE_SSL_CRL_BADFILE;
} }
/* store the CRL item so that we can free it in Curl_nss_close() */
if(!Curl_llist_insert_next(ssl->crl_list, ssl->crl_list->tail, crl_der)) {
SECITEM_FreeItem(crl_der, PR_FALSE);
return CURLE_OUT_OF_MEMORY;
}
/* acquire lock before call of CERT_CacheCRL() */ /* acquire lock before call of CERT_CacheCRL() */
PR_Lock(nss_crllock); PR_Lock(nss_crllock);
if(SECSuccess != CERT_CacheCRL(db, crl_der)) { if(SECSuccess != CERT_CacheCRL(db, crl_der)) {
/* unable to cache CRL */ /* unable to cache CRL */
PR_Unlock(nss_crllock); PR_Unlock(nss_crllock);
SECITEM_FreeItem(crl_der, PR_TRUE);
return CURLE_SSL_CRL_BADFILE; return CURLE_SSL_CRL_BADFILE;
} }
@ -457,7 +470,8 @@ static CURLcode nss_cache_crl(SECItem *crl_der)
return CURLE_OK; return CURLE_OK;
} }
static CURLcode nss_load_crl(const char* crlfilename) static CURLcode nss_load_crl(struct ssl_connect_data *connssl,
const char* crlfilename)
{ {
PRFileDesc *infile; PRFileDesc *infile;
PRFileInfo info; PRFileInfo info;
@ -512,7 +526,7 @@ static CURLcode nss_load_crl(const char* crlfilename)
*crl_der = filedata; *crl_der = filedata;
PR_Close(infile); PR_Close(infile);
return nss_cache_crl(crl_der); return nss_cache_crl(connssl, crl_der);
fail: fail:
PR_Close(infile); PR_Close(infile);
@ -1213,6 +1227,10 @@ void Curl_nss_close(struct connectdata *conn, int sockindex)
connssl->obj_list = NULL; connssl->obj_list = NULL;
connssl->obj_clicert = NULL; connssl->obj_clicert = NULL;
/* destroy all CRL items */
Curl_llist_destroy(connssl->crl_list, NULL);
connssl->crl_list = NULL;
PR_Close(connssl->handle); PR_Close(connssl->handle);
connssl->handle = NULL; connssl->handle = NULL;
} }
@ -1400,6 +1418,8 @@ static CURLcode nss_fail_connect(struct ssl_connect_data *connssl,
/* cleanup on connection failure */ /* cleanup on connection failure */
Curl_llist_destroy(connssl->obj_list, NULL); Curl_llist_destroy(connssl->obj_list, NULL);
connssl->obj_list = NULL; connssl->obj_list = NULL;
Curl_llist_destroy(connssl->crl_list, NULL);
connssl->crl_list = NULL;
if(connssl->handle if(connssl->handle
&& (SSL_VersionRangeGet(connssl->handle, &sslver) == SECSuccess) && (SSL_VersionRangeGet(connssl->handle, &sslver) == SECSuccess)
@ -1467,6 +1487,14 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)
if(!connssl->obj_list) if(!connssl->obj_list)
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
/* list of all CRL items we need to destroy in Curl_nss_close() */
connssl->crl_list = Curl_llist_alloc(nss_destroy_crl_item);
if(!connssl->crl_list) {
Curl_llist_destroy(connssl->obj_list, NULL);
connssl->obj_list = NULL;
return CURLE_OUT_OF_MEMORY;
}
/* FIXME. NSS doesn't support multiple databases open at the same time. */ /* FIXME. NSS doesn't support multiple databases open at the same time. */
PR_Lock(nss_initlock); PR_Lock(nss_initlock);
curlerr = nss_init(conn->data); curlerr = nss_init(conn->data);
@ -1569,7 +1597,7 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)
} }
if(data->set.ssl.CRLfile) { if(data->set.ssl.CRLfile) {
const CURLcode rv = nss_load_crl(data->set.ssl.CRLfile); const CURLcode rv = nss_load_crl(connssl, data->set.ssl.CRLfile);
if(CURLE_OK != rv) { if(CURLE_OK != rv) {
curlerr = rv; curlerr = rv;
goto error; goto error;