We would like the ability to make SSL connections not only for sending and receiving e-mail but also to authenticate to an LDAP server. It seems kind of silly to require a secure connection when authenticating for e-mail and then sending your user-name and password in plain text when authenticating to the LDAP server. Many corporate e-mail systems require secure transmission of all directory data as well as the messages themselves.

Reassigning to leif. First we need to land a more current version of the LDAP C SDK, then we can do this.

reassigning to dmose. setting 1.0 milestone.

to play with that patch, you need 122115 and 124244 first.

that patch has errors. debugging on win32, I'm getting a WSAENOTCONN (PR_NOT_CONNECTED_ERROR) when I try to send on the socket. my initial guess, I'm not connected. ha ha. _MD_win32_map_default_error(int 10057) line 252 _MD_win32_map_send_error(int 10057) line 395 + 9 bytes _PR_MD_SEND(PRFileDesc * 0x05228240, const void * 0x01601ee0, int 44, int 0, unsigned int 4294967295) line 241 + 9 bytes SocketSend(PRFileDesc * 0x05228240, const void * 0x01601ee0, int 44, int 0, unsigned int 4294967295) line 680 + 25 bytes pl_DefSend(PRFileDesc * 0x05247120, const void * 0x01601ee0, int 44, int 0, unsigned int 4294967295) line 287 + 34 bytes ssl_DefSend(sslSocketStr * 0x05244c10, const unsigned char * 0x01601ee0, int 44, int 0) line 125 + 34 bytes ssl2_SendClear(sslSocketStr * 0x05244c10, const unsigned char * 0x04595b24, int 42, int 0) line 946 + 30 bytes ssl_SecureSend(sslSocketStr * 0x05244c10, const unsigned char * 0x04595b24, int 42, int 0) line 1127 + 21 bytes ssl_Send(PRFileDesc * 0x05243c50, const void * 0x04595b24, int 42, int 0, unsigned int 4294967295) line 1212 + 25 bytes pl_DefSend(PRFileDesc * 0x039a0190, const void * 0x04595b24, int 42, int 0, unsigned int 4294967295) line 287 + 34 bytes PR_Send(PRFileDesc * 0x039a0190, const void * 0x04595b24, int 42, int 0, unsigned int 4294967295) line 221 + 28 bytes prldap_write(int 1, const void * 0x04595b24, int 42, lextiof_socket_private * 0x05246770) line 195 + 26 bytes ber_flush(sockbuf * 0x0524f7b0, berelement * 0x04595a00, int 0) line 396 + 36 bytes nsldapi_ber_flush(ldap * 0x0524f940, sockbuf * 0x0524f7b0, berelement * 0x04595a00, int 0, int 1) line 328 + 17 bytes nsldapi_send_server_request(ldap * 0x0524f940, berelement * 0x04595a00, int 1, ldapreq * 0x00000000, ldap_server * 0x00000000, ldap_conn * 0x052467b0, char * 0x05246720, int 0) line 274 + 23 bytes nsldapi_send_initial_request(ldap * 0x0524f940, int 1, unsigned long 96, char * 0x0012de20, berelement * 0x04595a00) line 134 + 41 bytes simple_bind_nolock(ldap * 0x0524f940, const char * 0x0012de20, const char * 0x0012de70, int 1) line 135 + 23 bytes ldap_simple_bind(ldap * 0x0524f940, const char * 0x0012de20, const char * 0x0012de70) line 64 + 19 bytes nsLDAPOperation::SimpleBind(nsLDAPOperation * const 0x05243c00, const unsigned short * 0x00000000) line 130 + 89 bytes nsAbQueryLDAPMessageListener::OnLDAPInit(nsAbQueryLDAPMessageListener * const 0x0524a090, unsigned int 0) line 266 + 25 bytes nsLDAPConnection::OnStopLookup(nsLDAPConnection * const 0x0524a7ac, nsISupports * 0x00000000, const char * 0x04f6b810, unsigned int 0) line 1317 XPTC_InvokeByIndex(nsISupports * 0x0524a7ac, unsigned int 5, unsigned int 3, nsXPTCVariant * 0x0524fc20) line 106 nsProxyObject::Post(unsigned int 5, nsXPTMethodInfo * 0x0451f5f0, nsXPTCMiniVariant * 0x0012e0ac, nsIInterfaceInfo * 0x04f486b0) line 429 + 34 bytes nsProxyEventObject::CallMethod(nsProxyEventObject * const 0x0524fd30, unsigned short 5, const nsXPTMethodInfo * 0x0451f5f0, nsXPTCMiniVariant * 0x0012e0ac) line 547 + 55 bytes PrepareAndDispatch(nsXPTCStubBase * 0x0524fd30, unsigned int 5, unsigned int * 0x0012e15c, unsigned int * 0x0012e14c) line 115 + 31 bytes SharedStub() line 139 nsDNSRequest::FireStop(unsigned int 0) line 396 + 49 bytes nsDNSLookup::ProcessRequests() line 788 + 18 bytes nsDNSService::Lookup(nsDNSService * const 0x03991560, const char * 0x0524a510, nsIDNSListener * 0x0524fd30, nsISupports * 0x00000000, nsIRequest * * 0x0524a7e4) line 1431 nsLDAPConnection::Init(nsLDAPConnection * const 0x0524a7a0, const char * 0x0524a510, short 389, const unsigned short * 0x0012e2e0, nsILDAPMessageListener * 0x0524a090) line 538 + 65 bytes nsAbLDAPDirectoryQuery::DoQuery(nsAbLDAPDirectoryQuery * const 0x0524b514, nsIAbDirectoryQueryArguments * 0x0524acf0, nsIAbDirectoryQueryResultListener * 0x0524a6c0, int 100, int 0, int * 0x0524b574) line 600 + 105 bytes nsAbLDAPDirectory::StartSearch(nsAbLDAPDirectory * const 0x0524b568) line 283 + 45 bytes nsAbLDAPDirectory::GetChildCards(nsAbLDAPDirectory * const 0x0524b4bc, nsIEnumerator * * 0x0012e660) line 161 + 21 bytes nsAbView::EnumerateCards() line 282 + 50 bytes nsAbView::Init(nsAbView * const 0x0524bbc0, const char * 0x0524b980, nsIAbViewListener * 0x0524b810, const unsigned short * 0x043ba840, const unsigned short * 0x043ba710, unsigned short * * 0x0012e984) line 221 + 8 bytes XPTC_InvokeByIndex(nsISupports * 0x0524bbc0, unsigned int 3, unsigned int 5, nsXPTCVariant * 0x0012e944) line 106 XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode CALL_METHOD) line 1998 + 42 bytes XPC_WN_CallMethod(JSContext * 0x03ec6da0, JSObject * 0x0454c880, unsigned int 4, long * 0x044de07c, long * 0x0012ec20) line 1266 + 14 bytes js_Invoke(JSContext * 0x03ec6da0, unsigned int 4, unsigned int 0) line 788 + 23 bytes js_Interpret(JSContext * 0x03ec6da0, long * 0x0012fad0) line 2745 + 15 bytes js_Execute(JSContext * 0x03ec6da0, JSObject * 0x01538288, JSScript * 0x05249580, JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012fad0) line 968 + 13 bytes JS_EvaluateUCScriptForPrincipals(JSContext * 0x03ec6da0, JSObject * 0x01538288, JSPrincipals * 0x03eabe54, const unsigned short * 0x043e3390, unsigned int 21, const char * 0x05248b40, unsigned int 509, long * 0x0012fad0) line 3332 + 25 bytes nsJSContext::EvaluateString(nsJSContext * const 0x03ec6f50, const nsAString & {...}, void * 0x01538288, nsIPrincipal * 0x03eabe50, const char * 0x05248b40, unsigned int 509, const char * 0x01be569c, nsAString & {...}, int * 0x0012fbf8) line 676 + 85 bytes GlobalWindowImpl::RunTimeout(nsTimeoutImpl * 0x05248bb0) line 4034 + 115 bytes GlobalWindowImpl::TimerCallback(nsITimer * 0x05249650, void * 0x05248bb0) line 4365 nsTimerImpl::Process() line 283 + 17 bytes handleMyEvent(MyEventType * 0x05248120) line 331 PL_HandleEvent(PLEvent * 0x05248120) line 590 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x0037bbe0) line 520 + 9 bytes _md_EventReceiverProc(HWND__ * 0x001902a8, unsigned int 49404, unsigned int 0, long 3652576) line 1071 + 9 bytes USER32! 77e13eb0() USER32! 77e1401a() USER32! 77e192da() nsAppShellService::Run(nsAppShellService * const 0x03a0cb20) line 308 main1(int 2, char * * 0x00304b90, nsISupports * 0x00000000) line 1285 + 32 bytes main(int 2, char * * 0x00304b90) line 1625 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 77e87903()

