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

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 1

[Julien Pierre]

Attachment: always call NSS_NoDB_Init

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.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 3

[Julien Pierre]

Attachment: don't take away working -d option. Only call NSS_NoDB_Init if -d is not supplied

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.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

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+ ).

Comment 5

[Julien Pierre]

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.

Comment 6

[Robert Relyea]

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

Comment 7

[Julien Pierre]

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.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 9

[Julien Pierre]

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.