Skip to content

Commit

Permalink
nss: work around race condition in PK11_FindSlotByName()
Browse files Browse the repository at this point in the history
Serialise the call to PK11_FindSlotByName() to avoid spurious errors in
a multi-threaded environment. The underlying cause is a race condition
in nssSlot_IsTokenPresent().

Bug: https://bugzilla.mozilla.org/1297397

Closes #985
  • Loading branch information
wangp authored and kdudka committed Aug 26, 2016
1 parent 7700fcb commit 3a5d5de
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
2 changes: 2 additions & 0 deletions RELEASE-NOTES
Expand Up @@ -38,6 +38,7 @@ This release includes the following bugfixes:
o sasl: Don't use GSSAPI authentication when domain name not specified [16]
o win: Basic support for Universal Windows Platform apps [17]
o nss: fix incorrect use of a previously loaded certificate from file
o nss: work around race condition in PK11_FindSlotByName() [18]

This release includes the following known bugs:

Expand Down Expand Up @@ -75,3 +76,4 @@ References to bug reports and discussions on issues:
[15] = https://curl.haxx.se/bug/?i=970
[16] = https://curl.haxx.se/bug/?i=718
[17] = https://curl.haxx.se/bug/?i=820
[18] = https://bugzilla.mozilla.org/1297397
22 changes: 19 additions & 3 deletions lib/vtls/nss.c
Expand Up @@ -80,6 +80,7 @@
PRFileDesc *PR_ImportTCPSocket(PRInt32 osfd);
static PRLock *nss_initlock = NULL;
static PRLock *nss_crllock = NULL;
static PRLock *nss_findslot_lock = NULL;
static struct curl_llist *nss_crl_list = NULL;
static NSSInitContext *nss_context = NULL;
static volatile int initialized = 0;
Expand Down Expand Up @@ -338,6 +339,19 @@ static char* dup_nickname(struct Curl_easy *data, enum dupstring cert_kind)
return NULL;
}

/* Lock/unlock wrapper for PK11_FindSlotByName() to work around race condition
* in nssSlot_IsTokenPresent() causing spurious SEC_ERROR_NO_TOKEN. For more
* details, go to <https://bugzilla.mozilla.org/1297397>.
*/
static PK11SlotInfo* nss_find_slot_by_name(const char *slot_name)
{
PK11SlotInfo *slot;
PR_Lock(nss_initlock);
slot = PK11_FindSlotByName(slot_name);
PR_Unlock(nss_initlock);
return slot;
}

/* 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(). */
Expand All @@ -360,7 +374,7 @@ static CURLcode nss_create_object(struct ssl_connect_data *ssl,
if(!slot_name)
return CURLE_OUT_OF_MEMORY;

slot = PK11_FindSlotByName(slot_name);
slot = nss_find_slot_by_name(slot_name);
free(slot_name);
if(!slot)
return result;
Expand Down Expand Up @@ -561,7 +575,7 @@ static CURLcode nss_load_key(struct connectdata *conn, int sockindex,
return result;
}

slot = PK11_FindSlotByName("PEM Token #1");
slot = nss_find_slot_by_name("PEM Token #1");
if(!slot)
return CURLE_SSL_CERTPROBLEM;

Expand Down Expand Up @@ -1011,7 +1025,7 @@ static SECStatus SelectClientCert(void *arg, PRFileDesc *sock,
struct CERTCertificateStr *cert;
struct SECKEYPrivateKeyStr *key;

PK11SlotInfo *slot = PK11_FindSlotByName(pem_slotname);
PK11SlotInfo *slot = nss_find_slot_by_name(pem_slotname);
if(NULL == slot) {
failf(data, "NSS: PK11 slot not found: %s", pem_slotname);
return SECFailure;
Expand Down Expand Up @@ -1247,6 +1261,7 @@ int Curl_nss_init(void)
PR_Init(PR_USER_THREAD, PR_PRIORITY_NORMAL, 256);
nss_initlock = PR_NewLock();
nss_crllock = PR_NewLock();
nss_findslot_lock = PR_NewLock();
}

/* We will actually initialize NSS later */
Expand Down Expand Up @@ -1301,6 +1316,7 @@ void Curl_nss_cleanup(void)

PR_DestroyLock(nss_initlock);
PR_DestroyLock(nss_crllock);
PR_DestroyLock(nss_findslot_lock);
nss_initlock = NULL;

initialized = 0;
Expand Down

0 comments on commit 3a5d5de

Please sign in to comment.