noirello / bonsai

Simple Python 3 module for LDAP, using libldap2 and winldap C libraries.
MIT License
116 stars 32 forks source link

Problem with filedescriptorleak on ldapi-Connections when passing wrong credentials #45

Closed senfomat closed 3 years ago

senfomat commented 3 years ago

The Setup

We are using bonsai to authenticate OpenVPN-users via a Python-Flask-Webservice.

We are using:

For the authentication we have the following function:

def LDAPAuthenticate(username, password):
    client = LDAPClient("ldapi://%2Frun%2Fslapd%2Fldapi")
    client.set_credentials("SIMPLE", user=f"uid={username},cn=people,dc=domain.org", password=password)

    session = False
    try:
        client.connect():
    except AuthenticationError as err:
        log_error(f"LDAP-Authentication for '{username}' failed: {err}")
    except Exception as e:
        log_error(f"LDAP-Service failed: {e}")
    else:
        log_debug(f"LDAP-Authentication for '{username}' succeeded.")
        session = True

    return session

The Problem

When a users enters a wrong password within this authentication, every call to LDAPAuthenticate() leaks around 14 filedescriptors on the ldapi-Socketconnection, which gets not closed after the connect()-call. These connections are only closed, when the python3-webservice process exits or the slapd gets restarted.

When this wrong authentication is done a few hundred times (as some of our users have misconfigured clients), the ldap-daemon stops responding at all, as the number of maximum open filedescriptors is exhausted.

We have installed a workaround to restart the gunicorn-Webservice-Processes after 100 Requests. That we can not reach the fd-limit on the slapd.

Debuglogs

The connection-debug of a failed connection (using bonsai.set_debug(True)):

DBG: ldapconnection_new [self:0x7ff8b0ad0340]
DBG: ldapconnection_init (self:0x7ff8b0ad0340)
DBG: ldapconnection_open (self:0x7ff8b0ad0340)
DBG: connecting (self:0x7ff8b0ad0340)
DBG: create_conn_info (mech:SIMPLE, sock:-1, creds:0x7ff8b1dbcc40)
DBG: ldapconnectiter_new [self:0x7ff8b0ad6440]
DBG: create_init_thread_data (client:0x7ff8b1da1a00, sock:-1)
DBG: create_init_thread (ld:0x151a2e0, info:0x151a290, thread:0)
DBG: ldapconnection_result (self:0x7ff8b0ad0340, args:0x7ff8b0ad9c00, kwds:(nil))[msgid:-1]
DBG: LDAPConnection_Result (self:0x7ff8b0ad0340, msgid:-1, millisec:-1)
DBG: LDAPConnectIter_Next (self:0x7ff8b0ad6440, timeout:-1) [tls:0, state:0]
DBG: ldap_init_thread_func (params:0x151a2e0)DBG: _ldap_finish_init_thread (async:0, thread:140706090530560, timeout:-1, misc:0x151a2e0)
DBG: _pthread_mutex_timedlock
DBG: set connecting async: 1
DBG: ldap_init_thread_func [retval:0]
DBG: LDAPConnectIter_Next (self:0x7ff8b0ad6440, timeout:-1) [tls:0, state:0]
DBG: _ldap_finish_init_thread (async:0, thread:140706090530560, timeout:-1, misc:0x151a2e0)
DBG: _pthread_mutex_timedlock
DBG: set_certificates (self:0x7ff8b0ad6440)
DBG: binding [state:3]
DBG: _ldap_bind (ld:0x7ff8ac000b60, info:0x151a290, ppolicy:0, result:(nil), msgid:0)
DBG: LDAPConnectIter_Next (self:0x7ff8b0ad6440, timeout:-1) [tls:0, state:4]
DBG: binding [state:4]
DBG: ldapconnectiter_dealloc (self:0x7ff8b0ad6440)
DBG: dealloc_conn_info (info:0x151a290)
LDAP-Authentication for 'senfomat' failed: Invalid credentials. (0x0031 [49])
DBG: ldapconnection_dealloc (self:0x7ff8b0ad0340)

The connection with a working auth looks like this:

