Closed
Bug 95150
Opened 23 years ago
Closed 23 years ago
PK11_DeleteTokenCertAndKey does not delete key
Categories
(NSS :: Libraries, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
3.2.2
People
(Reporter: bugz, Assigned: bugz)
Details
Attachments
(1 file)
1006 bytes,
patch
|
Details | Diff | Splinter Review |
The key is still in the database after the call.
Comment 1•23 years ago
|
||
Even when all the certs have been deleted? (if the key has more than one cert pointing to it, it won't get deleted). bob
Assignee | ||
Comment 2•23 years ago
|
||
I'm not sure about this one... What happened is that I was running QA on a build that had a bunch of printf's in it. For one, it printed out every key in the database after the call to SECKEY_OpenKeyDB. I would use certutil to delete the key, however, it showed up later on with pk12util. But I think the problem may have been that I didn't rebuild certutil, and it is statically linked. I think certutil -F (Delete Key) may take advantage of the patch I proposed for 95135, in that it is not finding the private key when the public key has a 0 in front. I'm rerunning QA now to see if I can prove this hypothesis. The bug itself, I think, is incorrect, because I verified that the key was no longer in the database in question. But I will wait for QA (I have to run the FIPS test about 15-20 times to get a failure, and that takes a while).
Assignee | ||
Comment 3•23 years ago
|
||
Okay, this is a bug. I think it is related to the other. It is only failing to delete the key when it thinks the key has a leading 0. Thus, it needs a similar workaround. I'm currently looking for where that would go, but if you know offhand could you post it here?
Comment 4•23 years ago
|
||
Somewhere in the softoken... probably from some call off of NSC_DestroyObject. bob
Assignee | ||
Comment 5•23 years ago
|
||
Assignee | ||
Comment 6•23 years ago
|
||
with this patch and the patch for 95135, I run the FIPS QA 30 times successively. Five of those runs had keys with a leading zero that took advantage of both patches. Thus the patches appear to be correct, at least functionally. The only question I had is what level the patches should be at. Is it right for them to be in the PKCS#11 layer, as I have it, or should they be in keydb.c? I decided this was right, since the key database code should be blind to what goes into it, i.e., it shouldn't know that the accessor may be a public key with a leading zero. So I think the patches are what we want. The only other question is whether similar workarounds need to happen in other parts of the code. I reviewed the entry points to keydb.c and looked at them through LXR, I think this is it. I'm ready for review then, for this and 95135.
Comment 7•23 years ago
|
||
r=wtc.
Comment 8•23 years ago
|
||
The patch looks good Ian. Yes it's at the right level. BTW, the comment may be wrong. RSA always has a leading '0', but I think it always has a leading '0', so I don't think it's the database that is stripping it off. I think that the input attribute (CKA_NETSCAPE_DB) is not getting the '0' added back again before it is used in setting the DB value. your fix is preferable to fixing the CKA_NETSCAPE_DB because we have already poluted some databases with bad key.db values. Check it in. r=relyea. bob
Assignee | ||
Comment 9•23 years ago
|
||
checked in. waiting for tomorrow's QA before resolving.
Comment 10•23 years ago
|
||
Ian, please set the target milestone and mark this bug fixed. Thanks.
Assignee: wtc → ian.mcgreer
Priority: -- → P1
Assignee | ||
Comment 11•23 years ago
|
||
fixed in 3.2.2
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.2.2
Comment 12•23 years ago
|
||
The fix is not in 3.3 but is in 3.3.1.
You need to log in
before you can comment on or make changes to this bug.
Description
•