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).
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.
Created attachment 45783 [details] [diff] [review]
patch that implements same workaround
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
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.
checked in. waiting for tomorrow's QA before resolving.
Ian, please set the target milestone and mark this bug fixed.
fixed in 3.2.2
The fix is not in 3.3 but is in 3.3.1.