Project

General

Profile

Actions

Bug #2849

closed

Linux OOM kills lighttpd when using mod_authn_ldap

Added by codehero almost 7 years ago. Updated over 5 years ago.

Status:
Fixed
Priority:
Normal
Category:
mod_auth
Target version:
ASK QUESTIONS IN Forums:

Description

Yes, this is clearly a memory leak but whether it is in mod_authn_ldap or openldap is still to be determined.

In stress testing lighttpd 1.4.47 on ARM Linux 3.1.8 with many concurrent GETs requiring ldap auth, I observed the memory footprint increase very quickly.

I tested the following to isolate to ldap:

1. Stress test on URL requiring no auth: small increase memory footprint
2. Stress test on URL requiring basic auth using plain user file: small increase in memory footprint
3. Stress test on URL requiring basic auth using ldap: very fast increase in memory footprint and eventual invocation of the Linux OOM.

Thus I can limit the scope of the "leak" to mod_authn_ldap.c

I will keep investing further on the ultimate cause.
As this is an embedded system, valgrind is not an option.


Files

Actions #1

Updated by codehero almost 7 years ago

Further information:

For tests 1-3, LDAP server was running on local system

4. Stress test on another ARM system, LDAP server remote: no leak
5. Stress test on an x86_64 system, LDAP server remote: no leak
6. Stress test on another ARM system, LDAP server local: leak

I will try to get a test going on x86_64 system with a local ldap server to see if the leak manifests itself there.

Actions #2

Updated by codehero almost 7 years ago

The following line is the culprit when LDAP server is on the same host as lighttpd:

auth.backend.ldap.hostname = "127.0.0.1"

changing this to

auth.backend.ldap.hostname = "192.168.1.1" (or the IP of the eth0 interface)

eliminates the memory leak.

Actions #3

Updated by codehero almost 7 years ago

Yes, there is a real "memory leak" in lighttpd.

I was able to recreate the leaking behaviour on an x86_64 desktop.
Attached is a valgrind log clearly showing unreachable bytes.

Further inspection of the mod_authn_ldap.c shows the incorrect function for freeing variables of type LDAP*

ldap_destroy() must be used instead of ldap_memfree() to free types of LDAP*

eg

The way to trigger the leak is to just repeatedly try to login with bad credentials.
Please patch immediately on your end as this is a vulnerability on internal systems.

(I doubt anyone would use ldap on an internet facing system, I could be wrong)

Actions #4

Updated by gstrauss almost 7 years ago

  • Status changed from New to Patch Pending
  • Target version changed from 1.4.x to 1.4.49

Thank you for the trace. Something is definitely amiss.
'man ldap_destroy' instructs

       When ldap_unbind() is called on a session handle with siblings, all the
       siblings become invalid.

       Siblings  must  be  destroyed  using  ldap_destroy().   Session  handle
       resources  associated with the original (LDAP *) will be freed when the
       last session handle is destroyed or when ldap_unbind() is called, if no
       other session handles currently exist.

and ldap_unbind_ext_s() is used on the handle when ldap_bind() succeeds, but not when the ldap_sasl_bind_s() fails.

Untested, but here is the 5 min patch to call ldap_destroy() (instead of ldap_memfree()) after a successful ldap_initialize(), even if the ldap_sasl_bind_s() fails.

diff --git a/src/mod_authn_ldap.c b/src/mod_authn_ldap.c
index c2e793d1..3e75cf10 100644
--- a/src/mod_authn_ldap.c
+++ b/src/mod_authn_ldap.c
@@ -398,7 +398,7 @@ static LDAP * mod_authn_ldap_host_init(server *srv, plugin_c
onfig *s) {
     ret = ldap_set_option(ld, LDAP_OPT_PROTOCOL_VERSION, &ret);
     if (LDAP_OPT_SUCCESS != ret) {
         mod_authn_ldap_err(srv, __FILE__, __LINE__, "ldap_set_options()", ret);
-        ldap_memfree(ld);
+        ldap_destroy(ld);
         return NULL;
     }

@@ -412,7 +412,7 @@ static LDAP * mod_authn_ldap_host_init(server *srv, plugin_c
onfig *s) {
                 mod_authn_ldap_err(srv, __FILE__, __LINE__,
                                    "ldap_set_option(LDAP_OPT_X_TLS_CACERTFILE)" 
,
                                    ret);
-                ldap_memfree(ld);
+                ldap_destroy(ld);
                 return NULL;
             }
         }
