nss: work around race condition in PK11_FindSlotByName()

wangp · kdudka · commit 3a5d5de9ef52 · 2016-08-26T14:42:22.000+02:00
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
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
@@ -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:
 
@@ -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
diff --git a/lib/vtls/nss.c b/lib/vtls/nss.c
@@ -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;
@@ -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(). */
@@ -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;
@@ -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;
 
@@ -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;
@@ -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 */
@@ -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;
