141754 - Enhancement: Should support startTls

Status: RESOLVED FIXED

(Directory Graveyard :: LDAP C SDK, enhancement)

Description

[Michael Hein]

The C SDK should support startTls

Comment 1

[Michael Hein]

Attachment: First cut

This is a first cut at starttls.  It has not been tested and only compiled on
Solaris for now.  I had to make a change to build.mk as well......the line 
PERL = ?perl is causing problems for the generation of *def/*exp files.

Comment 2

[Michael Hein]

Attachment: Test case

Small test program

Comment 3

[Mark Smith [:mcs]]

Thanks for submitting this. I have not tried the code yet, but I have a few of
initial comments:

- I do not think the application should pass the NSS cert db and key db paths to
ldapssl_tls_start_s(). Instead, we should require that the application call one
of the existing ldapssl_..._init() functions first.

- We need to check and see what start TLS APIs are provided in other LDAP API
implementations (to see if it makes sense to align with what they have done). I
will check Microsoft, IBM and OpenLDAP soon. For example, we should probably
have an ldapssl_tls_start() function and another one that parses the response
and switches the connection to secure mode.

- We need to think about how to factor this code so that it could be used by the
Mozilla 1.x client team. That might be difficult, because they do not use
libssldap at all.

Comment 4

[Mark Smith [:mcs]]

Comment on attachment 82029 [details] [diff] [review]
First cut

I need to try this code and do a more detailed review, but here are a few
comments:

1) We will want to support an asynchronous API in addition to
ldapssl_starttls_s(). Someone should look at the OpenLDAP (or IBM) API for
start TLS to see if it makes sense to implement the same API.

2) I'd rather not expose the LDAP_X_OPT_SOCKBUF functionality to other LDAP
applications. There might be a different way for the prldap layer to get at the
socket info.

3) Rather than providing the two new prldap_get_default_socket_info() and
prldap_set_default_socket_info() functions, why not just add an option or two
to   oprldap_get_session_option() to allow callers to retrieve the fd and
socketarg for the default connection, e.g.,

#define PRLDAP_OPT_DEFAULT_FD	      2
#define PRLDAP_OPT_DEFAULT_SOCKETARG  3
?

Then the existing prldap_set_socket_info() and prldap_get_socket_info()
functions can be used after calling prldap_get_session_option() to retrieve the
above.

4) I think we should keep the basic SSL initialization separate from
ldapssl_tls_start_s() and just require that initialization be done before
ldapssl_tls_start_s() is called. Applications that want to handle SSL
initialization themselves should be able to use our startTLS APIs. Basically,
just remove the ldapssl_clientauth_init() call inside
ldapssl_enableSSL_on_open_connection().

5) We need to think about how much effort we should put into helping
applications that can't use the libssldap layer use startTLS (e.g., the Mozilla
1.x client).

Comment 5

[Michael T. Babcock]

New preferences request to auto-detect server capabilities filed as bug 185631.

Comment 6

[Frankie]

Spam for bug 129472

Comment 7

[Mark Smith [:mcs]]

Mass reassign of 4 older LDAP C SDK bugs to [e-mail].

Comment 8

[David Boreham]

Attachment: Proposed patch for Start TLS

This code was written by Alok Gondhalekar <[e-mail]>, I'm uploading
it because I wanted to attach some commentary about the design
decisions. I will add that text to the bug report next...

Comment 9

[David Boreham]

Some comments on the patch I just uploaded:

1. It implements an -L option in the client tools. This tells ldapsearch etc 
to use Start TLS. Also implemented is a -LL option. This is similar to options 
supported in the OpenLDAP clients tools: -L says to try Start TLS but carry on
using plaintext if it fails. -LL says to fail hard if Start TLS fails.

2. The central problem with Start TLS is to convert an existing connection
to an SSL connection, mid-flight. There are two different cases to be handled:
a) Where the existing connection is using the prldap layer (NSPR I/O) and
b) Where the existing connection is not using prldap.
In both cases, the eventual goal is to have prldap enabled, and then SSL
(because NSS SSL relies on NSPR I/O).
For the already-prladp case, things are a bit easier: we only need to 
import the NSPR socket into SSL. For the non-prldap case, things are
more complex: we need to first import the connection into the prldap layer.
Subsequent to that step, the two cases are the same. To facilitate the 
connection import to prldap, two new functions were added to that layer:
prldap_check_nspr(), which tells the caller if the connection is already
using prldap/NSPR (the caller otherwise has no way to tell due to layering
issues). The second call is prldap_import_connection() which is patterned
after other 'import' functions: it takes the OS socket as its second argument,
and does whatever is necessary to enable prldap on the LD passed as the first 
argument.

3. SSL configuration is something of a problem. This is because it is
not possible to perform some of the SSL config until SSL-related structures are
created. These stuctures are not created until _after_ the decision to enable
SSL on the connection. However, once SSL has been enabled, it's logically
too late to perform the SSL config. This situation is also present in
'classic' SSL in the SDK. It seems that the API, and applications that use it
actually take advantage of the fact that no real SSL negotiation is done 
until the next PDU following SSL enable is sent. This might not be totally
clear to an application writer, but it's how the code has always worked, AFAIK.
Anyway, the result is that you _can_ call the ssl configuration functions
after enabling SSL, and the right thing happens. This is because those
functions change state in the SSL structures, and within NSS, and that 
state is not used until SSL negotation takes place. We have retained this
behavior with Start TLS: so an application can (and indeed must) call 
ldap_start_tls_s() first, then call ldaps_enable_clientauth() (or similar),
and _then_ perform an LDAP operation such as ldap_search(). SSL negotiation
on-the-wire only happens at the point the ldap_search() sends the search
request PDU. 

4. The API implemented (ldap_start_tls_s() ) is identical to the OpenLDAP
API, and very similar to Microsoft's and Novell's. 

5. Layering and modularity issues were a challenge in this implementation.
It seems hard to do Start TLS without introducing some accessors to 
break layering. We tried several different approches and this one has
the least added accessors. We added LDAP_X_OPT_EXTIO_SOCKET_FN_PTRS to allow
code outside of libldap (libsldap specifically) to retrieve the 
native connection socket from the BER layer.

This code has been tested against both the Netscape Directory Server
and the OpenLDAP server, on the following (client) platforms: Win32,
HP-UX 11.11 (both 32-bit and 64-bit), Solaris9 (both 32-bit and 64-bit),
Linux (Redhat AS3, Mandrake 9.2 and Mandrake 10.0).

  

Comment 10

[Mark Smith [:mcs]]

Comment on attachment 155600 [details] [diff] [review]
Proposed patch for Start TLS

Overall, the patch looks very good.  Here are my comments on the changes in
mozilla/directory/c-sdk/ldap/clients/tools/common.c:

1) We can't use -L because that option is already used by ldapsearch to request
that LDIF be output (which is the default, but given that many people include
-L because OpenLDAP's ldapsearch requires it, I'd hate to change the meaning of
-L).  All single letter options are used by one of the tools or another; see
the "not used by any of the tools" line in Options.txt for details.  My best
suggestion is to use -ZZ and -ZZZ in place of -L and -LL.  I am open to a
better suggestion (bug 199130 is already open saying we need to support GNU
style long options, e.g., --tls).

2) Please add a space after the word request in the -LL usage line.

3) Should we call perror() when the prldap_init() call fails?  That is what is
done below the startTLS code you added; I am not 100% sure the system errno is
interesting in that case though.

4) I think the ldapssl_client_init() call you added is redundant (it or
ldapssl_pkcs_init() is called above).

5) Please exit with the value returned by ldap_start_tls_s() instead of
LDAP_LOCAL_ERROR.

Comment 11

[Mark Smith [:mcs]]

Comment on attachment 155600 [details] [diff] [review]
Proposed patch for Start TLS

Here are my comments on the core libldap changes
(.../ldap/include/ldap-extension.h, .../ldap/libraries/libldap/extendop.c, 
getoption.c, ldap-int.h, and setoption.c):

1) LDAP_OPT_START_TLS should be replaced by LDAP_X_OPT_START_TLS.  New LDAP API
extensions should begin with LDAP_X_OPT and should be assigned numbers based
off LDAP_OPT_PRIVATE_EXTENSION_BASE.  This is required by the LDAP C API
Internet Draft (currently suspended, but I hope to bring it back to life again
soon).

2) The older LDAP_X_OPT_EXTIO_FN_PTRS and the new
LDAP_X_OPT_EXTIO_SOCKET_FN_PTRS option you added are very similar (the
difference is that the new one retrieves the I/O functions for the default/main
LDAP TCP connection).  I would prefer to add an option that is very similar to
LDAP_OPT_DESC (perhaps LDAP_X_OPT_SOCKETARG) that is used to get and set the
socketarg.

3) The fix to extendop.c is also the subject of bug 242143.  Please take a look
at the patch proposed there (nearly the same).

4) Another thought on LDAP_OPT_START_TLS: it seems like it is not something
applications should manipulate themselves but rather something needed
internally by the libssldap layer.  Is that true?  If so, why not just add it
to LDAPSSLSessionInfo?

Comment 12

[Mark Smith [:mcs]]

Comment on attachment 155600 [details] [diff] [review]
Proposed patch for Start TLS

Here are my comments on the libssldap related changes (files
.../include/ldap_ssl.h, .../libraries/libldap_ssl.ex,
.../libraries/libssldap/ldapsinit.c, .../libraries/msdos/winsock/ldapssl.def,
.../libraries/msdos/winsock/nsldapssl32.def, and
.../libraries/msdos/winsock/nssldap32.def); most of the comments are related to
ldapsinit.c:

1) Let's use the same name for the startTLS OID macro as OpenLDAP does.  They
have:

#define LDAP_EXOP_START_TLS   "1.3.6.1.4.1.1466.20037"	/* RFC 2830 */


