NSS: fix session cache lock initialisation.

This code previously used an unprotected global, that was accessed from
multiple threads, to guard whether global locks had been created. See bug for
details.

BUG=292999

Review URL: https://chromiumcodereview.appspot.com/24216005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224311 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 296 insertions, 99 deletions

diff --git a/net/third_party/nss/README.chromium b/net/third_party/nss/README.chromium
index 73eb259..b451f62 100644
--- a/net/third_party/nss/README.chromium
+++ b/net/third_party/nss/README.chromium
@@ -116,6 +116,10 @@ Patches:
     http://tools.ietf.org/html/draft-agl-tls-chacha20poly1305-01
     patches/chacha20poly1305.patch
 
+  * Fix session cache lock creation race.
+    patches/cachelocks.patch
+    https://bugzilla.mozilla.org/show_bug.cgi?id=764646
+
 Apply the patches to NSS by running the patches/applypatches.sh script.  Read
 the comments at the top of patches/applypatches.sh for instructions.
 
diff --git a/net/third_party/nss/patches/applypatches.sh b/net/third_party/nss/patches/applypatches.sh
index e936182..6bc2104 100755
--- a/net/third_party/nss/patches/applypatches.sh
+++ b/net/third_party/nss/patches/applypatches.sh
@@ -55,3 +55,5 @@ patch -p4 < $patches_dir/aesgcmchromium.patch
 patch -p4 < $patches_dir/tls12backuphash.patch
 
 patch -p4 < $patches_dir/chacha20poly1305.patch