problem #1, I'm trying to connect to port 389, it should be 636. here's my stack: do_ldapssl_connect(const char * 0x051ccf30, int 389, int -1, unsigned long 0, lextiof_session_private * 0x0509e1b0, lextiof_socket_private * * 0x04f692cc) line 158 ldapssl_connect(const char * 0x051ccf30, int 389, int -1, unsigned long 0, lextiof_session_private * 0x0509e1b0, lextiof_socket_private * * 0x04f692cc) line 236 + 29 bytes nsldapi_connect_to_host(ldap * 0x03f44e40, sockbuf * 0x04f69170, const char * 0x051ccf30, int 389, int 0, char * * 0x051ccef4) line 465 + 51 bytes nsldapi_new_connection(ldap * 0x03f44e40, ldap_server * * 0x0012f728, int 1, int 1, int 0) line 417 + 51 bytes nsldapi_open_ldap_defconn(ldap * 0x03f44e40) line 620 + 19 bytes nsldapi_send_server_request(ldap * 0x03f44e40, berelement * 0x04595a00, int 1, ldapreq * 0x00000000, ldap_server * 0x00000000, ldap_conn * 0x00000000, char * 0x0012fb18, int 0) line 174 + 9 bytes nsldapi_send_initial_request(ldap * 0x03f44e40, int 1, unsigned long 96, char * 0x0012fb18, berelement * 0x04595a00) line 134 + 41 bytes simple_bind_nolock(ldap * 0x03f44e40, const char * 0x0012fb18, const char * 0x0012fb68, int 1) line 135 + 23 bytes ldap_simple_bind(ldap * 0x03f44e40, const char * 0x0012fb18, const char * 0x0012fb68) line 64 + 19 bytes nsLDAPOperation::SimpleBind(nsLDAPOperation * const 0x051c88d0, const unsigned short * 0x00000000) line 130 + 89 bytes nsAbQueryLDAPMessageListener::OnLDAPInit(nsAbQueryLDAPMessageListener * const 0x05044e40, unsigned int 0) line 266 + 25 bytes nsLDAPConnection::OnStopLookup(nsLDAPConnection * const 0x04ff16ec, nsISupports * 0x00000000, const char * 0x04f6b810, unsigned int 0) line 1317 XPTC_InvokeByIndex(nsISupports * 0x04ff16ec, unsigned int 5, unsigned int 3, nsXPTCVariant * 0x03f57bb0) line 106 EventHandler(PLEvent * 0x03f57e70) line 515 + 41 bytes PL_HandleEvent(PLEvent * 0x03f57e70) line 590 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x0037bbe0) line 520 + 9 bytes _md_EventReceiverProc(HWND__ * 0x001902a8, unsigned int 49404, unsigned int 0, long 3652576) line 1071 + 9 bytes USER32! 77e13eb0() USER32! 77e1401a() USER32! 77e192da() nsAppShellService::Run(nsAppShellService * const 0x03a0cb20) line 308 main1(int 2, char * * 0x00304b90, nsISupports * 0x00000000) line 1285 + 32 bytes main(int 2, char * * 0x00304b90) line 1625 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 77e87903()

