Closed Bug 232604 Opened 16 years ago Closed 15 years ago

shlibsign tries to open DBs in $HOME/.netscape, even on windows

Categories

(NSS :: Tools, defect, P2)

x86
Windows 2000
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nelson, Assigned: julien.pierre)

Details

Attachments

(1 file, 1 obsolete file)

Background:

The shlibsign program always tries to init NSS DBs, and if that fails, 
it calls NSS_NoDB_Init().  It takes an optional -d <directory> argument, 
to tell it where to look for the DBs, but the makefile does not provide
a -d argument when it invokes shlibsign to sign the shared libs.  
In the absense of a -d argument, shlibsign tries to open the DBs in 
$HOME/.netscape, on all platforms.  This is particularly bad for the QA
progams, which ought to produce the same results regardless of which 
user runs them and what that user has in his/her $HOME/.netscape directory.

The bug:

Shlibsign should not be attempting to open DBs in $HOME/.netscape.

I don't know whether there is EVER any case where shlibsign needs any
certs or user's private keys, but if not, then shlibsign should not
use any DBs.

Proposed solutions:

a) change the makefile to invoke shlib sign with a -d argument that
points to an always non-existent directory, or 

b) change shlibsign to always call NSS_NoDB_Init, and never attempt to 
open DBs.
QA Contact: bishakhabanerjee → jason.m.reid
Attached patch always call NSS_NoDB_Init (obsolete) — Splinter Review
The -d option for shlibsign never worked, as you can see in this patch. SECU_ConfigDirectory was invoked first with the argument value, then again with a NULL value in the following line :

    rv = NSS_Init(SECU_ConfigDirectory(NULL));

This patch removes the -d option which couldn't have been used successfully.
Assignee: wtchang → julien.pierre.bugs
Status: NEW → ASSIGNED
Attachment #202446 - Flags: review?(nelson)
Comment on attachment 202446 [details] [diff] [review]
always call NSS_NoDB_Init

I agree this patch is the right fix.  The content of the DB files 
in the directory named by the -d option was never used in any way, 
AFAICT.  But I disagree with the assertion that the -d option never 
worked, and the implication that SECU_ConfigDirectory was being 
used improperly.  I'll send email about that.
Attachment #202446 - Flags: review?(nelson) → review+
Thanks for the review, Nelson. I didn't fully realize the behavior of SECU_ConfigDirectory with multiple calls. It wasn't my intention to take away an option that worked, even if our scripts didn't use it. So, I have supplied this new patch.
Attachment #202446 - Attachment is obsolete: true
Attachment #202562 - Flags: review?(nelson)
Well, AFAIK, shlibsign doesn't use any certs from the cert DB at all, nor 
any keys from the key DB.  So, whether it finds a cert DB or not really 
makes no difference.  The -d option leads the user to believe that the 
choice of cert DB makes some difference to the program's behavior, when
AFAIK, it does not.  So, I think it is best to not offer the -d option
and to always use NODB_Init.  

Bob, if you think there is any reason to retain the -d option for shlibsign,
please speak up here and tell us what the cert DB is actually used for 
(or intended to be used for).  Otherwise, I think Julien should proceed 
with his first patch above (which I already marked r+ ).
Even though no certs are used, shlibsign does a DSA keygen, and attempts to find the best slot for that operation. One might want to do a keygen in a hardware device. The -d option allows for a secmod.db with such a device configured, so it has some value.
I think it's fine to leave out the -d function. If we want allow other tokens to be used, we can add back the -d as well as a -t function to specify the token.

bob
Bob,

-t would only be needed if one wanted to do the keygen in the non-default token for DSA. secmod.db already has a preference for DSA, which would indicate the token. The -d option and modutil already give you the ability to choose the token for the keygen.

I would rather not take away this capability, so I prefer the second patch to the first one.
Priority: -- → P2
Target Milestone: --- → 3.11
Comment on attachment 202562 [details] [diff] [review]
don't take away working -d option. Only call NSS_NoDB_Init if -d is not supplied

This patch does not follow the prevailing coding style of this file, which is the "K&R" style.  
I'll give this patch r+ provided that the style of braces/indentation is changed to match the rest of the source file.
Attachment #202562 - Flags: review?(nelson) → review+
Thanks, Nelson. I checked in the second patch after changing the braces . 

Checking in shlibsign.c;
/cvsroot/mozilla/security/nss/cmd/shlibsign/shlibsign.c,v  <--  shlibsign.c
new revision: 1.14; previous revision: 1.13
done

Marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.