need a way to set SSL options when using libssldap

RESOLVED FIXED

Status

Directory
LDAP C SDK
P1
enhancement
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: mcs, Assigned: mcs)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

16 years ago
Several people have asked for a way to set SSL options (as you can do with the
NSS call SSL_OptionSet). Currently there is no way to do it if you use the
libssldap layer. Now we (Netscape) have a need for this ASAP. I propose we add
two API calls:

/*
 * Set or get SSL options on an existing SSL-enabled LDAP session handle.
 * If ld is NULL, the default options used for all future LDAP SSL sessions
 * are the ones affected. The option values are specific to the underlying
 * SSL provider; see ssl.h within the Network Security Services (NSS)
 * distribution for the options supported by NSS (the default SSL provider).
 *
 * These functions should be called before any LDAP connections are created.
 * Both functions return 0 if all goes well.
 */
int LDAP_CALL ldapssl_option_set( LDAP *ld, PRInt32 option, PRBool on );
int LDAP_CALL ldapssl_option_get( LDAP *ld, PRInt32 option, PRBool *onp );
(Assignee)

Comment 1

16 years ago
Working on this.
Status: NEW → ASSIGNED
(Assignee)

Comment 2

16 years ago
*** Bug 153249 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

16 years ago
OS: Windows 3.1 → All
Priority: -- → P1
(Assignee)

Comment 3

16 years ago
Created attachment 88689 [details] [diff] [review]
proposed fix

The new code is fairly straightforward, although supporting ldapssl_option_set(
NULL, ... ) as well as ldapssl_option_set( ld, ... ) and preserving the NSS
defaults in the absence of any ldapssl_option_set() makes for a bit more code
than I expected. Not too bad though.

I also fixed a bug in common.c:ldaptool_print_lderror (LDAP command line tools)
where we did not check for SSL errors when the error code was "can't connect."
We were only checking on "server down."

I also fixed a bug in ldapsinit:do_ldapssl_connect() that sometimes caused
PR_Close() to be called twice on an SSL file descriptor when an error occurred
(once in do_ldapssl_connect() itself and once in the libprldap close function
that is called from do_ldapssl_connect()).
(Assignee)

Comment 4

16 years ago
Forgot to mention that I also updated the error string mapping tables in
libssldap (a few newer NSPR and NSS errors were missing).
(Assignee)

Comment 5

16 years ago
Created attachment 88695 [details] [diff] [review]
revised fix: avoid NSPR types in the ldapssl_ API

Slight change of plans: we don't use NSPR types anywhere else in the ldapssl_
API, so I'd rather not start now. Replaced PRBool with an int. Otherwise
identical to the first patch.
Attachment #88689 - Attachment is obsolete: true

Comment 6

16 years ago
Looks ok, but it would be nice to get another reviewer.
(Assignee)

Comment 7

16 years ago
I just found out about bug 135261:

  "SSL_REQUIRE_CERTIFICATE semantic troubles"
  http://bugzilla.mozilla.org/show_bug.cgi?id=153250

The proposed fix involves allowing 4 values for the SSL_REQUIRE_CERTIFICATE
option (and shoving them into a PRBool so there is no API change within NSS).
But my proposed ldapssl_option_get/set() implementation assumes all SSL options
can be represented in one bit. How inconvenient.
(Assignee)

Comment 8

16 years ago
Created attachment 89269 [details] [diff] [review]
revised fix: store a PRBool value for each option

Same as the last patch, except for ldapsinit.c: we now store a complete PRBool
value for each option (same as the NSS API supports). This simplifies the code
as well (at the expense of taking up more space for each LDAPS session handle).
Attachment #88695 - Attachment is obsolete: true

Comment 9

16 years ago
Ok.  Doesn't take that much more space to use an array than a bit vector.
(Assignee)

Comment 10

16 years ago
Created attachment 89433 [details] [diff] [review]
revised fix: rename new ldapssl functions; add exports for MacOS and Windows
Attachment #89269 - Attachment is obsolete: true
(Assignee)

Comment 11

16 years ago
I have been convinced to go with these function names:

ldapssl_set_option()
ldapssl_get_option()

I think these names are more consistent with those used for other libssldap and
libldap functions, e.g., ldap_set_option(). Building on Win32 now before I
commit this.

Comment 12

16 years ago
Ok.
(Assignee)

Comment 13

16 years ago
Fix committed to the trunk:

mozilla/directory/c-sdk/ldap/clients/tools/common.c
  new revision: 5.4; previous revision: 5.3
mozilla/directory/c-sdk/ldap/include/ldap_ssl.h
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/libraries/libldap_ssl.ex
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/libraries/libssldap/ldapsinit.c
  new revision: 5.4; previous revision: 5.3
mozilla/directory/c-sdk/ldap/libraries/libssldap/prerrstrs.h
  new revision: 5.1; previous revision: 5.0
mozilla/directory/c-sdk/ldap/libraries/libssldap/secerrstrs.h
  new revision: 5.1; previous revision: 5.0
mozilla/directory/c-sdk/ldap/libraries/libssldap/sslerrstrs.h
  new revision: 5.1; previous revision: 5.0
mozilla/directory/c-sdk/ldap/libraries/macintosh/LDAPSSLClient.exp
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/ldapssl.def
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nsldapssl32.def
  new revision: 5.2; previous revision: 5.1
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nssldap32.def
  new revision: 5.2; previous revision: 5.1
    Fix bug # 153250 - need a way to set SSL options when using libssldap.
        Added two new libssldap public functions: ldapssl_set_option() and
            ldapssl_get_option().
        Also fixed a bug in ldapsinit:do_ldapssl_connect() that sometimes
            caused PR_Close() to be called twice on an SSL file descriptor
            if an error occurred (once in do_ldapssl_connect() itself and
            once in the libprldap close function that is called from
            do_ldapssl_connect()).
        Also updated the NSPR and NSS "error code to string" mapping
            tables that are used by ldapssl_err2string().
        Also fixed a bug in common.c:ldaptool_print_lderror (LDAP command
            line tools) where we did not check for SSL errors when the
            error code was "can't connect."  We were only checking on
            "server down" errors.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 14

16 years ago
Spam for bug 129472
QA Contact: nobody → nobody
You need to log in before you can comment on or make changes to this bug.