2) It would be nice to factor some of the common code in ldaptls_complete() and
ldapssl_connect() and place it in a common function that both call.  At a
glance, it doesn't look too difficult to me but maybe you already tried and
decided it wasn't worth the trouble?

3) If ldaptls_complete() fails it closes the connection.  Is that what we
really want to happen?	Doesn't that make it so the -L option won't always fail
softly?

4) Inside ldaptls_setup(), I would return LDAP_LOCAL_ERROR in the cases you
return LDAP_OTHER.

5) Let's create a small internal helper function called
ldapssl_alloc_sessioninfo() and call it from ldapssl_install_routines() and
ldaptls_setup().  That way if we ever improve that code it will be in one
place.

6) Inside ldap_start_tls_s() there are a lot of places where the return value
from ldap_set_option() is checked against LDAP_SUCCESS.  Technically,
ldap_set_option() returns 0 or -1, not an LDAP result code.  So it would be
clearer to check for == 0 or < 0.  Same for ldap_get_option().

7) In this code snippet from ldap_start_tls_s():

+   if ( ( rc = ldap_extended_operation_s( ld, START_TLS_OID, NULL,
serverctrls,
+	clientctrls, &oidptr, &dataptr ) ) != LDAP_SUCCESS )
+   {
+	ber_bvfree( dataptr );
+	ldap_memfree( oidptr );
+	ldap_set_lderrno( ld, LDAP_UNWILLING_TO_PERFORM, NULL, NULL );
+	return( LDAP_UNWILLING_TO_PERFORM );
+   }


