From dc0a7161f868ae2f8094289f45278cc702eafbd9 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Thu, 27 Jan 2011 10:55:02 +0100 Subject: nss: avoid memory leaks and failure of NSS shutdown ... in case more than one CA is loaded. Bug: https://bugzilla.redhat.com/670802 --- lib/nss.c | 190 +++++++++++++++++++++++++++++--------------------------------- 1 file changed, 89 insertions(+), 101 deletions(-) (limited to 'lib/nss.c') diff --git a/lib/nss.c b/lib/nss.c index 3d3e1c92c..5b648cdac 100644 --- a/lib/nss.c +++ b/lib/nss.c @@ -42,6 +42,7 @@ #include "strequal.h" #include "select.h" #include "sslgen.h" +#include "llist.h" #define _MPRINTF_REPLACE /* use the internal *printf() functions */ #include @@ -90,8 +91,12 @@ typedef struct { PRInt32 version; /* protocol version valid for this cipher */ } cipher_s; -#define PK11_SETATTRS(x,id,v,l) (x)->type = (id); \ - (x)->pValue=(v); (x)->ulValueLen = (l) +#define PK11_SETATTRS(_attr, _idx, _type, _val, _len) do { \ + CK_ATTRIBUTE *ptr = (_attr) + ((_idx)++); \ + ptr->type = (_type); \ + ptr->pValue = (_val); \ + ptr->ulValueLen = (_len); \ +} while(0) #define CERT_NewTempCertificate __CERT_NewTempCertificate @@ -308,18 +313,73 @@ static char *fmt_nickname(struct SessionHandle *data, enum dupstring cert_kind, return aprintf("PEM Token #%d:%s", 1, n); } +#ifdef HAVE_PK11_CREATEGENERICOBJECT +/* Call PK11_CreateGenericObject() with the given obj_class and filename. If + * the call succeeds, append the object handle to the list of objects so that + * the object can be destroyed in Curl_nss_close(). */ +static CURLcode nss_create_object(struct ssl_connect_data *ssl, + CK_OBJECT_CLASS obj_class, + const char *filename, bool cacert) +{ + PK11SlotInfo *slot; + PK11GenericObject *obj; + CK_BBOOL cktrue = CK_TRUE; + CK_BBOOL ckfalse = CK_FALSE; + CK_ATTRIBUTE attrs[/* max count of attributes */ 4]; + int attr_cnt = 0; + + const int slot_id = (cacert) ? 0 : 1; + char *slot_name = aprintf("PEM Token #%d", slot_id); + if(!slot_name) + return CURLE_OUT_OF_MEMORY; + + slot = PK11_FindSlotByName(slot_name); + free(slot_name); + if(!slot) + return CURLE_SSL_CERTPROBLEM; + + PK11_SETATTRS(attrs, attr_cnt, CKA_CLASS, &obj_class, sizeof(obj_class)); + PK11_SETATTRS(attrs, attr_cnt, CKA_TOKEN, &cktrue, sizeof(CK_BBOOL)); + PK11_SETATTRS(attrs, attr_cnt, CKA_LABEL, (unsigned char *)filename, + strlen(filename) + 1); + + if(CKO_CERTIFICATE == obj_class) { + CK_BBOOL *pval = (cacert) ? (&cktrue) : (&ckfalse); + PK11_SETATTRS(attrs, attr_cnt, CKA_TRUST, pval, sizeof(*pval)); + } + + obj = PK11_CreateGenericObject(slot, attrs, attr_cnt, PR_FALSE); + PK11_FreeSlot(slot); + if(!obj) + return CURLE_SSL_CERTPROBLEM; + + if(!Curl_llist_insert_next(ssl->obj_list, ssl->obj_list->tail, obj)) { + PK11_DestroyGenericObject(obj); + return CURLE_OUT_OF_MEMORY; + } + + return CURLE_OK; +} + +/* Destroy the NSS object whose handle is given by ptr. This function is + * a callback of Curl_llist_alloc() used by Curl_llist_destroy() to destroy + * NSS objects in Curl_nss_close() */ +static void nss_destroy_object(void *user, void *ptr) +{ + PK11GenericObject *obj = (PK11GenericObject *)ptr; + (void) user; + PK11_DestroyGenericObject(obj); +} +#endif + static int nss_load_cert(struct ssl_connect_data *ssl, const char *filename, PRBool cacert) { #ifdef HAVE_PK11_CREATEGENERICOBJECT - CK_SLOT_ID slotID; - PK11SlotInfo * slot = NULL; - CK_ATTRIBUTE *attrs; - CK_ATTRIBUTE theTemplate[20]; - CK_BBOOL cktrue = CK_TRUE; - CK_BBOOL ckfalse = CK_FALSE; - CK_OBJECT_CLASS objClass = CKO_CERTIFICATE; - char slotname[SLOTSIZE]; + /* All CA and trust objects go into slot 0. Other slots are used + * for storing certificates. + */ + const int slot_id = (cacert) ? 0 : 1; #endif CERTCertificate *cert; char *nickname = NULL; @@ -346,57 +406,15 @@ static int nss_load_cert(struct ssl_connect_data *ssl, } #ifdef HAVE_PK11_CREATEGENERICOBJECT - attrs = theTemplate; - - /* All CA and trust objects go into slot 0. Other slots are used - * for storing certificates. With each new user certificate we increment - * the slot count. We only support 1 user certificate right now. - */ - if(cacert) - slotID = 0; - else - slotID = 1; - - snprintf(slotname, SLOTSIZE, "PEM Token #%ld", slotID); - - nickname = aprintf("PEM Token #%ld:%s", slotID, n); + nickname = aprintf("PEM Token #%d:%s", slot_id, n); if(!nickname) return 0; - slot = PK11_FindSlotByName(slotname); - - if(!slot) { + if(CURLE_OK != nss_create_object(ssl, CKO_CERTIFICATE, filename, cacert)) { free(nickname); return 0; } - PK11_SETATTRS(attrs, CKA_CLASS, &objClass, sizeof(objClass) ); - attrs++; - PK11_SETATTRS(attrs, CKA_TOKEN, &cktrue, sizeof(CK_BBOOL) ); - attrs++; - PK11_SETATTRS(attrs, CKA_LABEL, (unsigned char *)filename, - strlen(filename)+1); - attrs++; - if(cacert) { - PK11_SETATTRS(attrs, CKA_TRUST, &cktrue, sizeof(CK_BBOOL) ); - } - else { - PK11_SETATTRS(attrs, CKA_TRUST, &ckfalse, sizeof(CK_BBOOL) ); - } - attrs++; - - /* This load the certificate in our PEM module into the appropriate - * slot. - */ - ssl->cacert[slotID] = PK11_CreateGenericObject(slot, theTemplate, 4, - PR_FALSE /* isPerm */); - - PK11_FreeSlot(slot); - - if(ssl->cacert[slotID] == NULL) { - free(nickname); - return 0; - } #else /* We don't have PK11_CreateGenericObject but a file-based cert was passed * in. We need to fail. @@ -516,54 +534,27 @@ static int nss_load_key(struct connectdata *conn, int sockindex, char *key_file) { #ifdef HAVE_PK11_CREATEGENERICOBJECT - PK11SlotInfo * slot = NULL; - CK_ATTRIBUTE *attrs; - CK_ATTRIBUTE theTemplate[20]; - CK_BBOOL cktrue = CK_TRUE; - CK_OBJECT_CLASS objClass = CKO_PRIVATE_KEY; - CK_SLOT_ID slotID; - char slotname[SLOTSIZE]; - struct ssl_connect_data *sslconn = &conn->ssl[sockindex]; - - attrs = theTemplate; - - /* FIXME: grok the various file types */ + PK11SlotInfo *slot; + SECStatus status; + struct ssl_connect_data *ssl = conn->ssl; - slotID = 1; /* hardcoded for now */ - - snprintf(slotname, sizeof(slotname), "PEM Token #%ld", slotID); - slot = PK11_FindSlotByName(slotname); - - if(!slot) - return 0; - - PK11_SETATTRS(attrs, CKA_CLASS, &objClass, sizeof(objClass) ); attrs++; - PK11_SETATTRS(attrs, CKA_TOKEN, &cktrue, sizeof(CK_BBOOL) ); attrs++; - PK11_SETATTRS(attrs, CKA_LABEL, (unsigned char *)key_file, - strlen(key_file)+1); attrs++; - - /* When adding an encrypted key the PKCS#11 will be set as removed */ - sslconn->key = PK11_CreateGenericObject(slot, theTemplate, 3, - PR_FALSE /* isPerm */); - if(sslconn->key == NULL) { + if(CURLE_OK != nss_create_object(ssl, CKO_PRIVATE_KEY, key_file, FALSE)) { PR_SetError(SEC_ERROR_BAD_KEY, 0); return 0; } + slot = PK11_FindSlotByName("PEM Token #1"); + if(!slot) + return 0; + /* This will force the token to be seen as re-inserted */ SECMOD_WaitForAnyTokenEvent(mod, 0, 0); PK11_IsPresent(slot); - /* parg is initialized in nss_Init_Tokens() */ - if(PK11_Authenticate(slot, PR_TRUE, - conn->data->set.str[STRING_KEY_PASSWD]) != SECSuccess) { - - PK11_FreeSlot(slot); - return 0; - } + status = PK11_Authenticate(slot, PR_TRUE, + conn->data->set.str[STRING_KEY_PASSWD]); PK11_FreeSlot(slot); - - return 1; + return (SECSuccess == status) ? 1 : 0; #else /* If we don't have PK11_CreateGenericObject then we can't load a file-based * key. @@ -1063,12 +1054,8 @@ void Curl_nss_close(struct connectdata *conn, int sockindex) connssl->client_nickname = NULL; } #ifdef HAVE_PK11_CREATEGENERICOBJECT - if(connssl->key) - (void)PK11_DestroyGenericObject(connssl->key); - if(connssl->cacert[1]) - (void)PK11_DestroyGenericObject(connssl->cacert[1]); - if(connssl->cacert[0]) - (void)PK11_DestroyGenericObject(connssl->cacert[0]); + /* destroy all NSS objects in order to avoid failure of NSS shutdown */ + Curl_llist_destroy(connssl->obj_list, NULL); #endif connssl->handle = NULL; } @@ -1179,9 +1166,10 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex) connssl->data = data; #ifdef HAVE_PK11_CREATEGENERICOBJECT - connssl->cacert[0] = NULL; - connssl->cacert[1] = NULL; - connssl->key = NULL; + /* list of all NSS objects we need to destroy in Curl_nss_close() */ + connssl->obj_list = Curl_llist_alloc(nss_destroy_object); + if(!connssl->obj_list) + return CURLE_OUT_OF_MEMORY; #endif /* FIXME. NSS doesn't support multiple databases open at the same time. */ -- cgit v1.2.3