From 9499265f5cf3f6f4d22d45a56c96977d703f74f0 Mon Sep 17 00:00:00 2001 From: "nelsonb%netscape.com" Date: Wed, 14 Sep 2005 04:12:50 +0000 Subject: [PATCH] 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 | 19 +++++++++++++------ security/nss/lib/freebl/alghmac.h | 2 +- security/nss/lib/freebl/loader.c | 6 +++--- security/nss/lib/freebl/loader.h | 4 ++-- security/nss/lib/freebl/tlsprfalg.c | 5 +++-- security/nss/lib/softoken/lowpbe.c | 4 ++-- security/nss/lib/softoken/pkcs11c.c | 8 +------- security/nss/lib/ssl/ssl3con.c | 3 ++- 8 files changed, 27 insertions(+), 24 deletions(-) diff --git a/security/nss/lib/freebl/alghmac.c b/security/nss/lib/freebl/alghmac.c index d0c57901bc19..5f09548e8693 100644 --- a/security/nss/lib/freebl/alghmac.c +++ b/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; } diff --git a/security/nss/lib/freebl/alghmac.h b/security/nss/lib/freebl/alghmac.h index 3af1975a3505..81c5bfa50d14 100644 --- a/security/nss/lib/freebl/alghmac.h +++ b/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[] diff --git a/security/nss/lib/freebl/loader.c b/security/nss/lib/freebl/loader.c index f6de5d91e7b3..53f2a02bc981 100644 --- a/security/nss/lib/freebl/loader.c +++ b/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 * diff --git a/security/nss/lib/freebl/loader.h b/security/nss/lib/freebl/loader.h index 5ac87a29cdc0..7f7e7ef558b3 100644 --- a/security/nss/lib/freebl/loader.h +++ b/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); diff --git a/security/nss/lib/freebl/tlsprfalg.c b/security/nss/lib/freebl/tlsprfalg.c index e1ba4fc2d4f8..e250ef9394e0 100644 --- a/security/nss/lib/freebl/tlsprfalg.c +++ b/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; diff --git a/security/nss/lib/softoken/lowpbe.c b/security/nss/lib/softoken/lowpbe.c index 8f173b271345..81a0cb06bb9e 100644 --- a/security/nss/lib/softoken/lowpbe.c +++ b/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); diff --git a/security/nss/lib/softoken/pkcs11c.c b/security/nss/lib/softoken/pkcs11c.c index 10bbdbd9b27e..56eb1814cee2 100644 --- a/security/nss/lib/softoken/pkcs11c.c +++ b/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; diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c index 03527faa54b5..d355b4f16417 100644 --- a/security/nss/lib/ssl/ssl3con.c +++ b/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 }