this patch gets me closer. I seem to be able to send across the SSL socket, but I think I only get back EOF.

new patch coming, I'm getting closer, here's my crash: dddddddd() _PR_MD_PR_POLL(PRPollDesc * 0x046fbb00, int 5, unsigned int 55930) line 147 + 42 bytes PR_Poll(PRPollDesc * 0x046fbb00, int 5, unsigned int 55930) line 136 + 17 bytes prldap_poll(ldap_x_pollfd * 0x046fa0c0, int 5, int 1000, lextiof_session_private * 0x040346e0) line 265 + 34 bytes nsldapi_iostatus_poll(ldap * 0x040355f0, timeval * 0x044ffbb0) line 1180 + 46 bytes wait4msg(ldap * 0x040355f0, int -1, int 0, int 1, timeval * 0x044fff34, ldapmsg * * 0x044fff28) line 351 + 13 bytes nsldapi_result_nolock(ldap * 0x040355f0, int -1, int 0, int 1, timeval * 0x044fff34, ldapmsg * * 0x044fff28) line 127 + 29 bytes ldap_result(ldap * 0x040355f0, int -1, int 0, timeval * 0x044fff34, ldapmsg * * 0x044fff28) line 93 + 27 bytes nsLDAPConnectionLoop::Run(nsLDAPConnectionLoop * const 0x040368d0) line 972 + 24 bytes nsThread::Main(void * 0x04036790) line 120 + 26 bytes _PR_NativeRunThread(void * 0x04036570) line 413 + 13 bytes _threadstartex(void * 0x04036470) line 212 + 13 bytes KERNEL32! 77e92ca8()

