Plug leaks in SSL bypass code. Add freeit argument to HMAC_Destroy function.

Change existing callers to pass this argument.  Call HMAC_Destroy from SSL.
Bug 305147. r=Julien.Pierre
Modified Files:  freebl/alghmac.c freebl/alghmac.h freebl/loader.c
  freebl/loader.h freebl/tlsprfalg.c softoken/lowpbe.c softoken/pkcs11c.c
  ssl/ssl3con.c

security/nss/lib/freebl/alghmac.c

@@ -44,19 +44,24 @@
 struct HMACContextStr {
     void *hash;
     const SECHashObject *hashobj;
+    PRBool        wasAllocated;
     unsigned char ipad[HMAC_PAD_SIZE];
     unsigned char opad[HMAC_PAD_SIZE];
 };
 
 void
-HMAC_Destroy(HMACContext *cx)
+HMAC_Destroy(HMACContext *cx, PRBool freeit)
 {
     if (cx == NULL)
 	return;
 
-    if (cx->hash != NULL)
+    PORT_Assert(!freeit == !cx->wasAllocated);
+    if (cx->hash != NULL) {
 	cx->hashobj->destroy(cx->hash, PR_TRUE);
-    PORT_ZFree(cx, sizeof(HMACContext));
+	PORT_Memset(cx, 0, sizeof *cx);
+    }
+    if (freeit)
+	PORT_Free(cx);
 }
 
 SECStatus
@@ -75,8 +80,8 @@ HMAC_Init( HMACContext * cx, const SECHashObject *hash_obj,
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return SECFailure;
     }
+    cx->wasAllocated = PR_FALSE;
     cx->hashobj = hash_obj;
-
     cx->hash = cx->hashobj->create();
     if (cx->hash == NULL)
 	goto loser;
@@ -121,8 +126,9 @@ HMAC_Create(const SECHashObject *hash_obj, const unsigned char *secret,
     if (cx == NULL)
 	return NULL;
     rv = HMAC_Init(cx, hash_obj, secret, secret_len, isFIPS);
+    cx->wasAllocated = PR_TRUE;
     if (rv != SECSuccess) {
-	PORT_ZFree(cx, sizeof(HMACContext));
+	PORT_Free(cx); /* contains no secret info */
 	cx = NULL;
     }
     return cx;
@@ -171,6 +177,7 @@ HMAC_Clone(HMACContext *cx)
     if (newcx == NULL)
 	goto loser;
 
+    newcx->wasAllocated = PR_TRUE;
     newcx->hashobj = cx->hashobj;
     newcx->hash = cx->hashobj->clone(cx->hash);
     if (newcx->hash == NULL)
@@ -180,6 +187,6 @@ HMAC_Clone(HMACContext *cx)
     return newcx;
 
 loser:
-    HMAC_Destroy(newcx);
+    HMAC_Destroy(newcx, PR_TRUE);
     return NULL;
 }

security/nss/lib/freebl/alghmac.h

@@ -43,7 +43,7 @@ SEC_BEGIN_PROTOS
 
 /* destroy HMAC context */
 extern void
-HMAC_Destroy(HMACContext *cx);
+HMAC_Destroy(HMACContext *cx, PRBool freeit);
 
 /* create HMAC context
  *  hash_obj    hash object from SECRawHashObjects[]

security/nss/lib/freebl/loader.c

@@ -37,7 +37,7 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-/* $Id: loader.c,v 1.22 2005/09/07 02:47:16 saul.edwards%sun.com Exp $ */
+/* $Id: loader.c,v 1.23 2005/09/14 04:12:49 nelsonb%netscape.com Exp $ */
 
 #include "loader.h"
 #include "prmem.h"
@@ -1487,11 +1487,11 @@ HASH_GetRawHashObject(HASH_HashType hashType)
 
 
 void
-HMAC_Destroy(HMACContext *cx)
+HMAC_Destroy(HMACContext *cx, PRBool freeit)
 {
   if (!vector && PR_SUCCESS != freebl_RunLoaderOnce())
       return;
-  (vector->p_HMAC_Destroy)(cx);
+  (vector->p_HMAC_Destroy)(cx, freeit);
 }
 
 HMACContext *

security/nss/lib/freebl/loader.h

