crash/assertion failure on shutdown after installing ECC cert

RESOLVED FIXED in 3.11.1

Status

NSS
Libraries
P1
major
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: kaie, Assigned: Alexei Volkov)

Tracking

(Depends on: 1 bug)

3.11
3.11.1
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

13 years ago
I am using Mozilla with patch from bug 326159 applied
with NSS_3_11_BRANCH and patch from bug 326503 applied.

This bug occurs after executing CRMF code.

Due to bug 328909, I have disabled PSM's code that adds a Proof-Of-Possession to CRMF requests.

Here is what I did to reproduce the problem:

- produce a CRMF request using http://kuix.de/misc/test3/crmf-ecc.php
- submitted the request to ECC enabled CS
- logged in to ECC with existing agent cert (client auth) and issued the cert
- fetched the issued cert
- looked at the cert with cert manager
- quit Mozilla

During shutdown, Mozilla aborts and crashes with this assertion:

Assertion failure: certHandle->ref == 1 || slot->slotID > FIPS_SLOT_ID, at pkcs11.c:2769

Stack:

#5  0x00002aaaab2469c6 in PR_Assert (s=Could not find the frame base for "PR_Assert".
) at /home/kaie/moz/18-ecc/mozilla/nsprpub/pr/src/io/prlog.c:538
#6  0x00002aaab3a8312c in sftk_DBShutdown (slot=0x110fef0) at pkcs11.c:2769
#7  0x00002aaab3a831e4 in SFTK_ShutdownSlot (slot=0x110fef0) at pkcs11.c:2803
#8  0x00002aaab3a83200 in SFTK_DestroySlotData (slot=0x110fef0) at pkcs11.c:2815
#9  0x00002aaab3a836da in nscFreeAllSlots (moduleIndex=0) at pkcs11.c:2930
#10 0x00002aaab3a83aa2 in nsc_CommonFinalize (pReserved=0x0, isFIPS=0) at pkcs11.c:3077
#11 0x00002aaab3a83b38 in NSC_Finalize (pReserved=0x0) at pkcs11.c:3123
#12 0x00002aaab38c7262 in SECMOD_UnloadModule (mod=0x10ff830) at pk11load.c:403
#13 0x00002aaab38dff26 in SECMOD_SlotDestroyModule (module=0x10ff830, fromSlot=1) at pk11util.c:808
#14 0x00002aaab38db1d1 in PK11_DestroySlot (slot=0x111aed0) at pk11slot.c:446
#15 0x00002aaab38db202 in PK11_FreeSlot (slot=0x111aed0) at pk11slot.c:459
#16 0x00002aaab38dfe3b in SECMOD_DestroyModule (module=0x10ff830) at pk11util.c:779
#17 0x00002aaab38dff8c in SECMOD_DestroyModuleListElement (element=0x10ff520) at pk11util.c:825
#18 0x00002aaab38dffc6 in SECMOD_DestroyModuleList (list=0x10ff520) at pk11util.c:841
#19 0x00002aaab38dee14 in SECMOD_Shutdown () at pk11util.c:99
#20 0x00002aaab38a1657 in NSS_Shutdown () at nssinit.c:610
#21 0x00002aaab348280c in nsNSSComponent::ShutdownNSS (this=0x10fa9f0)
    at /home/kaie/moz/18-ecc/mozilla/security/manager/ssl/src/nsNSSComponent.cpp:1527
#22 0x00002aaab3485f18 in nsNSSComponent::Observe (this=0x10fa9f0, aSubject=0x6ea5b0,
    aTopic=0x2aaab096fdf3 "profile-before-change", someData=0x2aaab096fd60)
    at /home/kaie/moz/18-ecc/mozilla/security/manager/ssl/src/nsNSSComponent.cpp:1839
#23 0x00002aaaaac09c6a in nsObserverService::NotifyObservers (this=0x5fc930, aSubject=0x6ea5b0,
    aTopic=0x2aaab096fdf3 "profile-before-change", someData=0x2aaab096fd60)
    at /home/kaie/moz/18-ecc/mozilla/xpcom/ds/nsObserverService.cpp:233
#24 0x00002aaab095859e in nsProfile::ShutDownCurrentProfile (this=0x6ea5b0, shutDownType=1)
    at /home/kaie/moz/18-ecc/mozilla/profile/src/nsProfile.cpp:1354
#25 0x000000000040373f in DoOnShutdown () at /home/kaie/moz/18-ecc/mozilla/xpfe/bootstrap/nsAppRunner.cpp:726
#26 0x0000000000407317 in main (argc=3, argv=0x7fffffa10858) at /home/kaie/moz/18-ecc/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1754
(Reporter)

Comment 1

13 years ago
FWIW, I retested with the new patch from bug 326503 applied, and I still see this crash.
Kai, IINM, this is the thing I asked about yesterday in 
https://bugzilla.mozilla.org/show_bug.cgi?id=245420#c17 

How do I reconcile this bug report with your answer in 
https://bugzilla.mozilla.org/show_bug.cgi?id=245420#c18 ?
(Reporter)

Comment 3

