Closed
Bug 254393
Opened 20 years ago
Closed 20 years ago
PK11_FindKeyByAnyCert always returns a key object even if there is no key
Categories
(NSS :: Libraries, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
3.9.3
People
(Reporter: julien.pierre, Assigned: julien.pierre)
Details
Attachments
(1 file, 1 obsolete file)
1.43 KB,
patch
|
Details | Diff | Splinter Review |
Our appserver won't start with NSS 3.9.2 RTM ; but it started sucessfully with our 3.9.2 beta2 snapshot taken on 6/18/2004 . I have narrowed down the regression to the following check-in in pk11cert.c, which is unfortunately quite large. To make things harder, a single check-in was made for two bugs. revision 1.132 date: 2004/06/21 23:01:52; author: relyea%netscape.com; state: Exp; lines: +101 -79 Bug 244914, 244907 r=nelsonb Add support for unprotected private keys without requiring authentication. Add support to access application specific PKCS #11 objects through NSS. The problem occurs with SEC_PKCS12AddCertAndKey failing. I traced this and the failure was in adding the key object. The private key object had a null key type . I will post more info with a detailed stack later. An NFS server just went down and I can't post it now ...
Assignee | ||
Comment 1•20 years ago
|
||
The error returned is SEC_ERROR_PKCS12_UNABLE_TO_EXPORT_KEY . Here is the deepest stack where the error happens. =>[1] pk11_private_key_encrypt_buffer_length(key = 0x629458), line 5084 in "pk11skey.c" [2] PK11_ExportEncryptedPrivKeyInfo(slot = 0xdd128, algTag = SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC, pwitem = 0xffbfc544, pk = 0x629458, iteration = 1, wincx = 0xffbfc704), line 5196 in "pk11skey.c" [3] PK11_ExportEncryptedPrivateKeyInfo(slot = 0xdd128, algTag = SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC, pwitem = 0xffbfc544, cert = 0x609c40, iteration = 1, wincx = 0xffbfc704), line 5290 in "pk11skey.c" [4] SEC_PKCS12AddKeyForCert(p12ctxt = 0x627038, safe = 0x6270b0, nestedDest = (nil), cert = 0x609c40, shroudKey = 1, algorithm = SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC, pwitem = 0x647500, keyId = 0x211e3c, nickName = (nil)), line 1312 in "p12e.c" [5] SEC_PKCS12AddCertAndKey(p12ctxt = 0x627038, certSafe = 0x627108, certNestedDest = (nil), cert = 0x609c40, certDb = 0xdb228, keySafe = 0x6270b0, keyNestedDest = (nil), shroudKey = 1, pwitem = 0x647500, algorithm = SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC), line 1525 in "p12e.c" At that point, we have : (.../dist/share/forte_dev,v6.2/SUNWspro/bin/../WS6U2/bin/sparcv9/dbx) p *key *key = { arena = 0x210a68 keyType = nullKey pkcs11Slot = 0xdd128 pkcs11ID = 0 pkcs11IsTemp = 1 wincx = 0xffbfc704 The pkcs11ID is invalid, and so is the keyType . I found that the regression is in the function PK11_FindKeyByAnyCert . With Bob's patch, it always returns a pointer to a SECKEYPrivateKey* object, even if the CK_OBJECT_HANDLE is 0. This is incorrect. A NULL pointer should be returned. This breaks the app server because at the beginning, it enumerates all the private keys for all the certs. And later it tries to back them up to a PKCS#12 file, but this fails because the keys don't actually exist. By fixing this function, the fictitious keys are no longer found, and the application only backs up the keys that are actually there to the PKCS#12 file, and thus succeeds. Patch fortcoming.
Summary: Regression in pk11cert.c → PK11_FindKeyByAnyCert always returns a key object even if there is no key
Assignee | ||
Comment 2•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #155318 -
Flags: superreview?(nelson)
Attachment #155318 -
Flags: review?(bugz)
Comment 3•20 years ago
|
||
Comment on attachment 155318 [details] [diff] [review] don't return a private key object if CK_OBJECT_HANDLE is invalid >+ if (keyHandle == CK_INVALID_HANDLE) { >+ goto loser; > } > privKey = PK11_MakePrivKey(slot, nullKey, PR_TRUE, keyHandle, wincx); > loser: I think I'd prefer that to be: if (keyHandle != CK_INVALID_HANDLE) { privKey = PK11_MakePrivKey(slot, nullKey, PR_TRUE, keyHandle, wincx); } loser: But either way, r=nelson
Attachment #155318 -
Flags: superreview?(nelson)
Attachment #155318 -
Flags: superreview?(bugz)
Attachment #155318 -
Flags: review?(bugz)
Attachment #155318 -
Flags: review+
Assignee | ||
Comment 4•20 years ago
|
||
Nelson, Thanks for the review and the suggestion. I have checked in a variant on it that allows the elimination of the evil goto statements. NSS_3_9_BRANCH : Checking in pk11cert.c; /cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cert.c,v <-- pk11cert.c new revision: 1.128.4.4; previous revision: 1.128.4.3 done And tip : Checking in pk11cert.c; /cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cert.c,v <-- pk11cert.c new revision: 1.137; previous revision: 1.136 done
Assignee | ||
Updated•20 years ago
|
Attachment #155318 -
Flags: superreview?(bugz)
Assignee | ||
Updated•20 years ago
|
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•20 years ago
|
Priority: -- → P1
Target Milestone: --- → 3.9.3
You need to log in
before you can comment on or make changes to this bug.
Description
•