@@ -420,7 +420,7 @@ static LDAP * mod_authn_ldap_host_init(server *srv, plugin_c
onfig *s) {
         ret = ldap_start_tls_s(ld, NULL,  NULL);
         if (LDAP_OPT_SUCCESS != ret) {
             mod_authn_ldap_err(srv,__FILE__,__LINE__,"ldap_start_tls_s()",ret);
-            ldap_memfree(ld);
+            ldap_destroy(ld);
             return NULL;
         }
     }
@@ -490,7 +490,7 @@ static LDAPMessage * mod_authn_ldap_search(server *srv, plugin_config *s, char *
                             s->auth_ldap_bindpw->ptr)
       : mod_authn_ldap_bind(srv, s->ldap, NULL, NULL);
     if (LDAP_SUCCESS != ret) {
-        ldap_memfree(s->ldap);
+        ldap_destroy(s->ldap);
         s->ldap = NULL;
         return NULL;
     }
@@ -635,7 +635,7 @@ static handler_t mod_authn_ldap_basic(server *srv, connection *con, void *p_d, c
     }

     if (LDAP_SUCCESS != mod_authn_ldap_bind(srv, ld, dn, pw)) {
-        ldap_memfree(ld);
+        ldap_destroy(ld);
         if (dn != p->ldap_filter->ptr) ldap_memfree(dn);
         return HANDLER_ERROR;
     }
diff --git a/src/mod_vhostdb_ldap.c b/src/mod_vhostdb_ldap.c
index 459d7d60..4f7e2a5f 100644
--- a/src/mod_vhostdb_ldap.c
+++ b/src/mod_vhostdb_ldap.c
@@ -251,7 +251,7 @@ static LDAP * mod_authn_ldap_host_init(server *srv, vhostdb_config *s) {
     ret = ldap_set_option(ld, LDAP_OPT_PROTOCOL_VERSION, &ret);
     if (LDAP_OPT_SUCCESS != ret) {
         mod_authn_ldap_err(srv, __FILE__, __LINE__, "ldap_set_options()", ret);
-        ldap_memfree(ld);
+        ldap_destroy(ld);
         return NULL;
     }

@@ -264,7 +264,7 @@ static LDAP * mod_authn_ldap_host_init(server *srv, vhostdb_config *s) {
                 mod_authn_ldap_err(srv, __FILE__, __LINE__,
                                    "ldap_set_option(LDAP_OPT_X_TLS_CACERTFILE)",
                                    ret);
-                ldap_memfree(ld);
+                ldap_destroy(ld);
                 return NULL;
             }
         }
@@ -272,7 +272,7 @@ static LDAP * mod_authn_ldap_host_init(server *srv, vhostdb_config *s) {
         ret = ldap_start_tls_s(ld, NULL,  NULL);
         if (LDAP_OPT_SUCCESS != ret) {
             mod_authn_ldap_err(srv,__FILE__,__LINE__,"ldap_start_tls_s()",ret);
-            ldap_memfree(ld);
+            ldap_destroy(ld);
             return NULL;
         }
     }
@@ -338,7 +338,7 @@ static LDAPMessage * mod_authn_ldap_search(server *srv, vhostdb_config *s, char

     ret = mod_authn_ldap_bind(srv, s->ldap, s->binddn, s->bindpw);
     if (LDAP_SUCCESS != ret) {
-        ldap_memfree(s->ldap);
+        ldap_destroy(s->ldap);
         s->ldap = NULL;
         return NULL;
     }

Actions #5

Updated by codehero almost 7 years ago

I implemented almost exactly the same patch yesterday (sans mod_vhostdb_ldap.c changes) and experienced no crashes or memory leaks on both my x86_64 and my target system.

Actions #6

Updated by gstrauss almost 7 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100
Actions #7

Updated by ef over 5 years ago

I think ldap_destroy() is not the best solution, ldap_unbind() woud be preferable.
Using ldap_destroy() silently adds a dependency on OpenLDAP>=2.4 and will mysteriously fail on earlier versions.
In a mail (https://www.openldap.org/lists/openldap-technical/201202/msg00452.html) on openldap-technical (probably predating the introduction of ldap_dup()/ldap_destroy()), Hallvard B Furuseth wrote

Note that ldap_unbind() is misnamed, it should have been
called ldap_destroy().  It does send an unbind, but the more important
part is that it destroys the LDAP*.

Actions #8

Updated by gstrauss over 5 years ago

@ef It does not appear that you know what you're talking about. Kindly refrain from sharing uninformed opinions.

Read the man page for ldap_destroy(). Don't continuing posting to this ticket. This conversation is over.

https://github.com/openldap/openldap/commit/0f30db1c46cf576230cd9397f0d5ad2892da7325

Actions

Also available in: Atom