13 years ago
I saw this bug only after I added the comment to bug 245420.
Key generation alone is not sufficient to trigger this crash on shutdown.
To see the crash it is necessary to exeucute the additional activity I list in this bug.
(Reporter)

Comment 4

13 years ago
I just did additional testing.

Neither key generation nor CRMF request submitting nor client auth is responsible for the crash.

I am able to trigger the crash by using a fresh session, with a cert database that contains the new private key, but not yet the certificate. Fetching the cert from the web page and existing afterwards triggers the crash.
(Reporter)

Comment 5

13 years ago
FYI:
  certHandle->ref is 2
  slot->slotID is 2
(Reporter)

Comment 6

13 years ago
updating summary
Summary: crash/assertion failure on shutdown after using ECC CRMF → crash/assertion failure on shutdown after installing ECC cert
Kai's comment 4 suggests that the problem is entirely in the downloading and
installing of the new cert, and not in the CRMF cert request.  

Now, I wonder.  What is the form of the cert download that leads to this 
failure?  Is it a simple binary cert?  or perhaps PEM encoded?  
Or is it a CMMF format download?  
(Reporter)

Comment 8

13 years ago
I think it's best if I give you a full testcase.

I started with a fresh cert database and tried to reproduce again, so I can give out my cert db files.

To my surprise, everything works fine when importing the first ecc cert. No crash on exit.

Only if I repeat the process, that is, use a cert database having one cert already, and try to import a second cert, I see the crash on exit.

I will attach a zip file that contains the cert database files, after generating the second key pair, after having sent the second cert request, but before installing the second cert. Password is "test".

I will attach the certificate in the form provided by the CA and imported into Mozilla over the web. It is a DER encoded cert, it gets downloaded using Content-Type: application/x-x509-user-cert

If you would like to see what code PSM executes to import the cert, look at nsNSSCertificateDB::ImportUserCertificate
(Reporter)

Comment 9

13 years ago
Created attachment 213748 [details]
Zip file containing cert database files
(Reporter)

Comment 10

13 years ago
Created attachment 213749 [details]
certificate
(Reporter)

Comment 11

13 years ago
To reproduce the crash:
- extract attached cert db files into profile dir
- start mozilla
- click certificate attachment, enter password test
- exit, crash
(Reporter)

Comment 12

13 years ago
Created attachment 213750 [details]
Text dump of certificate
Alexei, you're so good at this!  Here's another one.  :-)
Although this bug was originally reproduced with FireFox, 
I think we can try to reproduce it with certutil.  
Even if we have to reproduce it with FireFox, it shouldn't
be necessary to rebuild all of firefox to do the test.
Call me if you have any questions.
Assignee: wtchang → alexei.volkov.bugs
Priority: -- → P1
Target Milestone: --- → 3.11.1
(Reporter)

Comment 14

13 years ago
I initially reproduced the problem with SeaMonkey, but I confirm I can reproduce with Firefox, too.

I tried certutil, but it did not crash on exit.
On Friday, Alexei IMed me with the news that he had reproduced the problem
with certutil using the files Kai supplied.  That's good news.  I predict
he will nail this problem shortly.  Thanks for those files, Kai!
(Assignee)

Comment 16

13 years ago
Created attachment 214247 [details] [diff] [review]
mem leak fix

Memory leak in function AddPermSubjectNode(softtoken/pcertdb.c:3060). Leaking temporary cmpcert in the loop that sort cert keys.
Attachment #214247 - Flags: superreview?(rrelyea)
Attachment #214247 - Flags: review?(nelson)

Comment 17

13 years ago
Comment on attachment 214247 [details] [diff] [review]
mem leak fix

r+ rrelyea. Good detective work alexi.
Attachment #214247 - Flags: superreview?(rrelyea) → superreview+
Created attachment 214288 [details] [diff] [review]
Bigger patch for trunk, cleanup function, eliminate dup'ed code

When I went to review Alexei's patch (above), 
I reviewed the entire function which was changed, and
saw lots of duplicated code.  I found it a little hard
to follow, so I cleaned it up, eliminating duplicated code.
Alexei, Please review.

I suspect we will want the simpler/smaller patch for the 
3.11 branch, and the larger one for the trunk.
Attachment #214288 - Flags: review?(alexei.volkov.bugs)
Comment on attachment 214247 [details] [diff] [review]
mem leak fix

r=nelson for 3.11 branch.
Good work on that find!
Attachment #214247 - Flags: review?(nelson) → review+
Attachment #214288 - Flags: review?(rrelyea)
(Assignee)

Comment 20

13 years ago
/cvsroot/mozilla/security/nss/lib/softoken/pcertdb.c,v  <--  pcertdb.c
new revision: 1.53.2.3; previous revision: 1.53.2.2

Comment 21

13 years ago
Comment on attachment 214288 [details] [diff] [review]
Bigger patch for trunk, cleanup function, eliminate dup'ed code

r+ = relyea.
Attachment #214288 - Flags: review?(rrelyea) → review+
(Assignee)

Updated

13 years ago
Attachment #214288 - Flags: review?(alexei.volkov.bugs) → review+
checked in bigger patch on trunk.
Checking in pcertdb.c;  new revision: 1.57; previous revision: 1.56
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.