Last Comment Bug 95150 - PK11_DeleteTokenCertAndKey does not delete key
: PK11_DeleteTokenCertAndKey does not delete key
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.2.1
: All All
P1 blocker (vote)
: 3.2.2
Assigned To: Ian McGreer
: Sonja Mirtitsch
Depends on:
  Show dependency treegraph
Reported: 2001-08-13 14:12 PDT by Ian McGreer
Modified: 2001-11-08 17:20 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch that implements same workaround (1006 bytes, patch)
2001-08-14 08:01 PDT, Ian McGreer
no flags Details | Diff | Splinter Review

Description User image Ian McGreer 2001-08-13 14:12:42 PDT
The key is still in the database after the call.
Comment 1 User image Robert Relyea 2001-08-13 14:28:16 PDT
Even when all the certs have been deleted? (if the key has more than one cert
pointing to it, it won't get deleted).

Comment 2 User image Ian McGreer 2001-08-13 14:37:22 PDT
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).
Comment 3 User image Ian McGreer 2001-08-13 14:46:38 PDT
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 User image Robert Relyea 2001-08-13 14:54:59 PDT
Somewhere in the softoken... probably from some call off of NSC_DestroyObject.

Comment 5 User image Ian McGreer 2001-08-14 08:01:17 PDT
Created attachment 45783 [details] [diff] [review]
patch that implements same workaround
Comment 6 User image Ian McGreer 2001-08-14 08:08:18 PDT
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.
Comment 7 User image Wan-Teh Chang 2001-08-14 11:53:14 PDT
Comment 8 User image Robert Relyea 2001-08-14 13:05:53 PDT
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.

Comment 9 User image Ian McGreer 2001-08-14 13:28:37 PDT
checked in.  waiting for tomorrow's QA before resolving.
Comment 10 User image Wan-Teh Chang 2001-10-31 19:14:04 PST
Ian, please set the target milestone and mark this bug fixed.
Comment 11 User image Ian McGreer 2001-11-01 07:46:26 PST
fixed in 3.2.2
Comment 12 User image Wan-Teh Chang 2001-11-08 17:20:59 PST
The fix is not in 3.3 but is in 3.3.1.

Note You need to log in before you can comment on or make changes to this bug.