Bug 272484 Certificate manager crashes [@ _PR_MD_ATOMIC_DECREMENT - PK11_FreeSymKey]

r=wtc [part 3 of 3]
This commit is contained in:
relyea%netscape.com 2005-10-03 22:01:57 +00:00
parent 0cefb4acd8
commit 5c3685a18e
8 changed files with 73 additions and 102 deletions

View File

@ -110,7 +110,6 @@ struct SEC_PKCS12DecoderContextStr {
/* state variables for decoding authenticated safes. */ /* state variables for decoding authenticated safes. */
SEC_PKCS7DecoderContext *currentASafeP7Dcx; SEC_PKCS7DecoderContext *currentASafeP7Dcx;
SEC_PKCS5KeyAndPassword *currentASafeKeyPwd;
SEC_ASN1DecoderContext *aSafeDcx; SEC_ASN1DecoderContext *aSafeDcx;
SEC_PKCS7DecoderContext *aSafeP7Dcx; SEC_PKCS7DecoderContext *aSafeP7Dcx;
sec_PKCS12AuthenticatedSafe authSafe; sec_PKCS12AuthenticatedSafe authSafe;
@ -173,25 +172,43 @@ sec_pkcs12_proper_version(sec_PKCS12PFXItem *pfx)
static PK11SymKey * static PK11SymKey *
sec_pkcs12_decoder_get_decrypt_key(void *arg, SECAlgorithmID *algid) sec_pkcs12_decoder_get_decrypt_key(void *arg, SECAlgorithmID *algid)
{ {
SEC_PKCS5KeyAndPassword *keyPwd = SEC_PKCS12DecoderContext *p12dcx =
(SEC_PKCS5KeyAndPassword *)arg; (SEC_PKCS12DecoderContext *) arg;
PK11SlotInfo *slot;
PK11SymKey *bulkKey;
if(!keyPwd) { if(!p12dcx) {
return NULL; return NULL;
} }
/* if no slot specified, use the internal key slot */ /* if no slot specified, use the internal key slot */
if(!keyPwd->slot) { if(p12dcx->slot) {
keyPwd->slot = PK11_GetInternalKeySlot(); slot = PK11_ReferenceSlot(p12dcx->slot);
} else {
slot = PK11_GetInternalKeySlot();
} }
/* retrieve the key */ bulkKey = PK11_PBEKeyGen(slot, algid, p12dcx->pwitem,
if(!keyPwd->key) { PR_FALSE, p12dcx->wincx);
keyPwd->key = PK11_PBEKeyGen(keyPwd->slot, algid, /* some tokens can't generate PBE keys on their own, generate the
keyPwd->pwitem, PR_FALSE, keyPwd->wincx); * key in the internal slot, and let the Import code deal with it,
* (if the slot can't generate PBEs, then we need to use the internal
* slot anyway to unwrap). */
if (!bulkKey && !PK11_IsInternal(slot)) {
PK11_FreeSlot(slot);
slot = PK11_GetInternalKeySlot();
bulkKey = PK11_PBEKeyGen(slot, algid, p12dcx->pwitem,
PR_FALSE, p12dcx->wincx);
}
PK11_FreeSlot(slot);
/* set the password data on the key */
if (bulkKey) {
PK11_SetSymKeyUserData(bulkKey,p12dcx->pwitem, NULL);
} }
return (PK11SymKey *)keyPwd;
return bulkKey;
} }
/* XXX this needs to be modified to handle enveloped data. most /* XXX this needs to be modified to handle enveloped data. most
@ -778,25 +795,12 @@ sec_pkcs12_decoder_asafes_notify(void *arg, PRBool before, void *dest,
goto loser; goto loser;
} }
/* set up password and encryption key information */
p12dcx->currentASafeKeyPwd =
(SEC_PKCS5KeyAndPassword*)PORT_ArenaZAlloc(p12dcx->arena,
sizeof(SEC_PKCS5KeyAndPassword));
if(!p12dcx->currentASafeKeyPwd) {
p12dcx->errorValue = SEC_ERROR_NO_MEMORY;
goto loser;
}
p12dcx->currentASafeKeyPwd->pwitem = p12dcx->pwitem;
p12dcx->currentASafeKeyPwd->slot = p12dcx->slot;
p12dcx->currentASafeKeyPwd->wincx = p12dcx->wincx;
/* initiate the PKCS7ContentInfo decode */ /* initiate the PKCS7ContentInfo decode */
p12dcx->currentASafeP7Dcx = SEC_PKCS7DecoderStart( p12dcx->currentASafeP7Dcx = SEC_PKCS7DecoderStart(
sec_pkcs12_decoder_safe_contents_callback, sec_pkcs12_decoder_safe_contents_callback,
safeContentsCtx, safeContentsCtx,
p12dcx->pwfn, p12dcx->pwfnarg, p12dcx->pwfn, p12dcx->pwfnarg,
sec_pkcs12_decoder_get_decrypt_key, sec_pkcs12_decoder_get_decrypt_key, p12dcx,
p12dcx->currentASafeKeyPwd,
sec_pkcs12_decoder_decryption_allowed); sec_pkcs12_decoder_decryption_allowed);
if(!p12dcx->currentASafeP7Dcx) { if(!p12dcx->currentASafeP7Dcx) {
p12dcx->errorValue = PORT_GetError(); p12dcx->errorValue = PORT_GetError();
@ -818,9 +822,6 @@ sec_pkcs12_decoder_asafes_notify(void *arg, PRBool before, void *dest,
p12dcx->currentASafeP7Dcx = NULL; p12dcx->currentASafeP7Dcx = NULL;
} }
p12dcx->currentASafeP7Dcx = NULL; p12dcx->currentASafeP7Dcx = NULL;
if(p12dcx->currentASafeKeyPwd->key != NULL) {
p12dcx->currentASafeKeyPwd->key = NULL;
}
} }

View File

@ -1901,10 +1901,9 @@ sec_pkcs12_encoder_asafe_process(sec_PKCS12EncoderContext *p12ecx)
{ {
SEC_PKCS7EncoderContext *innerP7ecx; SEC_PKCS7EncoderContext *innerP7ecx;
SEC_PKCS7ContentInfo *cinfo; SEC_PKCS7ContentInfo *cinfo;
void *arg = NULL; PK11SymKey *bulkKey = NULL;
SEC_ASN1EncoderContext *innerA1ecx = NULL; SEC_ASN1EncoderContext *innerA1ecx = NULL;
SECStatus rv = SECSuccess; SECStatus rv = SECSuccess;
SEC_PKCS5KeyAndPassword keyPwd;
if(p12ecx->currentSafe < p12ecx->p12exp->authSafe.safeCount) { if(p12ecx->currentSafe < p12ecx->p12exp->authSafe.safeCount) {
SEC_PKCS12SafeInfo *safeInfo; SEC_PKCS12SafeInfo *safeInfo;
@ -1923,15 +1922,11 @@ sec_pkcs12_encoder_asafe_process(sec_PKCS12EncoderContext *p12ecx)
/* determine the safe type and set the appropriate argument */ /* determine the safe type and set the appropriate argument */
switch(cinfoType) { switch(cinfoType) {
case SEC_OID_PKCS7_DATA: case SEC_OID_PKCS7_DATA:
arg = NULL; case SEC_OID_PKCS7_ENVELOPED_DATA:
break; break;
case SEC_OID_PKCS7_ENCRYPTED_DATA: case SEC_OID_PKCS7_ENCRYPTED_DATA:
keyPwd.pwitem = &safeInfo->pwitem; bulkKey = safeInfo->encryptionKey;
keyPwd.key = safeInfo->encryptionKey; PK11_SetSymKeyUserData(bulkKey, &safeInfo->pwitem, NULL);
arg = &keyPwd;
break;
case SEC_OID_PKCS7_ENVELOPED_DATA:
arg = NULL;
break; break;
default: default:
return SECFailure; return SECFailure;
@ -1941,7 +1936,7 @@ sec_pkcs12_encoder_asafe_process(sec_PKCS12EncoderContext *p12ecx)
/* start the PKCS7 encoder */ /* start the PKCS7 encoder */
innerP7ecx = SEC_PKCS7EncoderStart(cinfo, innerP7ecx = SEC_PKCS7EncoderStart(cinfo,
sec_P12P7OutputCB_CallA1Update, sec_P12P7OutputCB_CallA1Update,
p12ecx->middleA1ecx, (PK11SymKey *)arg); p12ecx->middleA1ecx, bulkKey);
if(!innerP7ecx) { if(!innerP7ecx) {
goto loser; goto loser;
} }

View File

@ -38,7 +38,7 @@
/* /*
* PKCS7 decoding, verification. * PKCS7 decoding, verification.
* *
* $Id: p7decode.c,v 1.19 2005/09/02 01:24:56 wtchang%redhat.com Exp $ * $Id: p7decode.c,v 1.20 2005/10/03 22:01:56 relyea%netscape.com Exp $
*/ */
#include "nssrenam.h" #include "nssrenam.h"
@ -691,16 +691,6 @@ sec_pkcs7_decoder_start_decrypt (SEC_PKCS7DecoderContext *p7dcx, int depth,
decryptobj = sec_PKCS7CreateDecryptObject (bulkkey, decryptobj = sec_PKCS7CreateDecryptObject (bulkkey,
&(enccinfo->contentEncAlg)); &(enccinfo->contentEncAlg));
/*
* For PKCS5 Encryption Algorithms, the bulkkey is actually a different
* structure. Therefore, we need to set the bulkkey to the actual key
* prior to freeing it.
*/
if ( SEC_PKCS5IsAlgorithmPBEAlg(&(enccinfo->contentEncAlg)) && bulkkey ) {
SEC_PKCS5KeyAndPassword *keyPwd = (SEC_PKCS5KeyAndPassword *)bulkkey;
bulkkey = keyPwd->key;
}
/* /*
* We are done with (this) bulkkey now. * We are done with (this) bulkkey now.
*/ */

View File

@ -40,7 +40,7 @@
* encoding/creation side *and* the decoding/decryption side. Anything * encoding/creation side *and* the decoding/decryption side. Anything
* else should be static routines in the appropriate file. * else should be static routines in the appropriate file.
* *
* $Id: p7local.c,v 1.7 2004/04/25 15:03:13 gerv%gerv.net Exp $ * $Id: p7local.c,v 1.8 2005/10/03 22:01:56 relyea%netscape.com Exp $
*/ */
#include "p7local.h" #include "p7local.h"
@ -118,11 +118,12 @@ sec_PKCS7CreateDecryptObject (PK11SymKey *key, SECAlgorithmID *algid)
if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) { if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) {
CK_MECHANISM pbeMech, cryptoMech; CK_MECHANISM pbeMech, cryptoMech;
SECItem *pbeParams, *pwitem; SECItem *pbeParams, *pwitem;
SEC_PKCS5KeyAndPassword *keyPwd;
keyPwd = (SEC_PKCS5KeyAndPassword *)key; pwitem = (SECItem *)PK11_GetSymKeyUserData(key);
key = keyPwd->key; if (!pwitem) {
pwitem = keyPwd->pwitem; PORT_Free(result);
return NULL;
}
pbeMech.mechanism = PK11_AlgtagToMechanism(algtag); pbeMech.mechanism = PK11_AlgtagToMechanism(algtag);
pbeParams = PK11_ParamFromAlgid(algid); pbeParams = PK11_ParamFromAlgid(algid);
@ -211,25 +212,28 @@ sec_PKCS7CreateEncryptObject (PRArenaPool *poolp, PK11SymKey *key,
ciphercx = NULL; ciphercx = NULL;
if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) { if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) {
CK_MECHANISM pbeMech, cryptoMech; CK_MECHANISM pbeMech, cryptoMech;
SECItem *pbeParams; SECItem *pbeParams, *pwitem;
SEC_PKCS5KeyAndPassword *keyPwd;
PORT_Memset(&pbeMech, 0, sizeof(CK_MECHANISM)); PORT_Memset(&pbeMech, 0, sizeof(CK_MECHANISM));
PORT_Memset(&cryptoMech, 0, sizeof(CK_MECHANISM)); PORT_Memset(&cryptoMech, 0, sizeof(CK_MECHANISM));
pwitem = (SECItem *)PK11_GetSymKeyUserData(key);
if (!pwitem) {
PORT_Free(result);
return NULL;
}
pbeMech.mechanism = PK11_AlgtagToMechanism(algtag); pbeMech.mechanism = PK11_AlgtagToMechanism(algtag);
pbeParams = PK11_ParamFromAlgid(algid); pbeParams = PK11_ParamFromAlgid(algid);
if(!pbeParams) { if(!pbeParams) {
PORT_Free(result); PORT_Free(result);
return NULL; return NULL;
} }
keyPwd = (SEC_PKCS5KeyAndPassword *)key;
key = keyPwd->key;
pbeMech.pParameter = pbeParams->data; pbeMech.pParameter = pbeParams->data;
pbeMech.ulParameterLen = pbeParams->len; pbeMech.ulParameterLen = pbeParams->len;
if(PK11_MapPBEMechanismToCryptoMechanism(&pbeMech, &cryptoMech, if(PK11_MapPBEMechanismToCryptoMechanism(&pbeMech, &cryptoMech, pwitem,
keyPwd->pwitem, PR_FALSE) != CKR_OK) { PR_FALSE) != CKR_OK) {
PORT_Free(result); PORT_Free(result);
SECITEM_ZfreeItem(pbeParams, PR_TRUE); SECITEM_ZfreeItem(pbeParams, PR_TRUE);
return NULL; return NULL;

View File

@ -37,7 +37,7 @@
/* /*
* Encryption/decryption routines for CMS implementation, none of which are exported. * Encryption/decryption routines for CMS implementation, none of which are exported.
* *
* $Id: cmscipher.c,v 1.7 2004/04/25 15:03:16 gerv%gerv.net Exp $ * $Id: cmscipher.c,v 1.8 2005/10/03 22:01:57 relyea%netscape.com Exp $
*/ */
#include "cmslocal.h" #include "cmslocal.h"
@ -93,17 +93,14 @@ NSS_CMSCipherContext_StartDecrypt(PK11SymKey *key, SECAlgorithmID *algid)
/* set param and mechanism */ /* set param and mechanism */
if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) { if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) {
CK_MECHANISM pbeMech, cryptoMech; CK_MECHANISM pbeMech, cryptoMech;
SECItem *pbeParams; SECItem *pbeParams, *pwitem;
SEC_PKCS5KeyAndPassword *keyPwd;
PORT_Memset(&pbeMech, 0, sizeof(CK_MECHANISM)); PORT_Memset(&pbeMech, 0, sizeof(CK_MECHANISM));
PORT_Memset(&cryptoMech, 0, sizeof(CK_MECHANISM)); PORT_Memset(&cryptoMech, 0, sizeof(CK_MECHANISM));
/* HACK ALERT! pwitem = PK11_GetSymKeyUserData(key);
* in this case, key is not actually a PK11SymKey *, but a SEC_PKCS5KeyAndPassword * if (!pwitem)
*/ return NULL;
keyPwd = (SEC_PKCS5KeyAndPassword *)key;
key = keyPwd->key;
/* find correct PK11 mechanism and parameters to initialize pbeMech */ /* find correct PK11 mechanism and parameters to initialize pbeMech */
pbeMech.mechanism = PK11_AlgtagToMechanism(algtag); pbeMech.mechanism = PK11_AlgtagToMechanism(algtag);
@ -114,7 +111,7 @@ NSS_CMSCipherContext_StartDecrypt(PK11SymKey *key, SECAlgorithmID *algid)
pbeMech.ulParameterLen = pbeParams->len; pbeMech.ulParameterLen = pbeParams->len;
/* now map pbeMech to cryptoMech */ /* now map pbeMech to cryptoMech */
if (PK11_MapPBEMechanismToCryptoMechanism(&pbeMech, &cryptoMech, keyPwd->pwitem, if (PK11_MapPBEMechanismToCryptoMechanism(&pbeMech, &cryptoMech, pwitem,
PR_FALSE) != CKR_OK) { PR_FALSE) != CKR_OK) {
SECITEM_ZfreeItem(pbeParams, PR_TRUE); SECITEM_ZfreeItem(pbeParams, PR_TRUE);
return NULL; return NULL;
@ -187,17 +184,14 @@ NSS_CMSCipherContext_StartEncrypt(PRArenaPool *poolp, PK11SymKey *key, SECAlgori
/* set param and mechanism */ /* set param and mechanism */
if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) { if (SEC_PKCS5IsAlgorithmPBEAlg(algid)) {
CK_MECHANISM pbeMech, cryptoMech; CK_MECHANISM pbeMech, cryptoMech;
SECItem *pbeParams; SECItem *pbeParams, *pwitem;
SEC_PKCS5KeyAndPassword *keyPwd;
PORT_Memset(&pbeMech, 0, sizeof(CK_MECHANISM)); PORT_Memset(&pbeMech, 0, sizeof(CK_MECHANISM));
PORT_Memset(&cryptoMech, 0, sizeof(CK_MECHANISM)); PORT_Memset(&cryptoMech, 0, sizeof(CK_MECHANISM));
/* HACK ALERT! pwitem = PK11_GetSymKeyUserData(key);
* in this case, key is not actually a PK11SymKey *, but a SEC_PKCS5KeyAndPassword * if (!pwitem)
*/ return NULL;
keyPwd = (SEC_PKCS5KeyAndPassword *)key;
key = keyPwd->key;
/* find correct PK11 mechanism and parameters to initialize pbeMech */ /* find correct PK11 mechanism and parameters to initialize pbeMech */
pbeMech.mechanism = PK11_AlgtagToMechanism(algtag); pbeMech.mechanism = PK11_AlgtagToMechanism(algtag);
@ -208,7 +202,7 @@ NSS_CMSCipherContext_StartEncrypt(PRArenaPool *poolp, PK11SymKey *key, SECAlgori
pbeMech.ulParameterLen = pbeParams->len; pbeMech.ulParameterLen = pbeParams->len;
/* now map pbeMech to cryptoMech */ /* now map pbeMech to cryptoMech */
if (PK11_MapPBEMechanismToCryptoMechanism(&pbeMech, &cryptoMech, keyPwd->pwitem, if (PK11_MapPBEMechanismToCryptoMechanism(&pbeMech, &cryptoMech, pwitem,
PR_FALSE) != CKR_OK) { PR_FALSE) != CKR_OK) {
SECITEM_ZfreeItem(pbeParams, PR_TRUE); SECITEM_ZfreeItem(pbeParams, PR_TRUE);
return NULL; return NULL;

View File

@ -37,7 +37,7 @@
/* /*
* CMS encryptedData methods. * CMS encryptedData methods.
* *
* $Id: cmsencdata.c,v 1.7 2004/04/25 15:03:16 gerv%gerv.net Exp $ * $Id: cmsencdata.c,v 1.8 2005/10/03 22:01:57 relyea%netscape.com Exp $
*/ */
#include "cmslocal.h" #include "cmslocal.h"
@ -245,16 +245,6 @@ NSS_CMSEncryptedData_Decode_BeforeData(NSSCMSEncryptedData *encd)
if (cinfo->ciphcx == NULL) if (cinfo->ciphcx == NULL)
goto loser; /* error has been set by NSS_CMSCipherContext_StartDecrypt */ goto loser; /* error has been set by NSS_CMSCipherContext_StartDecrypt */
/*
* HACK ALERT!!
* For PKCS5 Encryption Algorithms, the bulkkey is actually a different
* structure. Therefore, we need to set the bulkkey to the actual key
* prior to freeing it.
*/
if (SEC_PKCS5IsAlgorithmPBEAlg(bulkalg)) {
SEC_PKCS5KeyAndPassword *keyPwd = (SEC_PKCS5KeyAndPassword *)bulkkey;
bulkkey = keyPwd->key;
}
/* we are done with (this) bulkkey now. */ /* we are done with (this) bulkkey now. */
PK11_FreeSymKey(bulkkey); PK11_FreeSymKey(bulkkey);

View File

@ -37,7 +37,7 @@
/* /*
* CMS envelopedData methods. * CMS envelopedData methods.
* *
* $Id: cmsenvdata.c,v 1.10 2004/04/25 15:03:16 gerv%gerv.net Exp $ * $Id: cmsenvdata.c,v 1.11 2005/10/03 22:01:57 relyea%netscape.com Exp $
*/ */
#include "cmslocal.h" #include "cmslocal.h"
@ -384,16 +384,6 @@ NSS_CMSEnvelopedData_Decode_BeforeData(NSSCMSEnvelopedData *envd)
if (cinfo->ciphcx == NULL) if (cinfo->ciphcx == NULL)
goto loser; /* error has been set by NSS_CMSCipherContext_StartDecrypt */ goto loser; /* error has been set by NSS_CMSCipherContext_StartDecrypt */
/*
* HACK ALERT!!
* For PKCS5 Encryption Algorithms, the bulkkey is actually a different
* structure. Therefore, we need to set the bulkkey to the actual key
* prior to freeing it.
*/
if (SEC_PKCS5IsAlgorithmPBEAlg(bulkalg)) {
SEC_PKCS5KeyAndPassword *keyPwd = (SEC_PKCS5KeyAndPassword *)bulkkey;
bulkkey = keyPwd->key;
}
rv = SECSuccess; rv = SECSuccess;

View File

@ -37,7 +37,7 @@
/* /*
* CMS recipientInfo methods. * CMS recipientInfo methods.
* *
* $Id: cmsrecinfo.c,v 1.15 2005/09/16 17:52:37 wtchang%redhat.com Exp $ * $Id: cmsrecinfo.c,v 1.16 2005/10/03 22:01:57 relyea%netscape.com Exp $
*/ */
#include "cmslocal.h" #include "cmslocal.h"
@ -63,8 +63,15 @@ nss_cmsrecipientinfo_usessubjectkeyid(NSSCMSRecipientInfo *ri)
return PR_FALSE; return PR_FALSE;
} }
/*
static SECOidData fakeContent = { 0 }; * NOTE: fakeContent marks CMSMessage structure which is only used as a carrier
* of pwfn_arg and arena pools. In an ideal world, NSSCMSMessage would not have
* been exported, and we would have added an ordinary enum to handle this
* check. Unfortunatly wo don't have that luxury so we are overloading the
* contentTypeTag field. NO code should every try to interpret this content tag
* as a real OID tag, or use any fields other than pwfn_arg or poolp of this
* CMSMessage for that matter */
static const SECOidData fakeContent;
NSSCMSRecipientInfo * NSSCMSRecipientInfo *
nss_cmsrecipientinfo_create(NSSCMSMessage *cmsg, NSSCMSRecipientIDSelector type, nss_cmsrecipientinfo_create(NSSCMSMessage *cmsg, NSSCMSRecipientIDSelector type,
CERTCertificate *cert, SECKEYPublicKey *pubKey, CERTCertificate *cert, SECKEYPublicKey *pubKey,