Shouldn't LDAP_UNWILLING_TO_PERFORM be replaced with rc?  If possible, I would
change ldaptls_setup(), ldaptls_complete() to return an LDAP result code and
return that instead of unwilling to perform in this section also:

+   /* Initialize TLS and enable SSL on the LDAP session */
+   if ( ( rc = ldaptls_setup( ld ) ) != LDAP_SUCCESS	||
+	 ( rc = ldaptls_complete( ld ) ) != LDAP_SUCCESS   ||
+	 ( rc = ldap_set_option( ld, LDAP_OPT_SSL, LDAP_OPT_ON ) )!=
LDAP_SUCCESS )
+   {
+
+	/* Allow to proceed in clear text if secure session fails */
+	ldap_set_option( ld, LDAP_OPT_SSL, LDAP_OPT_OFF );
+	ldap_set_option( ld, LDAP_OPT_START_TLS, LDAP_OPT_OFF );
+	ldap_set_lderrno( ld, LDAP_UNWILLING_TO_PERFORM, NULL, NULL );
+	return( LDAP_UNWILLING_TO_PERFORM );
+   }

Comment 13

[Mark Smith [:mcs]]

Comment on attachment 155600 [details] [diff] [review]
Proposed patch for Start TLS

This is the final batch... my comments on the proposed libprldap changes (files
.../include/ldappr.h, .../libraries/libprldap/ldappr-int.h,
.../libraries/libprldap/ldappr-io.c, .../libraries/libprldap/ldappr-public.c,
.../libraries/libprldap/libprldap.ex,
.../libraries/msdos/winsock/nsldappr-incl.def,
.../libraries/msdos/winsock/nsldappr32.def):

