Closed Bug 67068 Opened 24 years ago Closed 24 years ago

Need a SECMOD_Shutdown

Categories

(NSS :: Libraries, defect)

All
Windows NT
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: javi, Assigned: rrelyea)

References

Details

Attachments

(4 files)

This is related to Bug 66939. Client embedding groups are supporting switching profiles without exiting the application. NSS 3.2 uses a loadable PKCS11 module for its list of trusted roots. When shutting down NSS, that shared library is not unloaded and then the next time NSS is initialized, the PKCS11 libraries fail to initialize because the root certs module was never un-loaded. PSM bundles the PKCS11 module in the same directory as the executable because we can't guess the user profile directory before run-time, so every profile that gets run will be pointed to the same shared library. I worked around this by calling SECMOD_DeleteModule for the loadable root certs module before shutting down NSS. In order to make smart cards useable with such products in the future, we need to have NSS shut down the PKCS11 libraries when we shut down NSS.
I should add that implementing SECMOD_Shutdown and then calling it from within NSS_Shutdown would solve this problem as well.
I just did some more investigation and realized PSM 2.0 will need this in NSS3.2 What happens is that the SECMOD libraries keep a global modules variable around. When you try to re-initialize NSS, the modules variable is already defined so the new secmod.db is not read/created. This will mean any embedding app that uses PSM 2.0 will only use the first semod.db that was loaded for all profiles that get launced. That may be OK, but is that we want?
I can work on a SECMOD_Shutdown, but 1) I'm not sure we can support a full nss shutdown and restart in the 3.2 time frame (Mostly because NSS has never been designed to shutdown and restart. This is a Feature request that needs to be started *BEFORE* we ship betas). 3.2 RTM's in 2 weeks. 2) You definately won't be able to run two different profiles at the same time. That is a Stan feature which requires a line in a PRD and a major release to fulfill.
We don't want to run 2 profiles at the same time. We need to shut one down, and the initialize another one. cert and key db's are currently working on the Mac, it's just secmod that can't handle another initialization.
Right, we certainly don't need two profiles at the same time. In terms of the time frame, note that this blocks bug 64833. I'm sorry about the short notice, but at some point in the past month or so, this was working with profile switching and the problem showed up farily recently.
Attached patch Implement NSS_Shutdown() (deleted) — Splinter Review
wtc can you review the patch. javi. can you try the patch in psm and see if it does in fact allow NSS to be called and initialized again. bob
It'll be a day or so before I can get around to applying and testing this patch.
Bob, your patch is good. By the way, the code in SECMOD_Shutdown seems to be indented with a tab (or 8 spaces).
Oops. That's now fixed. BTW, This code assumes that NSS calls have quiessed.... that is if you have an open encryption or SSL session still running unexpected behavior may result. Nelson, is it necessary to flush the SSL cache as well (I think it's safest if you do, and after shutdown nothing in the cashe will be usable as the empheral wrapping key will have disappeared when the internal token gets closed.
Nelson, Bob has a question for you in this bug report.
Yes, The client must call SSL_ClearSessionCache() any time it changes the SSL configuration (e.g. the enabled ciphersuites, or versions of SSL/TLS, or if it wants to shutdown NSS.
OK, SDR wasn't happy last night. Turns out there were a couple of potential problems: 1) NSS_Shutdown can be called in the middle of NSS_Init if the db's don't initialize. SECMOD_Init is now tolerant of being called even if the security module hasn't been initialized. 2) The keys on the slot free list do not have a slot value in them (since we've freed the reference. Everything handled this case except when we freed up the slot, when we go to destroy all the keys were were trying to use symkey->slot which doesn't exit (this one was the real crash in sdrtest). Also NOTE: sdrtest does '-d' does not work. I have a separate patch for this which I'll file under a separate bug.
Instead of eliminating the call to SECU_ConfigDirectory in case 'd', why not change the NSS_Init call to < rv = NSS_Init(certDir); > rv = NSS_Init(SECU_ConfigDirectory(NULL));
I also wanted to do the strdup of the parameters as well. With the new Initialize sequence the SetConfigDirectory stuff really isn't needed anymore.
Attached patch Third times a charm... (deleted) — Splinter Review
The stack trace in the core file generated by sdr.sh is: (dbx on Solaris 2.6) detected a multithreaded program dbx: program is not active t@1 (l@1) terminated by signal SEGV (no mapping at the fault address) Current function is SECMOD_DestroyListLock 73 PK11_USE_THREADS(PZ_DestroyMonitor(lock->monitor);) (dbx) print lock lock = (nil) (dbx) where current thread: t@1 =>[1] SECMOD_DestroyListLock(lock = (nil)), line 73 in "pk11list.c" [2] SECMOD_Shutdown(), line 179 in "pk11util.c" [3] NSS_Shutdown(), line 292 in "nssinit.c" [4] nss_Init(configdir = 0x2b508 ".", certPrefix = 0xef675ea8 "", keyPrefix = 0xef675eac "", secmodName = 0xef675eb0 "secmod.db", readOnly = 1, nodb = 0), line 237 in "nssinit.c" [5] NSS_Init(configdir = 0x2b508 "."), line 244 in "nssinit.c" [6] main(argc = 7, argv = 0xeffff78c), line 163 in "sdrtest.c" (dbx) up Current function is SECMOD_Shutdown 179 SECMOD_DestroyListLock(moduleLock); (dbx) print moduleLock moduleLock = (nil) (dbx) list 179 SECMOD_DestroyListLock(moduleLock); 180 moduleLock = NULL; 181 /* free the internal module */ 182 SECMOD_DestroyModule(internalModule); 183 internalModule = NULL; 184 /* destroy the list */ 185 SECMOD_DestroyModuleList(modules); 186 modules = NULL; 187 188 /* make all the slots and the lists go away */ (dbx) This particular crash is fixed by Bob's change to pk11util.c. Bob, your changes to pk11util.c and pk11slot.c are fine. You can check them in to fix the crash in our test suite. I don't understand your change to pk11skey.c. You might want to have Terry or Nelson review it.
SECU_ConfigDirectory makes a copy of the input arguments, obviating the strdup. And it handled the logic of defaulting to $HOME in the absense of a -d option. Most of our commands do this. Should our commands be consistent about this?
I probably requires looking at more context, so I'll put verbage here to help whoever reviews it, BTW it's the change the is necessary if sdrtest is actually ran with a real database. (Wah-Teh's traceback is a case where the database wasn't available and NSS_Init call NSS_Shutdown before SECMOD_Init is called -- thus the null pointers for the lock. The change is in the PK11_CleanKeyList() function. This function is only called when you destroy a Slot (which only happens 1) at shutdown or 2) when you delete a module). The KeyList is a cache of keys we build up as your system runs so that we aren't pounding malloc all the time. Each slot has his own list of key structures that get recycled. One of the expenses of creating a key is creating a PKCS#11 session object with the key. The cache reuses these sessions as well. Now when the key for is put on this list, the key's pointer to the slot is release and the zero'ed. (PK11_FreeSymKey()). When the key is restored, that slot pointer is restored (PK11_NewSymKey()). When we destroy a slot structure, all the 'empty' key structures that have been cached in that slot are freed, and the PKCS #11 sessions associated with them are closed. It's this close step that was trying to dereference symKey->slot for a key on this list. Fortunately we know what slot this session belongs to -- it's the slot we are trying to free, so we use that pointer to tell the PKCS #11 code what slot and session to close. bob
OK, Bob, after reading your explanations a few times I finally got it. Patch reviewed.
Mozilla 0.8 builds started today. We would consider taking reviewed low risk fixes if they are available today (Wednesday, 7/Feb/01) or tomorrow (Thursday, 8/Feb/01). Otherwise, please set the target milestone to Mozilla 0.9. Thanks.
This will not make it into 0.8 since it would require picking up a new version of NSS that has not been officially released yet. The current implementation will not allow you to open 2 Security Module databases on the Mac iff the user(s) switch profiles. Embedding projects will not care about this limitation unless they want to allow different users to use different sets of smart cards on the Mac. I
Setting target milestone to NSS 3.2
Target Milestone: --- → 3.2
Javi, I'm going to close this bug now. NSS_Shutdown() seems to work in the contexts that we currently call it. If you have any problems with PSM use then open a new bug on that problem.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
*** Bug 69470 has been marked as a duplicate of this bug. ***
No longer blocks: 64833
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: