The key is still in the database after the call.
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
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).
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?
Somewhere in the softoken... probably from some call off of NSC_DestroyObject. bob
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.
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
checked in. waiting for tomorrow's QA before resolving.
Ian, please set the target milestone and mark this bug fixed. Thanks.
fixed in 3.2.2
The fix is not in 3.3 but is in 3.3.1.