From 67bf63e515183c5ff3761f1d97af74b2f86603e2 Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: Mar 12 2019 16:34:50 +0000 Subject: import nss-pem-1.0.3-5.el7_6.1 --- diff --git a/SOURCES/0003-nss-pem-1.0.3-make-pem_objs-a-list-instead-of-array.patch b/SOURCES/0003-nss-pem-1.0.3-make-pem_objs-a-list-instead-of-array.patch new file mode 100644 index 0000000..94cd574 --- /dev/null +++ b/SOURCES/0003-nss-pem-1.0.3-make-pem_objs-a-list-instead-of-array.patch @@ -0,0 +1,1010 @@ +From c4bd40a483c7962beb4a8b1c7f84d647de66bf0c Mon Sep 17 00:00:00 2001 +From: Kamil Dudka +Date: Tue, 29 Aug 2017 13:08:11 +0200 +Subject: [PATCH 1/5] util: fix the return value type of ReadDERFromFile() + +SECStatus is an inappropriate type for returning the count of objects. + +Upstream-commit: 01c13264e59050ea34aefc3d2a38e9ead247276c +Signed-off-by: Kamil Dudka +--- + src/ckpem.h | 6 ++---- + src/pinst.c | 15 +++++---------- + src/pobject.c | 10 ++++------ + src/util.c | 7 +++---- + 4 files changed, 14 insertions(+), 24 deletions(-) + +diff --git a/src/ckpem.h b/src/ckpem.h +index e6ecc5f..be9977a 100644 +--- a/src/ckpem.h ++++ b/src/ckpem.h +@@ -258,10 +258,8 @@ pem_CreateMDObject + #define NSS_PEM_ARRAY_SIZE(x) ((sizeof (x))/(sizeof ((x)[0]))) + + /* Read DER encoded data from a PEM file or a binary (der-encoded) file. */ +-/* NOTE: Discrepancy with the the way callers use of the return value as a count +- * Fix this when we sync. up with the cleanup work being done at nss-pem project. +- */ +-SECStatus ReadDERFromFile(SECItem ***derlist, char *filename, PRBool ascii, int *cipher, char **ivstring, PRBool certsonly); ++int ReadDERFromFile(SECItem ***derlist, char *filename, PRBool ascii, ++ int *cipher, char **ivstring, PRBool certsonly); + + /* Fetch an attribute of the specified type. */ + const NSSItem * pem_FetchAttribute ( pemInternalObject *io, CK_ATTRIBUTE_TYPE type, CK_RV *pError); +diff --git a/src/pinst.c b/src/pinst.c +index 25485b1..a5901f0 100644 +--- a/src/pinst.c ++++ b/src/pinst.c +@@ -474,14 +474,12 @@ AddCertificate(char *certfile, char *keyfile, PRBool cacert, + pemInternalObject *o = NULL; + CK_RV error = 0; + int objid, i = 0; +- int nobjs = 0; + SECItem **objs = NULL; + char *ivstring = NULL; + int cipher; + +- /* TODO: Fix discrepancy between our usage of the return value as +- * as an int (a count) and the declaration as a SECStatus. */ +- nobjs = (int) ReadDERFromFile(&objs, certfile, PR_TRUE, &cipher, &ivstring, PR_TRUE /* certs only */); ++ int nobjs = ReadDERFromFile(&objs, certfile, /* ascii */ PR_TRUE, &cipher, ++ &ivstring, /* certs only */ PR_TRUE); + if (nobjs <= 0) { + NSS_ZFreeIf(objs); + return CKR_GENERAL_ERROR; +@@ -516,12 +514,9 @@ AddCertificate(char *certfile, char *keyfile, PRBool cacert, + + if (o != NULL && keyfile != NULL) { /* add the private key */ + SECItem **keyobjs = NULL; +- int kobjs = 0; +- /* TODO: Fix discrepancy between our usage of the return value as +- * as an int and the declaration as a SECStatus. */ +- kobjs = +- (int) ReadDERFromFile(&keyobjs, keyfile, PR_TRUE, &cipher, +- &ivstring, PR_FALSE); ++ int kobjs = ReadDERFromFile(&keyobjs, keyfile, /* ascii */ PR_TRUE, ++ &cipher, &ivstring, ++ /*certs only */ PR_FALSE); + if (kobjs < 1) { + found_error = PR_TRUE; + } else { +diff --git a/src/pobject.c b/src/pobject.c +index 918b327..d80092a 100644 +--- a/src/pobject.c ++++ b/src/pobject.c +@@ -1165,11 +1165,8 @@ pem_CreateObject + } + + if (objClass == CKO_CERTIFICATE) { +- /* TODO: Fix discrepancy between our usage of the return value as +- * as an int and the declaration as a SECStatus. Typecasting as a +- * temporary workaround. +- */ +- nobjs = (int) ReadDERFromFile(&derlist, filename, PR_TRUE, &cipher, &ivstring, PR_TRUE /* certs only */); ++ nobjs = ReadDERFromFile(&derlist, filename, /* ascii */ PR_TRUE, ++ &cipher, &ivstring, /* certs only */ PR_TRUE); + if (nobjs < 1) + goto loser; + +@@ -1214,7 +1211,8 @@ pem_CreateObject + CK_SESSION_HANDLE hSession; + PRBool added; + +- nobjs = ReadDERFromFile(&derlist, filename, PR_TRUE, &cipher, &ivstring, PR_FALSE /* keys only */); ++ nobjs = ReadDERFromFile(&derlist, filename, /* ascii */ PR_TRUE, ++ &cipher, &ivstring, /* keys only */ PR_FALSE); + if (nobjs < 1) + goto loser; + +diff --git a/src/util.c b/src/util.c +index ee9e0a4..9bad613 100644 +--- a/src/util.c ++++ b/src/util.c +@@ -144,10 +144,9 @@ static SECStatus ConvertAsciiToZAllocItem(SECItem *der, const char *ascii) + return rv; + } + +-/* FIX: Returns a SECStatus yet callers take result as a count */ +-SECStatus +-ReadDERFromFile(SECItem *** derlist, char *filename, PRBool ascii, +- int *cipher, char **ivstring, PRBool certsonly) ++/* returns count of objects read, or -1 on error */ ++int ReadDERFromFile(SECItem *** derlist, char *filename, PRBool ascii, ++ int *cipher, char **ivstring, PRBool certsonly) + { + SECStatus rv; + PRFileDesc *inFile; +-- +2.17.2 + + +From 729f16122f0bae216583323389aa6ef1418e4c37 Mon Sep 17 00:00:00 2001 +From: Kamil Dudka +Date: Thu, 20 Dec 2018 18:42:33 +0100 +Subject: [PATCH 2/5] ckpem: make pem_objs a list instead of array + +Since libcurl started to use PK11_CreateManagedGenericObject(), +the calls to PK11_DestroyGenericObject() started to take affect +at the level of nss-pem. Internal objects were removed on each +call of curl_easy_cleanup() and allocated again on each call of +curl_easy_perform(). This would be fine if the corresponding items +of the global pem_objects array were reused, which was, however, +not the case. Consequently, the array grew unboundedly while most +of its items were NULL pointers. This caused severe performance +regression in libcurl-based applications. + +This commit replaces the global array by a global linked list, +which allows to delete arbitrary items instead of keeping NULL +pointers. For compatibility reasons, the original array indexes +are stored in the list nodes. + +Bug: https://bugzilla.redhat.com/1659108 + +Fixes #2 + +Upstream-commit: 8fc25a6b062a34d09473d4f80058fffda37f41a2 +Signed-off-by: Kamil Dudka +--- + src/ckpem.h | 11 ++- + src/list.h | 200 +++++++++++++++++++++++++++++++++++++++++++++++++ + src/pfind.c | 19 ++--- + src/pinst.c | 56 +++++++------- + src/pobject.c | 27 +++---- + src/psession.c | 11 +-- + 6 files changed, 258 insertions(+), 66 deletions(-) + create mode 100644 src/list.h + +diff --git a/src/ckpem.h b/src/ckpem.h +index be9977a..caad74b 100644 +--- a/src/ckpem.h ++++ b/src/ckpem.h +@@ -39,6 +39,8 @@ + #ifndef CKPEM_H + #define CKPEM_H + ++#include "list.h" ++ + #define USE_UTIL_DIRECTLY + #include + +@@ -197,9 +199,14 @@ struct pemInternalObjectStr { + char *nickname; + NSSCKMDObject mdObject; + CK_SLOT_ID slotID; +- CK_ULONG gobjIndex; + int refCount; + ++ /* all internal objects are linked in a global list */ ++ struct list_head gl_list; ++ ++ /* we represent sparse array as list but keep its elements indexed */ ++ long arrayIdx; ++ + /* used by pem_mdFindObjects_Next */ + CK_BBOOL extRef; + +@@ -208,7 +215,7 @@ struct pemInternalObjectStr { + pemObjectListItem *list; + }; + +-NSS_EXTERN_DATA pemInternalObject **pem_objs; ++NSS_EXTERN_DATA struct list_head pem_objs; + NSS_EXTERN_DATA int pem_nobjs; + NSS_EXTERN_DATA int token_needsLogin[]; + +diff --git a/src/list.h b/src/list.h +new file mode 100644 +index 0000000..7b4aec4 +--- /dev/null ++++ b/src/list.h +@@ -0,0 +1,200 @@ ++/* SPDX-License-Identifier: GPL-2.0 */ ++#ifndef _LINUX_LIST_H ++#define _LINUX_LIST_H ++ ++/* simplified for our use case */ ++#define READ_ONCE(x) (x) ++#define WRITE_ONCE(dst, src) (dst = (src)) ++#define typeof(x) __typeof__(x) ++#define POISON_POINTER_DELTA 0 ++ ++/* following code is taken from of v4.20-rc7-202-g1d51b4b1d3f2 */ ++ ++/** ++ * container_of - cast a member of a structure out to the containing structure ++ * @ptr: the pointer to the member. ++ * @type: the type of the container struct this is embedded in. ++ * @member: the name of the member within the struct. ++ * ++ */ ++#define container_of(ptr, type, member) ({ \ ++ void *__mptr = (void *)(ptr); \ ++ ((type *)(__mptr - offsetof(type, member))); }) ++ ++/* ++ * These are non-NULL pointers that will result in page faults ++ * under normal circumstances, used to verify that nobody uses ++ * non-initialized list entries. ++ */ ++#define LIST_POISON1 ((void *) 0x100 + POISON_POINTER_DELTA) ++#define LIST_POISON2 ((void *) 0x200 + POISON_POINTER_DELTA) ++ ++struct list_head { ++ struct list_head *next, *prev; ++}; ++ ++/* ++ * Simple doubly linked list implementation. ++ * ++ * Some of the internal functions ("__xxx") are useful when ++ * manipulating whole lists rather than single entries, as ++ * sometimes we already know the next/prev entries and we can ++ * generate better code by using them directly rather than ++ * using the generic single-entry routines. ++ */ ++ ++#define LIST_HEAD_INIT(name) { &(name), &(name) } ++ ++#define LIST_HEAD(name) \ ++ struct list_head name = LIST_HEAD_INIT(name) ++ ++static inline void INIT_LIST_HEAD(struct list_head *list) ++{ ++ WRITE_ONCE(list->next, list); ++ list->prev = list; ++} ++ ++/* ++ * Insert a new entry between two known consecutive entries. ++ * ++ * This is only for internal list manipulation where we know ++ * the prev/next entries already! ++ */ ++static inline void __list_add(struct list_head *new, ++ struct list_head *prev, ++ struct list_head *next) ++{ ++ next->prev = new; ++ new->next = next; ++ new->prev = prev; ++ WRITE_ONCE(prev->next, new); ++} ++ ++/** ++ * list_add - add a new entry ++ * @new: new entry to be added ++ * @head: list head to add it after ++ * ++ * Insert a new entry after the specified head. ++ * This is good for implementing stacks. ++ */ ++static inline void list_add(struct list_head *new, struct list_head *head) ++{ ++ __list_add(new, head, head->next); ++} ++ ++ ++/** ++ * list_add_tail - add a new entry ++ * @new: new entry to be added ++ * @head: list head to add it before ++ * ++ * Insert a new entry before the specified head. ++ * This is useful for implementing queues. ++ */ ++static inline void list_add_tail(struct list_head *new, struct list_head *head) ++{ ++ __list_add(new, head->prev, head); ++} ++ ++/* ++ * Delete a list entry by making the prev/next entries ++ * point to each other. ++ * ++ * This is only for internal list manipulation where we know ++ * the prev/next entries already! ++ */ ++static inline void __list_del(struct list_head * prev, struct list_head * next) ++{ ++ next->prev = prev; ++ WRITE_ONCE(prev->next, next); ++} ++ ++/** ++ * list_del - deletes entry from list. ++ * @entry: the element to delete from the list. ++ * Note: list_empty() on entry does not return true after this, the entry is ++ * in an undefined state. ++ */ ++static inline void __list_del_entry(struct list_head *entry) ++{ ++ __list_del(entry->prev, entry->next); ++} ++ ++static inline void list_del(struct list_head *entry) ++{ ++ __list_del_entry(entry); ++ entry->next = LIST_POISON1; ++ entry->prev = LIST_POISON2; ++} ++ ++/** ++ * list_entry - get the struct for this entry ++ * @ptr: the &struct list_head pointer. ++ * @type: the type of the struct this is embedded in. ++ * @member: the name of the list_head within the struct. ++ */ ++#define list_entry(ptr, type, member) \ ++ container_of(ptr, type, member) ++ ++/** ++ * list_first_entry - get the first element from a list ++ * @ptr: the list head to take the element from. ++ * @type: the type of the struct this is embedded in. ++ * @member: the name of the list_head within the struct. ++ * ++ * Note, that list is expected to be not empty. ++ */ ++#define list_first_entry(ptr, type, member) \ ++ list_entry((ptr)->next, type, member) ++ ++/** ++ * list_last_entry - get the last element from a list ++ * @ptr: the list head to take the element from. ++ * @type: the type of the struct this is embedded in. ++ * @member: the name of the list_head within the struct. ++ * ++ * Note, that list is expected to be not empty. ++ */ ++#define list_last_entry(ptr, type, member) \ ++ list_entry((ptr)->prev, type, member) ++ ++/** ++ * list_next_entry - get the next element in list ++ * @pos: the type * to cursor ++ * @member: the name of the list_head within the struct. ++ */ ++#define list_next_entry(pos, member) \ ++ list_entry((pos)->member.next, typeof(*(pos)), member) ++ ++/** ++ * list_prev_entry - get the prev element in list ++ * @pos: the type * to cursor ++ * @member: the name of the list_head within the struct. ++ */ ++#define list_prev_entry(pos, member) \ ++ list_entry((pos)->member.prev, typeof(*(pos)), member) ++ ++/** ++ * list_for_each_entry - iterate over list of given type ++ * @pos: the type * to use as a loop cursor. ++ * @head: the head for your list. ++ * @member: the name of the list_head within the struct. ++ */ ++#define list_for_each_entry(pos, head, member) \ ++ for (pos = list_first_entry(head, typeof(*pos), member); \ ++ &pos->member != (head); \ ++ pos = list_next_entry(pos, member)) ++ ++/** ++ * list_for_each_entry_reverse - iterate backwards over list of given type. ++ * @pos: the type * to use as a loop cursor. ++ * @head: the head for your list. ++ * @member: the name of the list_head within the struct. ++ */ ++#define list_for_each_entry_reverse(pos, head, member) \ ++ for (pos = list_last_entry(head, typeof(*pos), member); \ ++ &pos->member != (head); \ ++ pos = list_prev_entry(pos, member)) ++ ++#endif +diff --git a/src/pfind.c b/src/pfind.c +index 2aff8ff..3f0fdcb 100644 +--- a/src/pfind.c ++++ b/src/pfind.c +@@ -253,12 +253,13 @@ collect_objects(CK_ATTRIBUTE_PTR pTemplate, + size_t result_array_capacity = 0; + pemObjectType type = pemRaw; + CK_OBJECT_CLASS objClass = pem_GetObjectClass(pTemplate, ulAttributeCount); ++ pemInternalObject *obj = NULL; + + *pError = CKR_OK; + + plog("collect_objects slot #%ld, ", slotID); + plog("%d attributes, ", ulAttributeCount); +- plog("%d objects to look through.\n", pem_nobjs); ++ plog("%d objects created in total.\n", pem_nobjs); + plog("Looking for: "); + /* + * now determine type of the object +@@ -299,22 +300,18 @@ collect_objects(CK_ATTRIBUTE_PTR pTemplate, + } + + /* find objects */ +- for (i = 0; i < pem_nobjs; i++) { ++ list_for_each_entry(obj, &pem_objs, gl_list) { + int match = 1; /* matches type if type not specified */ +- if (NULL == pem_objs[i]) +- continue; +- +- plog(" %d type = %d\n", i, pem_objs[i]->type); ++ plog(" %d type = %d\n", i, obj->type); + if (type != pemAll) { + /* type specified - must match given type */ +- match = (type == pem_objs[i]->type); ++ match = (type == obj->type); + } + if (match) { +- match = (slotID == pem_objs[i]->slotID) && +- (CK_TRUE == pem_match(pTemplate, ulAttributeCount, pem_objs[i])); ++ match = (slotID == obj->slotID) && ++ (CK_TRUE == pem_match(pTemplate, ulAttributeCount, obj)); + } + if (match) { +- pemInternalObject *o = pem_objs[i]; + pemInternalObject **new_result_array = (pemInternalObject **) + ensure_array_capacity(*result_array, + &result_array_capacity, +@@ -328,7 +325,7 @@ collect_objects(CK_ATTRIBUTE_PTR pTemplate, + if (*result_array != new_result_array) { + *result_array = new_result_array; + } +- (*result_array)[ result_array_entries ] = o; ++ (*result_array)[ result_array_entries ] = obj; + result_array_entries++; + } + } +diff --git a/src/pinst.c b/src/pinst.c +index a5901f0..810db5a 100644 +--- a/src/pinst.c ++++ b/src/pinst.c +@@ -50,7 +50,7 @@ + + static PRBool pemInitialized = PR_FALSE; + +-pemInternalObject **pem_objs; ++LIST_HEAD(pem_objs); + int pem_nobjs = 0; + int token_needsLogin[NUM_SLOTS]; + +@@ -361,13 +361,9 @@ derEncodingsMatch(CK_OBJECT_CLASS objClass, pemInternalObject * obj, + static CK_RV + LinkSharedKeyObject(int oldKeyIdx, int newKeyIdx) + { +- int i; +- for (i = 0; i < pem_nobjs; i++) { ++ pemInternalObject *obj; ++ list_for_each_entry(obj, &pem_objs, gl_list) { + CK_RV rv; +- pemInternalObject *obj = pem_objs[i]; +- if (NULL == obj) +- continue; +- + if (atoi(obj->id.data) != oldKeyIdx) + continue; + +@@ -380,13 +376,26 @@ LinkSharedKeyObject(int oldKeyIdx, int newKeyIdx) + return CKR_OK; + } + ++/* return pointer to the internal object with given arrayIdx */ ++static pemInternalObject * ++FindObjectByArrayIdx(const long arrayIdx) ++{ ++ pemInternalObject *obj; ++ ++ list_for_each_entry(obj, &pem_objs, gl_list) ++ if (arrayIdx == obj->arrayIdx) ++ return obj; ++ ++ return NULL; ++} ++ + pemInternalObject * + AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + pemObjectType type, SECItem * certDER, + SECItem * keyDER, const char *filename, + int objid, CK_SLOT_ID slotID, PRBool *pAdded) + { +- int i; ++ pemInternalObject *curObj; + + const char *nickname = strrchr(filename, '/'); + if (nickname +@@ -400,11 +409,7 @@ AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + *pAdded = PR_FALSE; + + /* first look for the object in pem_objs, it might be already there */ +- for (i = 0; i < pem_nobjs; i++) { +- pemInternalObject *const curObj = pem_objs[i]; +- if (NULL == curObj) +- continue; +- ++ list_for_each_entry(curObj, &pem_objs, gl_list) { + /* Comparing DER encodings is dependable and frees the PEM module + * from having to require clients to provide unique nicknames. + */ +@@ -418,11 +423,11 @@ AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + * the key object is shared by multiple client certificates, such + * an assumption does not hold. We have to update the references. + */ +- LinkSharedKeyObject(pem_nobjs, i); ++ LinkSharedKeyObject(pem_nobjs, curObj->arrayIdx); + + if (CKO_CERTIFICATE == objClass) { + const int ref = atoi(curObj->id.data); +- if (0 < ref && ref < pem_nobjs && !pem_objs[ref]) { ++ if (0 < ref && ref < pem_nobjs && !FindObjectByArrayIdx(ref)) { + /* The certificate we are going to reuse refers to an + * object that has already been removed. Make it refer + * to the object that will be added next (private key). +@@ -432,7 +437,8 @@ AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + } + } + +- plog("AddObjectIfNeeded: re-using internal object #%i\n", i); ++ plog("AddObjectIfNeeded: re-using internal object #%li\n", ++ curObj->arrayIdx); + curObj->refCount ++; + return curObj; + } +@@ -447,18 +453,9 @@ AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + /* initialize pointers to functions */ + pem_CreateMDObject(NULL, io, NULL); + +- io->gobjIndex = pem_nobjs; +- +- /* add object to global array */ +- if (pem_objs) +- pem_objs = NSS_ZRealloc(pem_objs, +- (pem_nobjs + 1) * sizeof(pemInternalObject *)); +- else +- pem_objs = NSS_ZNEWARRAY(NULL, pemInternalObject *, 1); +- if (!pem_objs) +- return NULL; +- pem_objs[pem_nobjs] = io; +- pem_nobjs++; ++ /* add object to global list */ ++ io->arrayIdx = pem_nobjs++; ++ list_add_tail(&io->gl_list, &pem_objs); + + if (pAdded) + *pAdded = PR_TRUE; +@@ -747,8 +744,7 @@ pem_Finalize + if (!pemInitialized) + return; + +- NSS_ZFreeIf(pem_objs); +- pem_objs = NULL; ++ INIT_LIST_HEAD(&pem_objs); + pem_nobjs = 0; + + PR_AtomicSet(&pemInitialized, PR_FALSE); +diff --git a/src/pobject.c b/src/pobject.c +index d80092a..f118f5d 100644 +--- a/src/pobject.c ++++ b/src/pobject.c +@@ -672,10 +672,8 @@ pem_DestroyInternalObject + return; + } + +- if (NULL != pem_objs) +- /* remove reference to self from the global array */ +- pem_objs[io->gobjIndex] = NULL; +- ++ /* remove self from the global list */ ++ list_del(&io->gl_list); + NSS_ZFreeIf(io); + return; + } +@@ -1206,7 +1204,7 @@ pem_CreateObject + goto loser; + } + } else if (objClass == CKO_PRIVATE_KEY) { +- int i; ++ pemInternalObject *curObj; + SECItem certDER; + CK_SESSION_HANDLE hSession; + PRBool added; +@@ -1221,30 +1219,27 @@ pem_CreateObject + + /* Brute force: find the id of the certificate, if any, in this slot */ + objid = -1; +- for (i = pem_nobjs - 1; 0 <= i; i--) { +- if (NULL == pem_objs[i]) +- continue; +- +- if (slotID != pem_objs[i]->slotID) ++ list_for_each_entry_reverse(curObj, &pem_objs, gl_list) { ++ if (slotID != curObj->slotID) + continue; + +- if (pem_objs[i]->type != pemCert) ++ if (curObj->type != pemCert) + continue; + +- if (atoi(pem_objs[i]->id.data) != pem_nobjs) ++ if (atoi(curObj->id.data) != pem_nobjs) + /* not a certificate that refers to the key being added */ + continue; + + objid = pem_nobjs; +- certDER.data = NSS_ZAlloc(NULL, pem_objs[i]->derCert->len); ++ certDER.data = NSS_ZAlloc(NULL, curObj->derCert->len); + + if (certDER.data == NULL) + goto loser; + +- certDER.len = pem_objs[i]->derCert->len; ++ certDER.len = curObj->derCert->len; + memcpy(certDER.data, +- pem_objs[i]->derCert->data, +- pem_objs[i]->derCert->len); ++ curObj->derCert->data, ++ curObj->derCert->len); + break; + } + +diff --git a/src/psession.c b/src/psession.c +index 4e94a7f..13a5e5d 100644 +--- a/src/psession.c ++++ b/src/psession.c +@@ -241,7 +241,7 @@ pem_mdSession_Login + NSSLOWKEYPrivateKey *lpk = NULL; + PLArenaPool *arena; + SECItem plain; +- int i; ++ pemInternalObject *curObj; + + fwSlot = NSSCKFWToken_GetFWSlot(fwToken); + slotID = NSSCKFWSlot_GetSlotID(fwSlot); +@@ -256,12 +256,9 @@ pem_mdSession_Login + token_needsLogin[slotID - 1] = PR_FALSE; + + /* Find the right key object */ +- for (i = 0; i < pem_nobjs; i++) { +- if (NULL == pem_objs[i]) +- continue; +- +- if ((slotID == pem_objs[i]->slotID) && (pem_objs[i]->type == pemBareKey)) { +- io = pem_objs[i]; ++ list_for_each_entry(curObj, &pem_objs, gl_list) { ++ if ((slotID == curObj->slotID) && (curObj->type == pemBareKey)) { ++ io = curObj; + break; + } + } +-- +2.17.2 + + +From 6d968e0f54bfd48029086ae41067e6024bde76a2 Mon Sep 17 00:00:00 2001 +From: Kamil Dudka +Date: Fri, 21 Dec 2018 13:43:47 +0100 +Subject: [PATCH 3/5] ckpem: define pem_nobjs and objid as long to avoid + wraparound + +Fixes #2 + +Upstream-commit: 5a14b625f529b35f997f60f4ff302f0bfc6ddc16 +Signed-off-by: Kamil Dudka +--- + src/ckpem.h | 4 ++-- + src/pfind.c | 2 +- + src/pinst.c | 29 +++++++++++++++-------------- + src/pobject.c | 4 ++-- + 4 files changed, 20 insertions(+), 19 deletions(-) + +diff --git a/src/ckpem.h b/src/ckpem.h +index caad74b..5d05087 100644 +--- a/src/ckpem.h ++++ b/src/ckpem.h +@@ -216,7 +216,7 @@ struct pemInternalObjectStr { + }; + + NSS_EXTERN_DATA struct list_head pem_objs; +-NSS_EXTERN_DATA int pem_nobjs; ++NSS_EXTERN_DATA long pem_nobjs; + NSS_EXTERN_DATA int token_needsLogin[]; + + struct pemTokenStr { +@@ -303,7 +303,7 @@ PRBool pem_ParseString(const char *inputstring, const char delimiter, + + pemInternalObject * + AddObjectIfNeeded(CK_OBJECT_CLASS objClass, pemObjectType type, +- SECItem *certDER, SECItem *keyDER, const char *filename, int objid, ++ SECItem *certDER, SECItem *keyDER, const char *filename, long objid, + CK_SLOT_ID slotID, PRBool *pAdded); + + void pem_DestroyInternalObject (pemInternalObject *io); +diff --git a/src/pfind.c b/src/pfind.c +index 3f0fdcb..958c0d5 100644 +--- a/src/pfind.c ++++ b/src/pfind.c +@@ -259,7 +259,7 @@ collect_objects(CK_ATTRIBUTE_PTR pTemplate, + + plog("collect_objects slot #%ld, ", slotID); + plog("%d attributes, ", ulAttributeCount); +- plog("%d objects created in total.\n", pem_nobjs); ++ plog("%ld objects created in total.\n", pem_nobjs); + plog("Looking for: "); + /* + * now determine type of the object +diff --git a/src/pinst.c b/src/pinst.c +index 810db5a..87bf7c2 100644 +--- a/src/pinst.c ++++ b/src/pinst.c +@@ -51,7 +51,7 @@ + static PRBool pemInitialized = PR_FALSE; + + LIST_HEAD(pem_objs); +-int pem_nobjs = 0; ++long pem_nobjs = 0L; + int token_needsLogin[NUM_SLOTS]; + + /* +@@ -181,12 +181,12 @@ GetCertFields(unsigned char *cert, int cert_length, + } + + static CK_RV +-assignObjectID(pemInternalObject *o, int objid) ++assignObjectID(pemInternalObject *o, const long objid) + { +- char id[16]; ++ char id[24]; + int len; + +- sprintf(id, "%d", objid); ++ sprintf(id, "%ld", objid); + len = strlen(id) + 1; /* zero terminate */ + o->id.size = len; + o->id.data = NSS_ZAlloc(NULL, len); +@@ -201,7 +201,7 @@ static pemInternalObject * + CreateObject(CK_OBJECT_CLASS objClass, + pemObjectType type, SECItem * certDER, + SECItem * keyDER, const char *filename, +- int objid, CK_SLOT_ID slotID) ++ long objid, CK_SLOT_ID slotID) + { + pemInternalObject *o; + SECItem subject; +@@ -225,17 +225,17 @@ CreateObject(CK_OBJECT_CLASS objClass, + + switch (objClass) { + case CKO_CERTIFICATE: +- plog("Creating cert nick %s id %d in slot %ld\n", nickname, objid, slotID); ++ plog("Creating cert nick %s id %ld in slot %ld\n", nickname, objid, slotID); + memset(&o->u.cert, 0, sizeof(o->u.cert)); + break; + case CKO_PRIVATE_KEY: +- plog("Creating key id %d in slot %ld\n", objid, slotID); ++ plog("Creating key id %ld in slot %ld\n", objid, slotID); + memset(&o->u.key, 0, sizeof(o->u.key)); + /* more unique nicknames - https://bugzilla.redhat.com/689031#c66 */ + nickname = filename; + break; + case CKO_NETSCAPE_TRUST: +- plog("Creating trust nick %s id %d in slot %ld\n", nickname, objid, slotID); ++ plog("Creating trust nick %s id %ld in slot %ld\n", nickname, objid, slotID); + memset(&o->u.trust, 0, sizeof(o->u.trust)); + break; + } +@@ -359,12 +359,12 @@ derEncodingsMatch(CK_OBJECT_CLASS objClass, pemInternalObject * obj, + } + + static CK_RV +-LinkSharedKeyObject(int oldKeyIdx, int newKeyIdx) ++LinkSharedKeyObject(const long oldKeyIdx, const long newKeyIdx) + { + pemInternalObject *obj; + list_for_each_entry(obj, &pem_objs, gl_list) { + CK_RV rv; +- if (atoi(obj->id.data) != oldKeyIdx) ++ if (atol(obj->id.data) != oldKeyIdx) + continue; + + NSS_ZFreeIf(obj->id.data); +@@ -393,7 +393,7 @@ pemInternalObject * + AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + pemObjectType type, SECItem * certDER, + SECItem * keyDER, const char *filename, +- int objid, CK_SLOT_ID slotID, PRBool *pAdded) ++ long objid, CK_SLOT_ID slotID, PRBool *pAdded) + { + pemInternalObject *curObj; + +@@ -426,7 +426,7 @@ AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + LinkSharedKeyObject(pem_nobjs, curObj->arrayIdx); + + if (CKO_CERTIFICATE == objClass) { +- const int ref = atoi(curObj->id.data); ++ const long ref = atol(curObj->id.data); + if (0 < ref && ref < pem_nobjs && !FindObjectByArrayIdx(ref)) { + /* The certificate we are going to reuse refers to an + * object that has already been removed. Make it refer +@@ -470,7 +470,8 @@ AddCertificate(char *certfile, char *keyfile, PRBool cacert, + { + pemInternalObject *o = NULL; + CK_RV error = 0; +- int objid, i = 0; ++ long objid; ++ int i = 0; + SECItem **objs = NULL; + char *ivstring = NULL; + int cipher; +@@ -745,7 +746,7 @@ pem_Finalize + return; + + INIT_LIST_HEAD(&pem_objs); +- pem_nobjs = 0; ++ pem_nobjs = 0L; + + PR_AtomicSet(&pemInitialized, PR_FALSE); + } +diff --git a/src/pobject.c b/src/pobject.c +index f118f5d..9a8321c 100644 +--- a/src/pobject.c ++++ b/src/pobject.c +@@ -1089,7 +1089,7 @@ pem_CreateObject + SECItem **derlist = NULL; + int nobjs = 0; + int i; +- int objid; ++ long objid; + #if 0 + pemToken *token; + #endif +@@ -1226,7 +1226,7 @@ pem_CreateObject + if (curObj->type != pemCert) + continue; + +- if (atoi(curObj->id.data) != pem_nobjs) ++ if (atol(curObj->id.data) != pem_nobjs) + /* not a certificate that refers to the key being added */ + continue; + +-- +2.17.2 + + +From 513ac624ec4c640f53f2bcdee11dc332358b77d7 Mon Sep 17 00:00:00 2001 +From: Kamil Dudka +Date: Fri, 21 Dec 2018 14:26:01 +0100 +Subject: [PATCH 4/5] ckpem: keep objid as number to avoid use of atol() + +Fixes #2 + +Upstream-commit: 188c63f7b824e256d20b70ee080ea88d96d8e9c6 +Signed-off-by: Kamil Dudka +--- + src/ckpem.h | 1 + + src/pinst.c | 5 +++-- + src/pobject.c | 2 +- + 3 files changed, 5 insertions(+), 3 deletions(-) + +diff --git a/src/ckpem.h b/src/ckpem.h +index 5d05087..e2f18cf 100644 +--- a/src/ckpem.h ++++ b/src/ckpem.h +@@ -194,6 +194,7 @@ struct pemInternalObjectStr { + CK_OBJECT_CLASS objClass; + NSSItem hashKey; + NSSItem id; ++ long objid; + unsigned char hashKeyData[128]; + SECItem *derCert; + char *nickname; +diff --git a/src/pinst.c b/src/pinst.c +index 87bf7c2..26f3ac2 100644 +--- a/src/pinst.c ++++ b/src/pinst.c +@@ -194,6 +194,7 @@ assignObjectID(pemInternalObject *o, const long objid) + return CKR_HOST_MEMORY; + + memcpy(o->id.data, id, len); ++ o->objid = objid; + return CKR_OK; + } + +@@ -364,7 +365,7 @@ LinkSharedKeyObject(const long oldKeyIdx, const long newKeyIdx) + pemInternalObject *obj; + list_for_each_entry(obj, &pem_objs, gl_list) { + CK_RV rv; +- if (atol(obj->id.data) != oldKeyIdx) ++ if (obj->objid != oldKeyIdx) + continue; + + NSS_ZFreeIf(obj->id.data); +@@ -426,7 +427,7 @@ AddObjectIfNeeded(CK_OBJECT_CLASS objClass, + LinkSharedKeyObject(pem_nobjs, curObj->arrayIdx); + + if (CKO_CERTIFICATE == objClass) { +- const long ref = atol(curObj->id.data); ++ const long ref = curObj->objid; + if (0 < ref && ref < pem_nobjs && !FindObjectByArrayIdx(ref)) { + /* The certificate we are going to reuse refers to an + * object that has already been removed. Make it refer +diff --git a/src/pobject.c b/src/pobject.c +index 9a8321c..2ffffed 100644 +--- a/src/pobject.c ++++ b/src/pobject.c +@@ -1226,7 +1226,7 @@ pem_CreateObject + if (curObj->type != pemCert) + continue; + +- if (atol(curObj->id.data) != pem_nobjs) ++ if (curObj->objid != pem_nobjs) + /* not a certificate that refers to the key being added */ + continue; + +-- +2.17.2 + + +From db140f2d9259ac73d59c50a658d444e418b81c3c Mon Sep 17 00:00:00 2001 +From: Kamil Dudka +Date: Wed, 16 Jan 2019 16:19:50 +0100 +Subject: [PATCH 5/5] pfind: fix use of uninitialized var in plog() call + +Detected by static analysis: + +Error: UNINIT (CWE-457): +nss-pem-1.0.3/src/pfind.c:251: var_decl: Declaring variable "i" without initializer. +nss-pem-1.0.3/src/pfind.c:305: uninit_use_in_call: Using uninitialized value "i" when calling "plog". + 303| list_for_each_entry(obj, &pem_objs, gl_list) { + 304| int match = 1; /* matches type if type not specified */ + 305|-> plog(" %d type = %d\n", i, obj->type); + 306| if (type != pemAll) { + 307| /* type specified - must match given type */ + +Error: CLANG_WARNING: +nss-pem-1.0.3/src/pfind.c:305:9: warning: Function call argument is an uninitialized value +nss-pem-1.0.3/src/pfind.c:364:9: note: Assuming 'fwSlot' is not equal to null +nss-pem-1.0.3/src/pfind.c:364:5: note: Taking false branch +nss-pem-1.0.3/src/pfind.c:370:9: note: Assuming 'arena' is not equal to null +nss-pem-1.0.3/src/pfind.c:370:5: note: Taking false branch +nss-pem-1.0.3/src/pfind.c:375:9: note: Assuming 'rv' is not equal to null +nss-pem-1.0.3/src/pfind.c:375:5: note: Taking false branch +nss-pem-1.0.3/src/pfind.c:381:9: note: Assuming 'fo' is not equal to null +nss-pem-1.0.3/src/pfind.c:381:5: note: Taking false branch +nss-pem-1.0.3/src/pfind.c:395:9: note: Calling 'collect_objects' +nss-pem-1.0.3/src/pfind.c:251:5: note: 'i' declared without an initial value +nss-pem-1.0.3/src/pfind.c:267:5: note: Control jumps to 'case 0:' at line 293 +nss-pem-1.0.3/src/pfind.c:296:9: note: Execution continues on line 303 +nss-pem-1.0.3/src/pfind.c:305:9: note: Function call argument is an uninitialized value + +Error: COMPILER_WARNING: +nss-pem-1.0.3/src/pfind.c: scope_hint: In function 'pem_FindObjectsInit' +nss-pem-1.0.3/src/pfind.c:305:13: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized] +nss-pem-1.0.3/src/pfind.c:251:14: note: 'i' was declared here + +Upstream-commit: 5c05ed268675c7544b1a51198e1e9e566be17608 +Signed-off-by: Kamil Dudka +--- + src/pfind.c | 3 +-- + 1 file changed, 1 insertion(+), 2 deletions(-) + +diff --git a/src/pfind.c b/src/pfind.c +index 958c0d5..83d5b89 100644 +--- a/src/pfind.c ++++ b/src/pfind.c +@@ -248,7 +248,6 @@ collect_objects(CK_ATTRIBUTE_PTR pTemplate, + pemInternalObject *** result_array, + CK_RV * pError, CK_SLOT_ID slotID) + { +- PRUint32 i; + size_t result_array_entries = 0; + size_t result_array_capacity = 0; + pemObjectType type = pemRaw; +@@ -302,7 +301,7 @@ collect_objects(CK_ATTRIBUTE_PTR pTemplate, + /* find objects */ + list_for_each_entry(obj, &pem_objs, gl_list) { + int match = 1; /* matches type if type not specified */ +- plog(" %d type = %d\n", i, obj->type); ++ plog(" %ld type = %d\n", obj->arrayIdx, obj->type); + if (type != pemAll) { + /* type specified - must match given type */ + match = (type == obj->type); +-- +2.17.2 + diff --git a/SPECS/nss-pem.spec b/SPECS/nss-pem.spec index 80266bb..6dfbf01 100644 --- a/SPECS/nss-pem.spec +++ b/SPECS/nss-pem.spec @@ -1,6 +1,6 @@ Name: nss-pem Version: 1.0.3 -Release: 5%{?dist} +Release: 5%{?dist}.1 Summary: PEM file reader for Network Security Services (NSS) License: MPLv1.1 @@ -11,6 +11,9 @@ Patch1: 0001-nss-pem-1.0.3-drop-wait-for-slot-event-cb.patch # update object ID while reusing a certificate (#1610998) Patch2: 0002-nss-pem-1.0.3-key-reload.patch +# fix performance regression in libcurl (#1659108) +Patch3: 0003-nss-pem-1.0.3-make-pem_objs-a-list-instead-of-array.patch + BuildRequires: cmake BuildRequires: nss-pkcs11-devel @@ -28,6 +31,7 @@ module. %setup -q %patch1 -p1 %patch2 -p1 +%patch3 -p1 %build mkdir build @@ -48,6 +52,9 @@ ctest %{?_smp_mflags} --output-on-failure %license COPYING %changelog +* Mon Jan 14 2019 Kamil Dudka 1.0.3-5.el7_6.1 +- fix performance regression in libcurl (#1659108) + * Wed Aug 08 2018 Kamil Dudka 1.0.3-5 - update object ID while reusing a certificate (#1610998)