1) Can we rename the prldap_check_nspr() function to prldap_is_installed()?

2) Insider prldap_check_nspr(), please check the result of ldap_get_option()
against 0 instead of LDAP_SUCCESS.  Otherwise someone will think
ldap_get_option() could return values like LDAP_NO_MEMORY.

3) prldap_import_connection() assumes the file desc it is importing is the one
associated with the default/main LDAP TCP connection.  I think it would be
better to acknowledge that fact very clearly by removing the orig_socket
parameter (retrieve it via a call like ldap_get_option( ld, LDAP_OPT_DESC, ...)

4) My original thought use of the prldap layer would be a prerequisite to using
startTLS in the LDAP C SDK.  But now that you have written the code to import
into prldap, we might as well keep it.	The only thing that worries me is that
PR_ImportTCPSocket() does not appear in a public NSPR header file.  Any idea
why it is not a public NSPR function?

Comment 14

[Mark Smith [:mcs]]

I just noticed 2 new warnings when I built the revised code:

setoption.c:234: warning: passing arg 3 of `ber_sockbuf_set_option' discards
qualifiers from pointer target type

common.c:1003: warning: suggest parentheses around assignment used as truth value

I guess a cast should be added to fix the first one.  In common.c it looks like
a left paren. is misplaced:

+       /* Initialize TLS over the clear-text LDAP session */
+       if (rc = (ldapssl_client_init(ssl_certdbpath, NULL))< 0) {
...

Comment 15

[Mark Smith [:mcs]]

Added dependency on bug 242143 (ldap_extended_operation(3) sends requestValue
when not requested).

Comment 16

[David Boreham]

Comment on mcs's comment:

>4) Another thought on LDAP_OPT_START_TLS: it seems like it is not something
>applications should manipulate themselves but rather something needed
>internally by the libssldap layer.  Is that true?  If so, why not just add it
>to LDAPSSLSessionInfo?

I think the idea was that applications might want to know after the fact if
the connection is used start tls. If this seems unlikely then I agree that
it isn't necessary to expose this state to the application.

Comment 17

[David Boreham]

More comments on comments:

>2) It would be nice to factor some of the common code in ldaptls_complete() 
and
>ldapssl_connect() and place it in a common function that both call.  At a
>glance, it doesn't look too difficult to me but maybe you already tried and
>decided it wasn't worth the trouble?

I agree. We decided not to do this at this point on the basis that it
would further confuse the diffs. But in principle we think this is a very good 
idea.

Comment 18

[David Boreham]

Answer to mcs comment:

>4) My original thought use of the prldap layer would be a prerequisite to 
using
>startTLS in the LDAP C SDK.  But now that you have written the code to import
>into prldap, we might as well keep it.	The only thing that worries me is that
>PR_ImportTCPSocket() does not appear in a public NSPR header file.  Any idea
>why it is not a public NSPR function?

I'm not sure, but it was wan-teh that suggested its use, so I believe it is
kosher. I will ask him what the status of this function is (I believe he
might be on vacation right now).

Comment 19

[David Boreham]

Excepting the specific cases above where I've made a comment,
I agree with all mcs's review comments.

Comment 20

[Mark Smith [:mcs]]

My responses to David's responses:

On the LDAP_OPT_START_TLS question, I do not think it is worth exposing this to
applications.  I'd rather minimize the additional interfaces and options in
libldap; there are already many.  So I'd like to see it change to a libssldap
internal bit.

On refactoring the code in ldaptls_complete() and ldapssl_connect() so less code
is duplicated, I think we can defer doing that for now.  Maybe I will make a
pass at that after we land the startTLS feature.

On PR_ImportTCPSocket(), please check with Wan-Teh.  But for now it sounds like
it is OK to use.

Let me know if you want help with some of the other changes.  I do not currently
have a DS set up where I can test startTLS, but I could do that with only a
moderate amount of pain ;-)

Comment 21

[David Boreham]

Wan-Teh says that PR_ImportTCPSocket is ok to use if there is no alternative
(which there isn't in this case).

Comment 22

[Alok Gondhalekar]

Attachment: Patch after changes as per mcs review

New proposed patch for Start TLS; includes changes as per Comments #10 through
#15 and #17 by Mark Smith.

I would like to add some notes about the new patch and client tools -

1. Case of Hard failure : Missing CA cert in certificate db. 

2. Case of soft failure : Server is unwilling to perform Start TLS.( disabled
SSL/TLS on server or  no extended operation support by server).

The clause from section 2.3 from RFC 2830 holds true in this case; if the Start
TLS operation fails then, "The client MAY proceed with any LDAP operation, or
it MAY close the connection."

Comment 23

[Mark Smith [:mcs]]

Thanks for the revised patch.  Can you recreate it as a unified diff?  I always
use a command like this to create diffs for posting in bug reports:

cvs -nq diff -u8 -N

(-u8 for unified diffs with 8 lines of context and -N to include the contents of
new files).

Comment 24

[Alok Gondhalekar]

Attachment: unified diffs of patch (id=156540)

Let me know if you need the unified diffs with some blank line cleanup.


Thanks.

Comment 25

[David Boreham]

A note about testing: this patch has been incorporated into private builds
of the SDK that are in turn used in the Netscape Server and the client tools.
The resulting builds have been subjected to the server regression tests and 
passed. In addition new start TLS tests have been run that exercise the 
code quite well on : Win32, Linux, 64-bit Solaris, 32-bit Solaris, 64-bit HP-
UX and 32-bit HP-UX.

Comment 26

[Mark Smith [:mcs]]

Comment on attachment 156631 [details] [diff] [review]
unified diffs of patch (id=156540)

The new patch looks good.  Just a few small comments:


>Index: mozilla/directory/c-sdk/ldap/clients/tools/common.c
> ...
>+    
>+    /* If '-Z' is specified, ignore -ZZ or -ZZZ */
>+    if ( isZ && isZZ ) {
>+	fprintf( stderr, "%s: with -Z option, do not specify -ZZ or -ZZZ option\n\n", ldaptool_progname ); 
>+	return (-1);
>+     }

I do not think the above code is needed now that -ZZ effectively "turns off"
-Z.  Right?


>Index: mozilla/directory/c-sdk/ldap/libraries/libldap/getoption.c
> ...
>+	  
>+	/* extended socket i/o function pointers (to get socketargp)*/
>+	case LDAP_X_OPT_SOCKETARG:
>+		if ( ber_sockbuf_get_option( ld->ld_sbp,
>+		    LBER_SOCKBUF_OPT_EXT_IO_FNS, optdata ) != 0 ) {
>+			LDAP_SET_LDERRNO( ld, LDAP_LOCAL_ERROR, NULL, NULL );
>+			rc = -1;
>+		}
>+		break;

LDAP_X_OPT_SOCKETARG should not return all of the I/O function pointers; it
should just return the socketarg.  It looks like you renamed the option but did
not revise the implementation.


>Index: mozilla/directory/c-sdk/ldap/libraries/libldap/setoption.c
> ...
>+	 /* extended socket i/o function pointers (set Socket Arg)*/
>+	case LDAP_X_OPT_SOCKETARG:
>+		if ( ber_sockbuf_set_option( ld->ld_sbp,
>+		    LBER_SOCKBUF_OPT_EXT_IO_FNS, (void *)optdata ) != 0 ) {
>+			LDAP_SET_LDERRNO( ld, LDAP_LOCAL_ERROR, NULL, NULL );
>+			rc = -1;
>+		}
>+	  break;
>+	  

Same comment here.


>Index: mozilla/directory/c-sdk/ldap/libraries/libssldap/ldapsinit.c
> ...
>+    /*
>+     * Retrieve session info. so we can store a pointer to our session info.
>+     * in our socket info. later.
>+     */
>+    memset( &sei, 0, sizeof(sei));
>+    sei.seinfo_size = PRLDAP_SESSIONINFO_SIZE;
>+    if ( prldap_get_session_info( ld, NULL, &sei ) < 0 ) {
>+	return( -1 );
>+    }

Unlike ldap_get_option(), prldap_get_session_info() DOES return an LDAP result
code so it would be better to check for != LDAP_SUCCESS above. If we could
change ldap_get_option() we would but it is much too late for that ;-)

>+    /*
>+     * Let the standard NSPR to LDAP layer know about the new socket and
>+     * our own socket-specific data.
>+     */
>+    soi.soinfo_prfd = sslfd;
>+    soi.soinfo_appdata = (void *)ssoip;
>+
>+    if ( prldap_set_socket_info( intfd, socketargp, &soi ) < 
>+			0 ) {
>+		goto close_socket_and_exit_with_error;
>+    }

Same comment as above applies to prldap_set_socket_info() too.


>+    /* Check if NSPR Layer is Installed */
>+    if ( !prldap_is_installed(ld) )
>+    {  /* No NSPR Layer installed, 
>+        * Call prldap_import_connection() that installs the NSPR I/O layer
>+	* and imports connection details from Clear-text connection
>+	*/
>+    	if ( prldap_import_connection(ld, fd) <  0 )
>+    	{
>+		ldap_set_lderrno( ld, LDAP_OTHER, NULL, NULL );
>+		return( -1 );
>+    	}

prldap_import_connection() also returns an LDAP result code... at least it
should.  Please fix the calls to prldap_import_connection() as well as the
implementation of prldap_import_connection().

Also, these comments I made on attachment 155600 [details] [diff] [review] were not addressed.  I still
believe in them; please let me know if you disagree with them:

a) In tools/common.c, I think the ldapssl_client_init() call you added is
redundant (it or
ldapssl_pkcs_init() is called above).

b) prldap_import_connection() assumes the file desc it is importing is the one
associated with the default/main LDAP TCP connection.  I think it would be
better to acknowledge that fact very clearly by removing the orig_socket
parameter (retrieve it via a call like ldap_get_option( ld, LDAP_OPT_DESC, ...)

c) Let's create a small internal helper function called
ldapssl_alloc_sessioninfo() and call it from ldapssl_install_routines() and
ldaptls_setup().  That way if we ever improve that code it will be in one
place.

Comment 27

[David Boreham]

Attachment: Patch incorporating mcs full set of review comments

Comment 28

[Mark Smith [:mcs]]

Attachment: revised patch: improved error handling

In reviewing attachment 158112 [details] [diff] [review] I noticed a few remaining error handling issues.
 I fixed them, and this is the revised patch.  I will also attach the changes
compared to the previous proposal (attachment 158112 [details] [diff] [review]) and the revised files
where I made the biggest changes.

Comment 29

[Mark Smith [:mcs]]

Attachment: diffs against previous proposal

Comment 30

[Mark Smith [:mcs]]

Attachment: revised ldapsinit.c and prldap-public.c

Comment 31

[David Boreham]

Thanks! We will re-test the revised code and report back.

Comment 32

[David Boreham]

These last changes test out ok for us.

Comment 33

[Mark Smith [:mcs]]

Much thanks to Sun, Netscape/AOL, especially Alok and David for their
contributed fixes.

This fix has now been committed to the trunk.  It is good to close this aging bug!

mozilla/directory/c-sdk/ldap/clients/tools/common.c
  new revision: 5.14; previous revision: 5.13
mozilla/directory/c-sdk/ldap/include/lber.h
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/include/ldap-extension.h
  new revision: 5.4; previous revision: 5.3
mozilla/directory/c-sdk/ldap/include/ldap_ssl.h
  new revision: 5.5; previous revision: 5.4
mozilla/directory/c-sdk/ldap/include/ldappr.h
  new revision: 5.5; previous revision: 5.4
mozilla/directory/c-sdk/ldap/libraries/libldap_ssl.ex
  new revision: 5.5; previous revision: 5.4
mozilla/directory/c-sdk/ldap/libraries/liblber/io.c
  new revision: 5.8; previous revision: 5.7
mozilla/directory/c-sdk/ldap/libraries/libldap/getoption.c
  new revision: 5.4; previous revision: 5.3
mozilla/directory/c-sdk/ldap/libraries/libldap/setoption.c
  new revision: 5.5; previous revision: 5.4
mozilla/directory/c-sdk/ldap/libraries/libprldap/ldappr-int.h
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/libraries/libprldap/ldappr-io.c
  new revision: 5.4; previous revision: 5.3
mozilla/directory/c-sdk/ldap/libraries/libprldap/ldappr-public.c
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/libraries/libprldap/libprldap.ex
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/libraries/libssldap/ldapsinit.c
  new revision: 5.10; previous revision: 5.9
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/ldapssl.def
  new revision: 5.5; previous revision: 5.4
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nsldappr-incl.def
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nsldappr32.def
  new revision: 5.3; previous revision: 5.2
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nsldapssl32.def
  new revision: 5.5; previous revision: 5.4
mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nssldap32.def
  new revision: 5.5; previous revision: 5.4
    Fix bug # 141754 - Enhancement: Should support startTLS.
     LDAP command line tools now accept 2 new options:
        -ZZ  (issue a startTLS request)
        -ZZZ (like -ZZ but require a successful response).
     API extensions:
      ldap_ssl.h: LDAP_EXOP_START_TLS macro (OID of start TLS extended op.).
      libssldap:  Added ldap_start_tls_s() function.
      libprldap:  Added prldap_is_installed() and prldap_import_connection().
      libldap:    Added new LDAP_X_OPT_SOCKETARG option for ldap_get_option()
                    and ldap_set_option() (get/set the socketarg associated
                    with the main LDAP TCP connection).
      liblber:    Added new LBER_SOCKBUF_OPT_SOCK_ARG option for
                     ber_sockbuf_set_option() and ber_sockbuf_get_option()
                     (get/set the socketarg associated with a Sockbuf).
    Also, some refactoring was done in libssldap to simplify the code.

Comment 34

[Alok Gondhalekar]

Attachment: Patch over attachment 158207

An additional semi-colon is causing build failures on some non-gcc compilers.
The diff is attached to show the location of the extra character.

Comment 35

[Mark Smith [:mcs]]

Oops.  Fix committed:

mozilla/directory/c-sdk/ldap/libraries/libssldap/ldapsinit.c
  new revision: 5.11; previous revision: 5.10
    Additional fix for 141754 - Enhancement: Should support startTls:
      Remove extra semicolon which causes an error on some non-gcc compilers.