183385 - eliminate references to NSS key/cert/module files

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect, P1)

Description

[Mark Smith [:mcs]]

There are a few places where the LDAP C SDK code references NSS key,
certificate, or module database files by name. We need to eliminate all such
references so that changes made by NSS do not affect the LDAP C SDK. Some
occurrences I know about (will look for more):

mozilla/directory/c-sdk/ldap/libraries/libssldap/clientinit.c (.db and secmod.db)
mozilla/directory/c-sdk/ldap/clients/tools/common.c (see certpath2keypath())

Comment 1

[Mark Smith [:mcs]]

I don't see an easy way to avoid using "secmod.db" as a default name
(NSS_Initialize() does not accept NULL). And because of the way the
ldapssl_.*_init() APIs were originally defined, they need to know enough to
split a full key/cert DB path into a directory and prefix. But I fixed as much
as I can for now. Patch coming.

Comment 2

[Mark Smith [:mcs]]

Attachment: proposed fix

Most of the changes are de-linting (I made the mistake of running lint because
I was looking for a specific problem, and then felt compelled to fix many
things it complained about).

There real changes are threefold:

1) In ldap/libraries/libssldap/clientinit.c:ldapssl_pkcs_init(), the secmod.db
name (-m option to ldapsearch et al) was ignored. Now it is retrieved and used.


2) In ldap/client/tools/common.c, I eliminated a function named
certpath2keypath() that constructed a key DB path from a cert DB path. It isn't
really needed, since all NSS needs to know is the overall path and the prefix
(which is computed inside the libssldap code without relying on the fact that
key3.db is in the name). Now if no key DB path is given by the user but a cert
DB path is, the code in common.c just passes the cert DB path for the key DB
path.

3) The ldapsearch and ldapcmp tools now exit( LDAP_NO_MEMORY ) if an LDIF
fragment can't be constructed. Previously such failures were ignored.

Comment 3

[Mark Smith [:mcs]]

Attachment: improved fix: added comment

Sorry. Same as the last diff, except I added this comment in
ldap/libraries/libssldap/clientinit.c above ldapssl_basic_init():

/*
 * Note: by design, the keydbpath can actually be a certdbpath.  Some
 * callers rely on this behavior, e.g., the LDAP command line tools.
 * This function simply does not care whether the paths end in the
 * correct NSS filenames or not; the mission here is just to extract
 * the base directory (which is pulled out of certdbpath) and the
 * cert and key prefixes (pulled out of certdbpath and keydbpath
 * respectively).
 */

Hopefully that will prevent someone from breaking this in the future.

Comment 4

[Rich Megginson]

Fix looks good.

Comment 5

[Mark Smith [:mcs]]

Fix committed to the trunk:

mozilla/directory/c-sdk/ldap/libraries/libssldap/clientinit.c
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/libraries/libssldap/errormap.c
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/libraries/libssldap/ldapsinit.c
  new revision: 5.6; previous revision: 5.5
mozilla/directory/c-sdk/ldap/clients/tools/argpin.c
  new revision: 5.1; previous revision: 5.0
mozilla/directory/c-sdk/ldap/clients/tools/common.c
  new revision: 5.6; previous revision: 5.5
mozilla/directory/c-sdk/ldap/clients/tools/ldapcmp.c
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/clients/tools/ldapcompare.c
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/clients/tools/ldapdelete.c
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/clients/tools/ldapmodify.c
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/clients/tools/ldapsearch.c
  new revision: 5.5; previous revision: 5.4
    Fix 183385 - eliminate references to NSS key/cert/module files.
        The LDAP tools code no longer has any knowledge of the NSS
        file names; the certpath2keypath() function has been deleted
        and we now simply use the certdbpath as keydbpath when it is
        provided (it makes no difference in the end). But note that
        because we need to maintain backwards compatibility, the
        libssldap code used by the ldapssl_.*_init() functions still
        knows the default name of the NSS module file (secmod.db),
        and the code also relies on the fact that the suffix for the
        key and cert files is ".db" and that the first letter in the
        main part of the name is either 'c' or 'k'.
    Also fixed a bug that caused the module file name specified on
        the LDAP tools command line (-m name) to be ignored.
    The ldapsearch and ldapcmp tools now exit with LDAP_NO_MEMORY
        if an LDIF fragment can't be constructed.
    Also fixed some issues reported by lint:
        Return values that were ignored.
        Make more functions and global variables static.
        Add /*ARGSUSED*/ and similar lint-friendly comments.

Comment 6

[Frankie]

Spam for bug 129472

Comment 7

[Mozilla_rocks]

Comment on attachment 117852 [details] [diff] [review]
improved fix: added comment

removing obsolete review request