Use unsigned char consistently instead of an odd uint8. Make

ssl_GetSessionTicketKeysPKCS11 use the same locking pattern as
ssl_GetSessionTicketKeys.
Modified Files:
 Tag: NSS_RFC4507BIS_BRANCH
	ssl3ext.c sslimpl.h sslsnce.c

3 files changed, 144 insertions, 128 deletions

diff --git a/security/nss/lib/ssl/ssl3ext.c b/security/nss/lib/ssl/ssl3ext.c
index 6bd29c29b..2ecbdbc76 100644
--- a/security/nss/lib/ssl/ssl3ext.c
+++ b/security/nss/lib/ssl/ssl3ext.c
@@ -51,7 +51,7 @@
 #include "blapi.h"
 #include "prinit.h"
 
-static uint8          key_name[SESS_TICKET_KEY_NAME_LEN];
+static unsigned char  key_name[SESS_TICKET_KEY_NAME_LEN];
 static PK11SymKey    *session_ticket_enc_key_pkcs11 = NULL;
 static PK11SymKey    *session_ticket_mac_key_pkcs11 = NULL;
 
diff --git a/security/nss/lib/ssl/sslimpl.h b/security/nss/lib/ssl/sslimpl.h
index c2c52717e..0a0f9f25f 100644
--- a/security/nss/lib/ssl/sslimpl.h
+++ b/security/nss/lib/ssl/sslimpl.h
@@ -1480,8 +1480,8 @@ extern PRBool ssl3_ExtensionNegotiated(sslSocket *ss, PRUint16 ex_type);
 extern SECStatus ssl3_SetSIDSessionTicket(sslSessionID *sid,
 			NewSessionTicket *session_ticket);
 extern SECStatus ssl3_SendNewSessionTicket(sslSocket *ss);
