mhardin@symas.com wrote:
Full_Name: Matthew Hardin Version: 2.3.38 OS: N/A URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (12.168.121.2)
Multiple binds on the same connection where at least one bind ends up being passed through to a target and where the credentials for that bind are incorrect will cause back-meta to throw an assertion failure during a subsequent bind. In this situation back-meta prints the message "slapd: bind.c:229: meta_back_bind: Assertion `tmpmc == mc' failed".
Analysis: In bind.c there is no cleanup of the metaconn structure after a failed bind to a target database. This is ordinarily not a problem, as most applications perform an unbind immediately after a failed bind and so fail to trigger this bug. However, applications such as pam_ldap will perform several binds to as many as two different identities on the same connection, and the credentials for one of those identities are routinely incorrect. This bug causes the metaconn structure from the failed bind to be left in the mi_conninfo.lai_tree avl tree and then found during the re-insertion of a metaconn structure from a subsequent bind (approx between lines 209-258 in bind.c), causing the assertion failure and killing slapd in the process.
Note: This bug can be exploited as a DOS attack, as the behavior is relatively easy to reproduce on an unpatched system with a short perl script.
Proposed Fix: The simplest and best fix appears to be to mark the metaconn struct from the failed bind as tainted. This causes the struct to be deleted in the call to meta_back_release_conn around line 281.
The patch is as follows:
bind.c
*** 189,194 **** --- 189,198 ----
if ( lerr != LDAP_SUCCESS ) { rc = rs->sr_err = lerr;
/* Mark the meta_conn struct as tainted so* it'll be freed by meta_conn_back_destroy below */LDAP_BACK_CONN_TAINTED_SET( mc );/* FIXME: in some cases (e.g. unavailable) * do not assume it's not candidate; rather * mark this as an error to be eventually
I think the "real" fix should be different: binds should always receive a fresh connection from meta_back_getconn(), which shouldn't be put into the cache at all. meta_back_bind() should cache them only in case of success, otherwise they should be destroyed. This would allow to remove the need to set a BINDING flag to guarantee connections used for bind are not shared.
In the meanwhile, the solution you propose should be just fine, as it fills the hole occurring when a bind fails.
I'll work at that ASAP.
p.
Ing. Pierangelo Masarati OpenLDAP Core Team
SysNet s.r.l. via Dossi, 8 - 27100 Pavia - ITALIA http://www.sys-net.it --------------------------------------- Office: +39 02 23998309 Mobile: +39 333 4963172 Email: pierangelo.masarati@sys-net.it ---------------------------------------