@@ -37,7 +37,7 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-/* $Id: loader.h,v 1.16 2005/09/07 02:47:16 saul.edwards%sun.com Exp $ */
+/* $Id: loader.h,v 1.17 2005/09/14 04:12:49 nelsonb%netscape.com Exp $ */
 
 #ifndef _LOADER_H_
 #define _LOADER_H_ 1
@@ -444,7 +444,7 @@ struct FREEBLVectorStr {
  SECStatus (* p_HMAC_Finish)(HMACContext *cx, unsigned char *result, 
 			     unsigned int *result_len, 
 			     unsigned int max_result_len);
- void (* p_HMAC_Destroy)(HMACContext *cx);
+ void (* p_HMAC_Destroy)(HMACContext *cx, PRBool freeit);
 
  void (* p_RNG_SystemInfoForRNG)(void);
 

security/nss/lib/freebl/tlsprfalg.c

@@ -35,7 +35,7 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-/* $Id: tlsprfalg.c,v 1.3 2005/08/09 02:54:54 nelsonb%netscape.com Exp $ */
+/* $Id: tlsprfalg.c,v 1.4 2005/09/14 04:12:49 nelsonb%netscape.com Exp $ */
 
 #include "sechash.h"
 #include "alghmac.h"
@@ -109,7 +109,8 @@ sftk_P_hash(HASH_HashType hashType, const SECItem *secret, const char *label,
 
 loser:
     /* clear out state so it's not left on the stack */
-    if (cx) HMAC_Destroy(cx);
+    if (cx) 
+    	HMAC_Destroy(cx, PR_TRUE);
     PORT_Memset(state, 0, sizeof(state));
     PORT_Memset(outbuf, 0, sizeof(outbuf));
     return rv;

security/nss/lib/softoken/lowpbe.c

@@ -249,7 +249,7 @@ nsspkcs5_PFXPBE(const SECHashObject *hashObj, NSSPKCS5PBEParameter *pbe_param,
 loser:
     if (state != NULL)
 	PORT_ZFree(state, state_len);
-    HMAC_Destroy(cx);
+    HMAC_Destroy(cx, PR_TRUE);
 
     if(rv != SECSuccess) {
 	SECITEM_ZfreeItem(ret_bits, PR_TRUE);
@@ -363,7 +363,7 @@ nsspkcs5_PBKFD2_F(const SECHashObject *hashobj, SECItem *pwitem, SECItem *salt,
     }
 loser:
     if (cx) {
-	HMAC_DestroyContext(cx);
+	HMAC_DestroyContext(cx, PR_TRUE);
     }
     if (last) {
 	PORT_ZFree(last,reaLastLength);

security/nss/lib/softoken/pkcs11c.c

@@ -121,12 +121,6 @@ sftk_FreePrivKey(NSSLOWKEYPrivateKey *key, PRBool freeit)
     nsslowkey_DestroyPrivateKey(key);
 }
 
-static void
-sftk_HMAC_Destroy(HMACContext *context, PRBool freeit)
-{
-    HMAC_Destroy(context);
-}
-
 static void
 sftk_Space(void *data, PRBool freeit)
 {
@@ -1292,7 +1286,7 @@ sftk_doHMACInit(SFTKSessionContext *context,HASH_HashType hash,
     context->hashUpdate = (SFTKHash) HMAC_Update;
     context->end = (SFTKEnd) HMAC_Finish;
 
-    context->hashdestroy = (SFTKDestroy) sftk_HMAC_Destroy;
+    context->hashdestroy = (SFTKDestroy) HMAC_Destroy;
     intpointer = (CK_ULONG *) PORT_Alloc(sizeof(CK_ULONG));
     if (intpointer == NULL) {
 	return CKR_HOST_MEMORY;

security/nss/lib/ssl/ssl3con.c

@@ -39,7 +39,7 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-/* $Id: ssl3con.c,v 1.73 2005/09/09 03:02:16 nelsonb%netscape.com Exp $ */
+/* $Id: ssl3con.c,v 1.74 2005/09/14 04:12:50 nelsonb%netscape.com Exp $ */
 
 #include "nssrenam.h"
 #include "cert.h"
@@ -1648,6 +1648,7 @@ ssl3_ComputeRecordMAC(
 		HMAC_Update(cx, temp, tempLen);
 		HMAC_Update(cx, input, inputLength);
 		rv = HMAC_Finish(cx, outbuf, outLength, spec->mac_size);
+		HMAC_Destroy(cx, PR_FALSE);
 	    }
 #undef cx
 	}