+
+patch -p4 < $patches_dir/cachelocks.patch
diff --git a/net/third_party/nss/patches/cachelocks.patch b/net/third_party/nss/patches/cachelocks.patch
new file mode 100644
index 0000000..5b3f93e
--- /dev/null
+++ b/net/third_party/nss/patches/cachelocks.patch
@@ -0,0 +1,246 @@
+diff --git a/nss/lib/ssl/ssl3con.c b/nss/lib/ssl/ssl3con.c
+index 53c29f0..bc54c99 100644
+--- a/nss/lib/ssl/ssl3con.c
++++ b/nss/lib/ssl/ssl3con.c
+@@ -5593,7 +5593,6 @@ SSL3_ShutdownServerCache(void)
+     }
+ 
+     PZ_Unlock(symWrapKeysLock);
+-    ssl_FreeSessionCacheLocks();
+     return SECSuccess;
+ }
+ 
+@@ -5645,7 +5644,7 @@ getWrappingKey( sslSocket *       ss,
+ 
+     pSymWrapKey = &symWrapKeys[symWrapMechIndex].symWrapKey[exchKeyType];
+ 
+-    ssl_InitSessionCacheLocks(PR_TRUE);
++    ssl_InitSessionCacheLocks();
+ 
+     PZ_Lock(symWrapKeysLock);
+ 
+diff --git a/nss/lib/ssl/sslimpl.h b/nss/lib/ssl/sslimpl.h
+index e3ae9ce..59140f8 100644
+--- a/nss/lib/ssl/sslimpl.h
++++ b/nss/lib/ssl/sslimpl.h
+@@ -1845,9 +1845,7 @@ extern SECStatus ssl_InitSymWrapKeysLock(void);
+ 
+ extern SECStatus ssl_FreeSymWrapKeysLock(void);
+ 
+-extern SECStatus ssl_InitSessionCacheLocks(PRBool lazyInit);
+-
+-extern SECStatus ssl_FreeSessionCacheLocks(void);
++extern SECStatus ssl_InitSessionCacheLocks(void);
+ 
+ /***************** platform client auth ****************/
+ 
+diff --git a/nss/lib/ssl/sslnonce.c b/nss/lib/ssl/sslnonce.c
+index 5d8a954..a6f7349 100644
+--- a/nss/lib/ssl/sslnonce.c
++++ b/nss/lib/ssl/sslnonce.c
+@@ -35,91 +35,55 @@ static PZLock *      cacheLock = NULL;
+ #define LOCK_CACHE 	lock_cache()
+ #define UNLOCK_CACHE	PZ_Unlock(cacheLock)
+ 
++static PRCallOnceType lockOnce;
++
++/* FreeSessionCacheLocks is a callback from NSS_RegisterShutdown which destroys
++ * the session cache locks on shutdown and resets them to their initial
++ * state. */
+ static SECStatus
+-ssl_InitClientSessionCacheLock(void)
++FreeSessionCacheLocks(void* appData, void* nssData)
+ {
++    static const PRCallOnceType pristineCallOnce;
++    SECStatus rv;
++
++    if (!cacheLock) {
++        PORT_SetError(SEC_ERROR_NOT_INITIALIZED);
++        return SECFailure;
++    }
++
++    PZ_DestroyLock(cacheLock);
++    cacheLock = NULL;
++
++    rv = ssl_FreeSymWrapKeysLock();
++    if (rv != SECSuccess) {
++        return rv;
++    }
++
++    lockOnce = pristineCallOnce;
++    return SECSuccess;
++}
++
++/* InitSessionCacheLocks is called, protected by lockOnce, to create the
++ * session cache locks. */
++static PRStatus
++InitSessionCacheLocks(void)
++{
++    SECStatus rv;
++
+     cacheLock = PZ_NewLock(nssILockCache);
+-    return cacheLock ? SECSuccess : SECFailure;
+-}
+-
+-static SECStatus
+-ssl_FreeClientSessionCacheLock(void)
+-{
+-    if (cacheLock) {
++    if (cacheLock == NULL) {
++        return PR_FAILURE;
++    }
++    rv = ssl_InitSymWrapKeysLock();
++    if (rv != SECSuccess) {
++        PRErrorCode error = PORT_GetError();
+         PZ_DestroyLock(cacheLock);
+         cacheLock = NULL;
+-        return SECSuccess;
+-    }
+-    PORT_SetError(SEC_ERROR_NOT_INITIALIZED);
+-    return SECFailure;
+-}
+-
+-static PRBool LocksInitializedEarly = PR_FALSE;
+-
+-static SECStatus
+-FreeSessionCacheLocks()
+-{
+-    SECStatus rv1, rv2;
+-    rv1 = ssl_FreeSymWrapKeysLock();
+-    rv2 = ssl_FreeClientSessionCacheLock();
+-    if ( (SECSuccess == rv1) && (SECSuccess == rv2) ) {
+-        return SECSuccess;
+-    }
+-    return SECFailure;
+-}
+-
+-static SECStatus
+-InitSessionCacheLocks(void)
+-{
+-    SECStatus rv1, rv2;
+-    PRErrorCode rc;
+-    rv1 = ssl_InitSymWrapKeysLock();
+-    rv2 = ssl_InitClientSessionCacheLock();
+-    if ( (SECSuccess == rv1) && (SECSuccess == rv2) ) {
+-        return SECSuccess;
+-    }
+-    rc = PORT_GetError();
+-    FreeSessionCacheLocks();
+-    PORT_SetError(rc);
+-    return SECFailure;
+-}
+-
+-/* free the session cache locks if they were initialized early */
+-SECStatus
+-ssl_FreeSessionCacheLocks()
+-{
+-    PORT_Assert(PR_TRUE == LocksInitializedEarly);
+-    if (!LocksInitializedEarly) {
+-        PORT_SetError(SEC_ERROR_NOT_INITIALIZED);
+-        return SECFailure;
+-    }
+-    FreeSessionCacheLocks();
+-    LocksInitializedEarly = PR_FALSE;
+-    return SECSuccess;
+-}
+-
+-static PRCallOnceType lockOnce;
+-
+-/* free the session cache locks if they were initialized lazily */
+-static SECStatus ssl_ShutdownLocks(void* appData, void* nssData)
+-{
+-    PORT_Assert(PR_FALSE == LocksInitializedEarly);
+-    if (LocksInitializedEarly) {
+-        PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
+-        return SECFailure;
+-    }
+-    FreeSessionCacheLocks();
+-    memset(&lockOnce, 0, sizeof(lockOnce));
+-    return SECSuccess;
+-}
+-
+-static PRStatus initSessionCacheLocksLazily(void)
+-{
+-    SECStatus rv = InitSessionCacheLocks();
+-    if (SECSuccess != rv) {
++        PORT_SetError(error);
+         return PR_FAILURE;
+     }
+-    rv = NSS_RegisterShutdown(ssl_ShutdownLocks, NULL);
++
++    rv = NSS_RegisterShutdown(FreeSessionCacheLocks, NULL);
+     PORT_Assert(SECSuccess == rv);
+     if (SECSuccess != rv) {
+         return PR_FAILURE;
+@@ -127,34 +91,18 @@ static PRStatus initSessionCacheLocksLazily(void)
+     return PR_SUCCESS;
+ }
+ 
+-/* lazyInit means that the call is not happening during a 1-time
+- * initialization function, but rather during dynamic, lazy initialization
+- */
+ SECStatus
+-ssl_InitSessionCacheLocks(PRBool lazyInit)
++ssl_InitSessionCacheLocks(void)
+ {
+-    if (LocksInitializedEarly) {
+-        return SECSuccess;
+-    }
+-
+-    if (lazyInit) {
+-        return (PR_SUCCESS ==
+-                PR_CallOnce(&lockOnce, initSessionCacheLocksLazily)) ?
+-               SECSuccess : SECFailure;
+-    }
+-     
+-    if (SECSuccess == InitSessionCacheLocks()) {
+-        LocksInitializedEarly = PR_TRUE;
+-        return SECSuccess;
+-    }
+-
+-    return SECFailure;
++    return (PR_SUCCESS ==
++            PR_CallOnce(&lockOnce, InitSessionCacheLocks)) ?
++           SECSuccess : SECFailure;
+ }
+ 
+-static void 
++static void
+ lock_cache(void)
+ {
+-    ssl_InitSessionCacheLocks(PR_TRUE);
++    ssl_InitSessionCacheLocks();
+     PZ_Lock(cacheLock);
+ }
+ 
+diff --git a/nss/lib/ssl/sslsnce.c b/nss/lib/ssl/sslsnce.c
+index b0446ad..34e07b0 100644
+--- a/nss/lib/ssl/sslsnce.c
++++ b/nss/lib/ssl/sslsnce.c
+@@ -1353,7 +1353,7 @@ SSL_ConfigServerSessionIDCache(	int      maxCacheEntries,
+ 			       	PRUint32 ssl3_timeout, 
+ 			  const char *   directory)
+ {
+-    ssl_InitSessionCacheLocks(PR_FALSE);
++    ssl_InitSessionCacheLocks();
+     return SSL_ConfigServerSessionIDCacheInstance(&globalCache, 
+     		maxCacheEntries, ssl2_timeout, ssl3_timeout, directory, PR_FALSE);
+ }
+@@ -1467,7 +1467,7 @@ SSL_ConfigServerSessionIDCacheWithOpt(
+                                 PRBool enableMPCache)
+ {
+     if (!enableMPCache) {
+-        ssl_InitSessionCacheLocks(PR_FALSE);
++        ssl_InitSessionCacheLocks();
+         return ssl_ConfigServerSessionIDCacheInstanceWithOpt(&globalCache, 
+            ssl2_timeout, ssl3_timeout, directory, PR_FALSE,
+            maxCacheEntries, maxCertCacheEntries, maxSrvNameCacheEntries);
+@@ -1512,7 +1512,7 @@ SSL_InheritMPServerSIDCacheInstance(cacheDesc *cache, const char * envString)
+     	return SECSuccess;	/* already done. */
+     }
+ 
+-    ssl_InitSessionCacheLocks(PR_FALSE);
++    ssl_InitSessionCacheLocks();
+ 
+     ssl_sid_lookup  = ServerSessionIDLookup;
+     ssl_sid_cache   = ServerSessionIDCache;
diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
index 53c29f0..bc54c99 100644
--- a/net/third_party/nss/ssl/ssl3con.c
+++ b/net/third_party/nss/ssl/ssl3con.c
@@ -5593,7 +5593,6 @@ SSL3_ShutdownServerCache(void)
     }
 
     PZ_Unlock(symWrapKeysLock);
-    ssl_FreeSessionCacheLocks();
     return SECSuccess;
 }
 
@@ -5645,7 +5644,7 @@ getWrappingKey( sslSocket *       ss,
 
     pSymWrapKey = &symWrapKeys[symWrapMechIndex].symWrapKey[exchKeyType];
 
-    ssl_InitSessionCacheLocks(PR_TRUE);
+    ssl_InitSessionCacheLocks();
 
     PZ_Lock(symWrapKeysLock);
 
diff --git a/net/third_party/nss/ssl/sslimpl.h b/net/third_party/nss/ssl/sslimpl.h
index e3ae9ce..59140f8 100644
--- a/net/third_party/nss/ssl/sslimpl.h
+++ b/net/third_party/nss/ssl/sslimpl.h
@@ -1845,9 +1845,7 @@ extern SECStatus ssl_InitSymWrapKeysLock(void);
 
 extern SECStatus ssl_FreeSymWrapKeysLock(void);
 
-extern SECStatus ssl_InitSessionCacheLocks(PRBool lazyInit);
-
-extern SECStatus ssl_FreeSessionCacheLocks(void);
+extern SECStatus ssl_InitSessionCacheLocks(void);
 
 /***************** platform client auth ****************/
 
diff --git a/net/third_party/nss/ssl/sslnonce.c b/net/third_party/nss/ssl/sslnonce.c
index 5d8a954..a6f7349 100644
--- a/net/third_party/nss/ssl/sslnonce.c
+++ b/net/third_party/nss/ssl/sslnonce.c
@@ -35,91 +35,55 @@ static PZLock *      cacheLock = NULL;
 #define LOCK_CACHE 	lock_cache()
 #define UNLOCK_CACHE	PZ_Unlock(cacheLock)
 
-static SECStatus
-ssl_InitClientSessionCacheLock(void)
-{
-    cacheLock = PZ_NewLock(nssILockCache);
-    return cacheLock ? SECSuccess : SECFailure;
-}
-
-static SECStatus
-ssl_FreeClientSessionCacheLock(void)
-{
-    if (cacheLock) {
-        PZ_DestroyLock(cacheLock);
-        cacheLock = NULL;
-        return SECSuccess;
-    }
-    PORT_SetError(SEC_ERROR_NOT_INITIALIZED);
-    return SECFailure;
-}
-
-static PRBool LocksInitializedEarly = PR_FALSE;
-
-static SECStatus
-FreeSessionCacheLocks()
-{
-    SECStatus rv1, rv2;
-    rv1 = ssl_FreeSymWrapKeysLock();
-    rv2 = ssl_FreeClientSessionCacheLock();
-    if ( (SECSuccess == rv1) && (SECSuccess == rv2) ) {
-        return SECSuccess;
-    }
-    return SECFailure;
-}
+static PRCallOnceType lockOnce;
 
+/* FreeSessionCacheLocks is a callback from NSS_RegisterShutdown which destroys
+ * the session cache locks on shutdown and resets them to their initial
+ * state. */
 static SECStatus
-InitSessionCacheLocks(void)
+FreeSessionCacheLocks(void* appData, void* nssData)
 {
-    SECStatus rv1, rv2;
-    PRErrorCode rc;
-    rv1 = ssl_InitSymWrapKeysLock();
-    rv2 = ssl_InitClientSessionCacheLock();
-    if ( (SECSuccess == rv1) && (SECSuccess == rv2) ) {
-        return SECSuccess;
-    }
-    rc = PORT_GetError();
-    FreeSessionCacheLocks();
-    PORT_SetError(rc);
-    return SECFailure;
-}
+    static const PRCallOnceType pristineCallOnce;
+    SECStatus rv;
 
-/* free the session cache locks if they were initialized early */
-SECStatus
-ssl_FreeSessionCacheLocks()
-{
-    PORT_Assert(PR_TRUE == LocksInitializedEarly);
-    if (!LocksInitializedEarly) {
+    if (!cacheLock) {
         PORT_SetError(SEC_ERROR_NOT_INITIALIZED);
         return SECFailure;
     }
-    FreeSessionCacheLocks();
-    LocksInitializedEarly = PR_FALSE;
-    return SECSuccess;
-}
 
-static PRCallOnceType lockOnce;
+    PZ_DestroyLock(cacheLock);
+    cacheLock = NULL;
 
-/* free the session cache locks if they were initialized lazily */
-static SECStatus ssl_ShutdownLocks(void* appData, void* nssData)
-{
-    PORT_Assert(PR_FALSE == LocksInitializedEarly);
-    if (LocksInitializedEarly) {
-        PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
-        return SECFailure;
+    rv = ssl_FreeSymWrapKeysLock();
+    if (rv != SECSuccess) {
+        return rv;
     }
-    FreeSessionCacheLocks();
-    memset(&lockOnce, 0, sizeof(lockOnce));
+
+    lockOnce = pristineCallOnce;
     return SECSuccess;
 }
 
-static PRStatus initSessionCacheLocksLazily(void)
+/* InitSessionCacheLocks is called, protected by lockOnce, to create the
+ * session cache locks. */
+static PRStatus
+InitSessionCacheLocks(void)
 {
-    SECStatus rv = InitSessionCacheLocks();
-    if (SECSuccess != rv) {
+    SECStatus rv;
+
+    cacheLock = PZ_NewLock(nssILockCache);
+    if (cacheLock == NULL) {
+        return PR_FAILURE;
+    }
+    rv = ssl_InitSymWrapKeysLock();
+    if (rv != SECSuccess) {
+        PRErrorCode error = PORT_GetError();
+        PZ_DestroyLock(cacheLock);
+        cacheLock = NULL;
+        PORT_SetError(error);
         return PR_FAILURE;
     }
-    rv = NSS_RegisterShutdown(ssl_ShutdownLocks, NULL);
+
+    rv = NSS_RegisterShutdown(FreeSessionCacheLocks, NULL);
     PORT_Assert(SECSuccess == rv);
     if (SECSuccess != rv) {
         return PR_FAILURE;
@@ -127,34 +91,18 @@ static PRStatus initSessionCacheLocksLazily(void)
     return PR_SUCCESS;
 }
 
-/* lazyInit means that the call is not happening during a 1-time
- * initialization function, but rather during dynamic, lazy initialization
- */
 SECStatus
-ssl_InitSessionCacheLocks(PRBool lazyInit)
+ssl_InitSessionCacheLocks(void)
 {
-    if (LocksInitializedEarly) {
-        return SECSuccess;
-    }
-
-    if (lazyInit) {
-        return (PR_SUCCESS ==
-                PR_CallOnce(&lockOnce, initSessionCacheLocksLazily)) ?
-               SECSuccess : SECFailure;
-    }
-     
-    if (SECSuccess == InitSessionCacheLocks()) {
-        LocksInitializedEarly = PR_TRUE;
-        return SECSuccess;
-    }
-
-    return SECFailure;
+    return (PR_SUCCESS ==
+            PR_CallOnce(&lockOnce, InitSessionCacheLocks)) ?
+           SECSuccess : SECFailure;
 }
 
-static void 
+static void
 lock_cache(void)
 {
-    ssl_InitSessionCacheLocks(PR_TRUE);
+    ssl_InitSessionCacheLocks();
     PZ_Lock(cacheLock);
 }
 
diff --git a/net/third_party/nss/ssl/sslsnce.c b/net/third_party/nss/ssl/sslsnce.c
index b0446ad..34e07b0 100644
--- a/net/third_party/nss/ssl/sslsnce.c
+++ b/net/third_party/nss/ssl/sslsnce.c
@@ -1353,7 +1353,7 @@ SSL_ConfigServerSessionIDCache(	int      maxCacheEntries,
 			       	PRUint32 ssl3_timeout, 
 			  const char *   directory)
 {
-    ssl_InitSessionCacheLocks(PR_FALSE);
+    ssl_InitSessionCacheLocks();
     return SSL_ConfigServerSessionIDCacheInstance(&globalCache, 
     		maxCacheEntries, ssl2_timeout, ssl3_timeout, directory, PR_FALSE);
 }
@@ -1467,7 +1467,7 @@ SSL_ConfigServerSessionIDCacheWithOpt(
                                 PRBool enableMPCache)
 {
     if (!enableMPCache) {
-        ssl_InitSessionCacheLocks(PR_FALSE);
+        ssl_InitSessionCacheLocks();
         return ssl_ConfigServerSessionIDCacheInstanceWithOpt(&globalCache, 
            ssl2_timeout, ssl3_timeout, directory, PR_FALSE,
            maxCacheEntries, maxCertCacheEntries, maxSrvNameCacheEntries);
@@ -1512,7 +1512,7 @@ SSL_InheritMPServerSIDCacheInstance(cacheDesc *cache, const char * envString)
     	return SECSuccess;	/* already done. */
     }
 
-    ssl_InitSessionCacheLocks(PR_FALSE);
+    ssl_InitSessionCacheLocks();
 
     ssl_sid_lookup  = ServerSessionIDLookup;
     ssl_sid_cache   = ServerSessionIDCache;