-extern PRBool ssl_GetSessionTicketKeys(uint8 *key_name, unsigned char *encKey,
-			unsigned char *macKey);
+extern PRBool ssl_GetSessionTicketKeys(unsigned char *keyName,
+			unsigned char *encKey, unsigned char *macKey);
 extern PRBool ssl_GetSessionTicketKeysPKCS11(SECKEYPrivateKey *svrPrivKey,
 			SECKEYPublicKey *svrPubKey, void *pwArg,
 			unsigned char *keyName, PK11SymKey **aesKey,
diff --git a/security/nss/lib/ssl/sslsnce.c b/security/nss/lib/ssl/sslsnce.c
index 0d69bebd9..5e32b4f2a 100644
--- a/security/nss/lib/ssl/sslsnce.c
+++ b/security/nss/lib/ssl/sslsnce.c
@@ -1636,106 +1636,43 @@ ssl_GetWrappingKey( PRInt32                   symWrapMechIndex,
     return rv;
 }
 
-PRBool
-ssl_GetSessionTicketKeys(uint8 *keyName, unsigned char *encKey,
-                         unsigned char *macKey)
+/* Wrap and cache a session ticket key. */
+static PRBool
+WrapTicketKey(SECKEYPublicKey *svrPubKey, PK11SymKey *symKey,
+              const char *keyName, encKeyCacheEntry* cacheEntry)
 {
-    PRBool rv = PR_FALSE;
-    PRUint32  now = 0;
-    cacheDesc *cache = &globalCache;
-
-    /* Grab lock. */
-    now = LockSidCacheLock(cache->keyCacheLock, now);
-    if (!now)
-	return rv;
-
-    if (!*(cache->ticketKeysValid)) {
-	if (PK11_GenerateRandom(cache->ticketKeyNameSuffix,
-		SESS_TICKET_KEY_VAR_NAME_LEN) != SECSuccess)
-	    goto loser;
-	if (PK11_GenerateRandom(cache->ticketEncKey->bytes, 32) != SECSuccess)
-	    goto loser;
-	if (PK11_GenerateRandom(cache->ticketMacKey->bytes,
-		SHA256_LENGTH) != SECSuccess)
-	    goto loser;
-	*(cache->ticketKeysValid) = 1;
-    }
+    SECItem wrappedKey = {siBuffer, NULL, 0};
 
-    rv = PR_TRUE;
+    wrappedKey.len = SECKEY_PublicKeyStrength(svrPubKey);
+    PORT_Assert(wrappedKey.len <= sizeof(cacheEntry->bytes));
+    if (wrappedKey.len > sizeof(cacheEntry->bytes))
+	return PR_FALSE;
+    wrappedKey.data = cacheEntry->bytes;
 
- loser:
-    UnlockSidCacheLock(cache->keyCacheLock);
-    if (rv) {
-	PORT_Memcpy(keyName, cache->ticketKeyNameSuffix,
-	    SESS_TICKET_KEY_VAR_NAME_LEN);
-	PORT_Memcpy(encKey, cache->ticketEncKey->bytes, 32);
-	PORT_Memcpy(macKey, cache->ticketMacKey->bytes, SHA256_LENGTH);
+    if (PK11_PubWrapSymKey(CKM_RSA_PKCS, svrPubKey, symKey, &wrappedKey)
+	    != SECSuccess) {
+	SSL_DBG(("%d: SSL[%s]: Unable to wrap session ticket %s.",
+		    SSL_GETPID(), "unknown", keyName));
+	return PR_FALSE;
     }
-    return rv;
+    cacheEntry->length = wrappedKey.len;
+    return PR_TRUE;
 }
 
-PRBool
-ssl_GetSessionTicketKeysPKCS11(SECKEYPrivateKey *svrPrivKey,
-                               SECKEYPublicKey *svrPubKey, void *pwArg,
-                               unsigned char *keyName, PK11SymKey **aesKey,
-                               PK11SymKey **macKey)
+static PRBool
+GenerateAndWrapTicketKeys(SECKEYPublicKey *svrPubKey, void *pwArg,
+                          unsigned char *keyName, PK11SymKey **aesKey,
+                          PK11SymKey **macKey)
 {
     PK11SlotInfo *slot;
-    PRUint32 now = 0;
     CK_MECHANISM_TYPE mechanismArray[2];
     PK11SymKey *aesKeyTmp = NULL;
     PK11SymKey *macKeyTmp = NULL;
-    SECItem wrappedAesKey = {siBuffer, NULL, 0};
-    SECItem wrappedMacKey = {siBuffer, NULL, 0};
-    PRBool rv = PR_FALSE;
     cacheDesc *cache = &globalCache;
 
-    /* No need to grab a lock, we are reading. */
-    if (*(cache->ticketKeysValid)) {
-	wrappedAesKey.type = siBuffer;
-	wrappedAesKey.data = cache->ticketEncKey->bytes;
-	wrappedAesKey.len = cache->ticketEncKey->length;
-	PORT_Assert(wrappedAesKey.len <= sizeof(cache->ticketEncKey->bytes));
-	aesKeyTmp = PK11_PubUnwrapSymKey(svrPrivKey, &wrappedAesKey,
-	    CKM_AES_CBC, CKA_DECRYPT, 0);
-
-	wrappedMacKey.type = siBuffer;
-	wrappedMacKey.data = cache->ticketMacKey->bytes;
-	wrappedMacKey.len = cache->ticketMacKey->length;
-	PORT_Assert(wrappedMacKey.len <= sizeof(cache->ticketMacKey->bytes));
-	macKeyTmp = PK11_PubUnwrapSymKey(svrPrivKey, &wrappedMacKey,
-	    CKM_SHA256_HMAC, CKA_SIGN, 0);
-
-	if (aesKeyTmp == NULL || macKeyTmp == NULL) {
-	    if (aesKeyTmp)
-		PK11_FreeSymKey(aesKeyTmp);
-	    if (macKeyTmp)
-		PK11_FreeSymKey(macKeyTmp);
-	    SSL_DBG(("%d: SSL[%s]: Unable to unwrap session ticket keys.",
-			SSL_GETPID(), "unknown"));
-	    return PR_FALSE;
-	}
-
-	PORT_Memcpy(keyName, cache->ticketKeyNameSuffix,
-	    SESS_TICKET_KEY_VAR_NAME_LEN);
-	*aesKey = aesKeyTmp;
-	*macKey = macKeyTmp;
-	SSL_DBG(("%d: SSL[%s]: Successfully unwrapped session ticket keys.",
-		    SSL_GETPID(), "unknown"));
-	return PR_TRUE;
-    }
-
-    /* Keys do not exist, create them. */
-    now = LockSidCacheLock(cache->keyCacheLock, now);
-    if (!now) {
-	SSL_DBG(("%d: SSL[%s]: Unable to grab keyCacheLock.",
-		    SSL_GETPID(), "unknown"));
-	return PR_FALSE;
-    }
-
     if (PK11_GenerateRandom(cache->ticketKeyNameSuffix,
 	    SESS_TICKET_KEY_VAR_NAME_LEN) != SECSuccess) {
-	SSL_DBG(("%d: SSL[%s]: Unable to generate random key_name bytes.",
+	SSL_DBG(("%d: SSL[%s]: Unable to generate random key name bytes.",
 		    SSL_GETPID(), "unknown"));
 	goto loser;
     }
@@ -1744,9 +1681,12 @@ ssl_GetSessionTicketKeysPKCS11(SECKEYPrivateKey *svrPrivKey,
     mechanismArray[1] = CKM_SHA256_HMAC;
 
     slot = PK11_GetBestSlotMultiple(mechanismArray, 2, pwArg);
-    aesKeyTmp = PK11_KeyGen(slot, mechanismArray[0], NULL, 32, pwArg);
-    macKeyTmp = PK11_KeyGen(slot, mechanismArray[1], NULL, SHA256_LENGTH, pwArg);
-    PK11_FreeSlot(slot);
+    if (slot) {
+	aesKeyTmp = PK11_KeyGen(slot, mechanismArray[0], NULL, 32, pwArg);
+	macKeyTmp = PK11_KeyGen(slot, mechanismArray[1], NULL, SHA256_LENGTH,
+				pwArg);
+	PK11_FreeSlot(slot);
+    }
 
     if (aesKeyTmp == NULL || macKeyTmp == NULL) {
 	SSL_DBG(("%d: SSL[%s]: Unable to generate session ticket keys.",
@@ -1755,59 +1695,135 @@ ssl_GetSessionTicketKeysPKCS11(SECKEYPrivateKey *svrPrivKey,
     }
 
     /* Export the keys to the shared cache in wrapped form. */
-    wrappedAesKey.len = SECKEY_PublicKeyStrength(svrPubKey);
-    PORT_Assert(wrappedAesKey.len <= sizeof(cache->ticketEncKey->bytes));
-    if (wrappedAesKey.len > sizeof(cache->ticketEncKey->bytes))
+    if (!WrapTicketKey(svrPubKey, aesKeyTmp, "enc key", cache->ticketEncKey))
 	goto loser;
-    wrappedAesKey.data = (unsigned char*)PORT_Alloc(wrappedAesKey.len);
-
-    if (PK11_PubWrapSymKey(CKM_RSA_PKCS, svrPubKey,
-	    aesKeyTmp, &wrappedAesKey) != SECSuccess) {
-	SSL_DBG(("%d: SSL[%s]: Unable to wrap session ticket enc key.",
-		    SSL_GETPID(), "unknown"));
+    if (!WrapTicketKey(svrPubKey, macKeyTmp, "mac key", cache->ticketMacKey))
 	goto loser;
-    }
 
-    wrappedMacKey.len  = SECKEY_PublicKeyStrength(svrPubKey);
-    PORT_Assert(wrappedMacKey.len <= sizeof(cache->ticketMacKey->bytes));
-    if (wrappedMacKey.len > sizeof(cache->ticketMacKey->bytes))
-	goto loser;
-    wrappedMacKey.data = (unsigned char*)PORT_Alloc(wrappedMacKey.len);
+    PORT_Memcpy(keyName, cache->ticketKeyNameSuffix,
+	SESS_TICKET_KEY_VAR_NAME_LEN);
+    *aesKey = aesKeyTmp;
+    *macKey = macKeyTmp;
+    return PR_TRUE;
+
+loser:
+    if (aesKeyTmp)
+	PK11_FreeSymKey(aesKeyTmp);
+    if (macKeyTmp)
+	PK11_FreeSymKey(macKeyTmp);
+    return PR_FALSE;
+}
+
+static PRBool
+UnwrapCachedTicketKeys(SECKEYPrivateKey *svrPrivKey, unsigned char *keyName,
+                       PK11SymKey **aesKey, PK11SymKey **macKey)
+{
+    SECItem wrappedKey = {siBuffer, NULL, 0};
+    PK11SymKey *aesKeyTmp = NULL;
+    PK11SymKey *macKeyTmp = NULL;
+    cacheDesc *cache = &globalCache;
+
+    wrappedKey.data = cache->ticketEncKey->bytes;
+    wrappedKey.len = cache->ticketEncKey->length;
+    PORT_Assert(wrappedKey.len <= sizeof(cache->ticketEncKey->bytes));
+    aesKeyTmp = PK11_PubUnwrapSymKey(svrPrivKey, &wrappedKey,
+	CKM_AES_CBC, CKA_DECRYPT, 0);
 
-    if (PK11_PubWrapSymKey(CKM_RSA_PKCS, svrPubKey,
-	    macKeyTmp, &wrappedMacKey) != SECSuccess) {
-	SSL_DBG(("%d: SSL[%s]: Unable to wrap session ticket mac key.",
+    wrappedKey.data = cache->ticketMacKey->bytes;
+    wrappedKey.len = cache->ticketMacKey->length;
+    PORT_Assert(wrappedKey.len <= sizeof(cache->ticketMacKey->bytes));
+    macKeyTmp = PK11_PubUnwrapSymKey(svrPrivKey, &wrappedKey,
+	CKM_SHA256_HMAC, CKA_SIGN, 0);
+
+    if (aesKeyTmp == NULL || macKeyTmp == NULL) {
+	SSL_DBG(("%d: SSL[%s]: Unable to unwrap session ticket keys.",
 		    SSL_GETPID(), "unknown"));
 	goto loser;
     }
-
-    cache->ticketEncKey->length = wrappedAesKey.len;
-    cache->ticketMacKey->length = wrappedMacKey.len;
-
-    PORT_Memcpy(cache->ticketEncKey->bytes, wrappedAesKey.data,
-	wrappedAesKey.len);
-    PORT_Memcpy(cache->ticketMacKey->bytes, wrappedMacKey.data,
-	wrappedMacKey.len);
-    *(cache->ticketKeysValid) = 1;
+    SSL_DBG(("%d: SSL[%s]: Successfully unwrapped session ticket keys.",
+		SSL_GETPID(), "unknown"));
 
     PORT_Memcpy(keyName, cache->ticketKeyNameSuffix,
 	SESS_TICKET_KEY_VAR_NAME_LEN);
     *aesKey = aesKeyTmp;
     *macKey = macKeyTmp;
+    return PR_TRUE;
+
+loser:
+    if (aesKeyTmp)
+	PK11_FreeSymKey(aesKeyTmp);
+    if (macKeyTmp)
+	PK11_FreeSymKey(macKeyTmp);
+    return PR_FALSE;
+}
+
+PRBool
+ssl_GetSessionTicketKeysPKCS11(SECKEYPrivateKey *svrPrivKey,
+                               SECKEYPublicKey *svrPubKey, void *pwArg,
+                               unsigned char *keyName, PK11SymKey **aesKey,
+                               PK11SymKey **macKey)
+{
+    PRUint32 now = 0;
+    PRBool rv = PR_FALSE;
+    PRBool keysGenerated = PR_FALSE;
+    cacheDesc *cache = &globalCache;
+
+    now = LockSidCacheLock(cache->keyCacheLock, now);
+    if (!now)
+	return rv;
+
+    if (!*(cache->ticketKeysValid)) {
+	/* Keys do not exist, create them. */
+	if (!GenerateAndWrapTicketKeys(svrPubKey, pwArg, keyName,
+		aesKey, macKey))
+	    goto loser;
+	keysGenerated = PR_TRUE;
+	*(cache->ticketKeysValid) = 1;
+    }
+
+    rv = PR_TRUE;
+
+ loser:
+    UnlockSidCacheLock(cache->keyCacheLock);
+    if (rv && !keysGenerated)
+	rv = UnwrapCachedTicketKeys(svrPrivKey, keyName, aesKey, macKey);
+    return rv;
+}
+
+PRBool
+ssl_GetSessionTicketKeys(unsigned char *keyName, unsigned char *encKey,
+                         unsigned char *macKey)
+{
+    PRBool rv = PR_FALSE;
+    PRUint32 now = 0;
+    cacheDesc *cache = &globalCache;
+
+    /* Grab lock. */
+    now = LockSidCacheLock(cache->keyCacheLock, now);
+    if (!now)
+	return rv;
+
+    if (!*(cache->ticketKeysValid)) {
+	if (PK11_GenerateRandom(cache->ticketKeyNameSuffix,
+		SESS_TICKET_KEY_VAR_NAME_LEN) != SECSuccess)
+	    goto loser;
+	if (PK11_GenerateRandom(cache->ticketEncKey->bytes, 32) != SECSuccess)
+	    goto loser;
+	if (PK11_GenerateRandom(cache->ticketMacKey->bytes,
+		SHA256_LENGTH) != SECSuccess)
+	    goto loser;
+	*(cache->ticketKeysValid) = 1;
+    }
+
     rv = PR_TRUE;
 
  loser:
     UnlockSidCacheLock(cache->keyCacheLock);
-    if (wrappedAesKey.data)
-	SECITEM_FreeItem(&wrappedAesKey, PR_FALSE);
-    if (wrappedMacKey.data)
-	SECITEM_FreeItem(&wrappedMacKey, PR_FALSE);
-
-    if (!rv) {
-	if (aesKeyTmp)
-	    PK11_FreeSymKey(aesKeyTmp);
-	if (macKeyTmp)
-	    PK11_FreeSymKey(macKeyTmp);
+    if (rv) {
+	PORT_Memcpy(keyName, cache->ticketKeyNameSuffix,
+	    SESS_TICKET_KEY_VAR_NAME_LEN);
+	PORT_Memcpy(encKey, cache->ticketEncKey->bytes, 32);
+	PORT_Memcpy(macKey, cache->ticketMacKey->bytes, SHA256_LENGTH);
     }
     return rv;
 }