from my debugging, it looks like this this is where things go bad: in prldap_poll() /* populate NSPR poll info. based on LDAP info. */ for ( i = 0; i < nfds; ++i ) { if ( NULL == fds[i].lpoll_socketarg ) { pds[i].fd = NULL; } else { --> pds[i].fd = PRLDAP_GET_PRFD( fds[i].lpoll_socketarg ); } that seems to take a valid fds[i].lpoll_socketarg and corrupt it and pds[i].fd not sure why. prldap_poll(ldap_x_pollfd * 0x03d89a70, int 5, int 1000, lextiof_session_private * 0x02cc2940) line 251 nsldapi_iostatus_poll(ldap * 0x02cc26f0, timeval * 0x052dfbb0) line 1180 + 46 bytes wait4msg(ldap * 0x02cc26f0, int -1, int 0, int 1, timeval * 0x052dff34, ldapmsg * * 0x052dff28) line 351 + 13 bytes nsldapi_result_nolock(ldap * 0x02cc26f0, int -1, int 0, int 1, timeval * 0x052dff34, ldapmsg * * 0x052dff28) line 127 + 29 bytes ldap_result(ldap * 0x02cc26f0, int -1, int 0, timeval * 0x052dff34, ldapmsg * * 0x052dff28) line 93 + 27 bytes nsLDAPConnectionLoop::Run(nsLDAPConnectionLoop * const 0x02cc5930) line 972 + 24 bytes nsThread::Main(void * 0x02cc57f0) line 120 + 26 bytes _PR_NativeRunThread(void * 0x02cc55d0) line 413 + 13 bytes _threadstartex(void * 0x02cc54d0) line 212 + 13 bytes KERNEL32! 77e92ca8()

a patch that works. I'm doing all that ugly copy and pasting, just to override the call to PR_OpenTCPSocket() with my call to sslSocketProvider->NewSocket(). note, this patch puts up some PSM ui, since I do the NewSocket() with the ip address of the host.

this patch also assumes that port 636 and only that port is secure, as a proof of concept hack.

what comes next: 1. work with mcs, can we extend the ldap sdk code to allow me to just override the PR_OpenTCPSocket() call? 2. alternatively, could I use sslSocketProvider->AddToSocket()? what if I call the prldap_connect() from my override, and then call AddToSocket()? or is it too late (I think I tried this, and it was too late but I should double check, it might save us from having to do #1)

The libprldap layer within the LDAP C SDK was not designed with the requirement that someone would need to replace the PR_OpenTCPSocket() call. After all, if you are using NSPR, that is the standard call. We could add a hook (callback?) specifically to allow you to override PR_OpenTCPSocket(), but that will be ugly from an API point of view. Of course it is even uglier to have to copy the libprldap code. I like the AddToSocket() idea. I don't really know what you need to do that is different from PR_OpenTCPSocket(), but I assume AddToSocket() takes a PRFileDesc * and does the "right things" to it. Essentially, a socket import kind of method? But that probably needs to happen before the TCP connection is opened.... NSPR also has an I/O layers concept that allows you to override all of the methods. Do you use that underneath sslSocketProvider->NewSocket? If so, maybe libprldap could be modified to let you add your own I/O layer to the stack.

sorry for the delay mcs, I'll do some research soon and try to answer your questions.

mcs / putterman / dmose: still haven't gotten back to this. now that we've switch to LDAP SDK 5.0, this will get some attention.

This is the first cut at a hook into libprldap which will hopefully allow us to use AddToSocket() as discussed previously. Not yet tested (look, Ma, it compiles!).

while dmose pursues the cleaner fix, I'm going to hedge our bets with the cut-and-paste fix. the change between this patch and the last one is that I get and pass through options to all callers to Init(). For example, it now does LDAP over SSL during autocomplete.

and LDAP replication over SSL.

Adding myself.

Comment on attachment 77767 [details] [diff] [review] same as before, but keep the hostname in the session so that we can use it for AddToSocket(), and no more the man-in-the-middle warning UI. Excellent! I reviewed the parts where you interface with libldap and libprldap (nsLDAPConnection.cpp, mainly). Remove the PRLDAPIOSessionArg and PRLDAPIOSocketArg typedefs (you are not using them). Remove the fields from the LDAPSSLSessionInfo struct that are not used (you only use hostname and lssei_std_functions). Use consistent names (I would rename the entire struct, but I am probably one of only two or three people who might look at the libssldap code -- not used in the mozilla client -- and be confused by the similarities here). Avoid the MAXHOSTNAMELEN dillema and just strdup() the hostname and free it when you dispose of the LDAPSSLSessionInfo struct. Speaking of that... I think you need to override the prldap close callback so you can free the LDAPSSLSessionInfo struct that is allocated inside secure_ldap_install_routines() and the LDAPSSLSocketInfo struct that is allocated inside my_prldap_connect(). Get rid of either ldapssl_connect() or my_prldap_connect() -- they take the same arguments and one is called from the other.

> Avoid the MAXHOSTNAMELEN dillema and just strdup() the hostname and free it > when you dispose of the LDAPSSLSessionInfo struct. yes, that's just a temporary hack to show that we can cleanly pass the hostname around, so that when we call AddToSocket(), we can use the hostname and avoid the man-in-the-middle warning UI we got when using the IP address. dmose has plans to clean that up, override close(), improve how we're passing options, fix any leaks, etc.

Addressed mcs' comments; cleaned up various leaks; asserted local style (naming, comments, warnings, etc); used nsMemory::Alloc where practical for memory pressure observer goodness. I suspect we may want to take out the "user name / login" UI for Mozilla 1.0, since that doesn't yet work.

What's the ETA for this landing? Can you please add it to the Status Whiteboard? Thanks!

looks good. two comments: 1) + sessionClosure->hostname = PL_strdup(aHostName); + if (!sessionClosure->hostname) { + NS_WARNING("nsLDAPInstallSSL(): PL_strdup failed\n"); + } Isn't this an out of memory error? 2) about the UI changes: we need to get srilatha's new patch for #120432 that has the port and the secure checkbox on the same tab. (note, it's in her local tree, not in the bug yet.) We'd want to take her newest patch, and hide the "use username and password" checkbox.

I updated the patch in bug 120432.

Comment on attachment 78155 [details] [diff] [review] AddToSocket patch, v5 r=mcs for the nsLDAPSecurityGlue.cpp file. For the issue of multiple IP addresses for one SSL hostname, I think there is no problem -- the important thing is that the hostname in whatever cert is returned must match that used by the client to find the IP addresses. But that is the server's problem. At least that is my understanding.

I'll wait until we've got an official final patch (with srilatha's new UI patch) before I sr=, but that's just a formality. The code looks good.

Updated to address sspitzer's comment #1 and incorpated with srilatha's latest UI patch (but with username/login disabled).

Includes some Mac build changes.

Fixes a bug in the mozldap makefile.win.

Changes one error to a warning on sspitzer's advice.

Comment on attachment 78286 [details] [diff] [review] AddToSocket patch, v9 sr=sspitzer, I tested this on windows as well. assuming smime works on mac still. (I saw LDAP over SSL work in dmose's cube)

mozilla, win bits for QA: http://warp/u/sspitzer/moz-ldap-ssl.zip

ns, win bits for QA: http://warp/u/sspitzer/ns-ldap-ssl.zip

reviewed all (except nsLDAPSecurityGlue.cpp which seems to be reviewed by experts). Very minor nits : 1. var gMaxHits = 100; in pref-directory-add.js -> Do we change this value or is this a const. If const, then declare as const. 2. style="width: 36em" in pref-directory-add.xul css file -> Is there a css files for this file. You may want to add the above style to the id addDirectory i.e., the dialog. r=bhuvan

Changed var gMaxHits to be const kDefaultMaxHits. After discussion with sspitzer, moved the directory width specification to the DTD file instead of the theme CSS, since people who are writing themes shouldn't be forced to know about this.

Comment on attachment 78300 [details] [diff] [review] AddToSocket patch, v10 Carrying forward r=bhuvan,mcs sr=sspitzer.

here are the mac and linux ldap over ssl bits: http://warp/u/dmose/mac-moz-ldapssl-0408.sea.hqx http://warp/u/dmose/mozilla-ldapssl-i686-pc-linux7.2-redhat.tar.gz

Adding adt1.0.0 because this now has reviews (r=bhuvan,mcs sr=sspitzer), and is desired by the ADT to land in 1.0.

adt1.0.0+ (on ADT's behalf) approval for checkin into 1.0, pending postive outcome of testing by QA.

Merged to CVS tip at branch time.

Comment on attachment 78431 [details] [diff] [review] AddToSocket patch, v11 Carrying forward r=bhuvan,mcs; sr=sspitzer.

Javi, in the latest patch, there are three changes to mac/macbuild files, can you please review them?

win, mac and linux ldap over ssl bits from Dan and Seth are all working properly for LDAP Address Book searching and autocomplete.

From the update of (id=78481) r=rdayal.

Comment on attachment 78481 [details] [diff] [review] XML project file diffs for Mac r=rdayal, sr=sspitzer

Mac related PSM patches for using MOZ_PSM in conjunction with ENABLE_SMIME look good. r=javi on those.

Comment on attachment 78431 [details] [diff] [review] AddToSocket patch, v11 a=asa (on behalf of drivers) for checkin to the 1.0 branch.

Comment on attachment 78481 [details] [diff] [review] XML project file diffs for Mac a=asa (on behalf of drivers) for checkin to the 1.0 branch.

Checked in on both the trunk and branch.

*** Bug 104084 has been marked as a duplicate of this bug. ***

Verified with 20020410 trunk builds on various platforms

20020323 branch build Verified.