DBG: ldapconnection_new [self:0x7fb50112b340]
DBG: ldapconnection_init (self:0x7fb50112b340)
DBG: ldapconnection_open (self:0x7fb50112b340)
DBG: connecting (self:0x7fb50112b340)
DBG: create_conn_info (mech:SIMPLE, sock:-1, creds:0x7fb502417c40)
DBG: ldapconnectiter_new [self:0x7fb5011314e0]
DBG: create_init_thread_data (client:0x7fb502309b50, sock:-1)
DBG: create_init_thread (ld:0x1e34740, info:0x1e51290, thread:0)
DBG: ldapconnection_result (self:0x7fb50112b340, args:0x7fb501134b40, kwds:(nil))[msgid:-1]
DBG: LDAPConnection_Result (self:0x7fb50112b340, msgid:-1, millisec:-1)
DBG: LDAPConnectIter_Next (self:0x7fb5011314e0, timeout:-1) [tls:0, state:0]
DBG: _ldap_finish_init_thread (async:0, thread:140415381595904, timeout:-1, misc:0x1e34740)
DBG: _pthread_mutex_timedlock
DBG: ldap_init_thread_func (params:0x1e34740)
DBG: set connecting async: 1
DBG: ldap_init_thread_func [retval:0]
DBG: LDAPConnectIter_Next (self:0x7fb5011314e0, timeout:-1) [tls:0, state:0]
DBG: _ldap_finish_init_thread (async:0, thread:140415381595904, timeout:-1, misc:0x1e34740)
DBG: _pthread_mutex_timedlock
DBG: set_certificates (self:0x7fb5011314e0)
DBG: binding [state:3]
DBG: _ldap_bind (ld:0x7fb4fc002fc0, info:0x1e51290, ppolicy:0, result:(nil), msgid:0)
DBG: LDAPConnectIter_Next (self:0x7fb5011314e0, timeout:-1) [tls:0, state:4]
DBG: binding [state:4]
DBG: ldapconnectiter_dealloc (self:0x7fb5011314e0)
DBG: dealloc_conn_info (info:0x1e51290)
DBG: ldapconnection_dealloc (self:0x7fb50112b340)
LDAP-Authentication for 'senfotest' succeeded.

We are not sure, why the bonsai-Module is doing the connection in async mode. We do not want this, we left it on the default of False. But as the debuglog shows in the middle, the module is forcing it to async.

noirello commented 3 years ago

Hi, the default value for connect_async is True on Linux, you have to call bonsai.set_connect_async(False) explicitly to turn it off.

Is the IPC protocol (ldapi) affected only or the basic ones (ldap/ldaps) as well?

senfomat commented 3 years ago

Hi, the default value for connect_async is True on Linux, you have to call bonsai.set_connect_async(False) explicitly to turn it off.

Turning it off, even the succeeding call leak filedescriptors.

Is the IPC protocol (ldapi) affected only or the basic ones (ldap/ldaps) as well?

I just tested with ldap:// . It left the TCP-Connection open, until the script was exiting. And it leaked the succeeding calls as well, with our without the async-setting from above.

noirello commented 3 years ago

Thank you for the extra information, I'll look into it.

senfomat commented 3 years ago

When I change the try:-block like this:

     session = False
     try:
-        client.connect()
+        with client.connect():
+            pass
     except AuthenticationError as err:
         print(f"LDAP-Authentication for '{username}' failed: {err}")

the succeeding calls always get closed, even with ldap:// and with ldapi:// and async=True or async=False.

But no change for the auth-failed-Calls at all, these connections stay open.

noirello commented 3 years ago

Have you tried using a finally block?

def LDAPAuthenticate(username, password):
    client = LDAPClient("ldapi://%2Frun%2Fslapd%2Fldapi")
    client.set_credentials("SIMPLE", user=f"uid={username},cn=people,dc=domain.org", password=password)

    session = False
    try:
        conn = client.connect()
    except AuthenticationError as err:
        log_error(f"LDAP-Authentication for '{username}' failed: {err}")
    except Exception as e:
        log_error(f"LDAP-Service failed: {e}")
    else:
        log_debug(f"LDAP-Authentication for '{username}' succeeded.")
        session = True
    finally:
        conn.close()

    return session

Edit: On second thought: on error the variable bind will fail, and then conn will be an unbound variable in the finally block I suppose. But it seems your input narrows it down pretty well. On connection error the close function should be called on C's side during the clean up.

senfomat commented 3 years ago

Using your suggestion the failing connections get this stracktrace:

Traceback (most recent call last):
  File "./test.py", line 71, in <module>
    loginf()
  File "./test.py", line 41, in loginf
    print(LDAPAuthenticate(username, password))
  File "./test.py", line 33, in LDAPAuthenticate
    conn.close()
UnboundLocalError: local variable 'conn' referenced before assignment

the succeeding connections get closed as stated. but the failed connections gives a False as return-value, where close() is not available.

noirello commented 3 years ago

Added to call ldap_unbind_ext during LDAPConnection object's deallocation. That will handle freeing the file descriptors, allocated by the LDAP structure. Same LDAP API function is called during the LDAPConnection.close() method.

senfomat commented 3 years ago

The fix in the refactor-ci-branch works for us, no fd-leakage or open ports anymore. Thank you! (The ticket can get closed, but I dont know if you want to let it stay open until the merge into master)

noirello commented 3 years ago

Thank you for reporting it in such details.