httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brad Nicholes" <BNICHO...@novell.com>
Subject Re: [util_ldap.c] Your recent correction in util_ldap_cache_checkuserid().
Date Wed, 12 May 2004 21:25:06 GMT
   I'm not sure that I understand what the problem is.  By updating the
binddn/bindpw in the connection record, we always know exactly which
userid the connection is bound to.  During the
util_ldap_connection_find() routine there are two searches performed. 
The first search looks for a connection within the cache that exactly
matches the connection credentials that were passed in.  If a connection
is found, it is used without the overhead of having to rebind the
connection.  If an exact connection is not found, then a second search
is performed looking for a connection that matches the connection
attributes but not the credentials.  If a connection is found that
matches the connection attributes then the l->bound flag is turned off
which forces the connection to be rebound to the correct user during the
call to util_ldap_conection_open() routine.  The advantage to this is
avoiding unnecessary rebinds when the credential caching is turned off. 
The patch you submitted rebinds the connection always whether it needs
to be or not.
   But as a result, I did find one problem with the current code that
does have an affect when a bind fails during the
util_ldap_cache_checkuserid() processing.  If the bind fails while
validating the user credentials then the connection is left in an
unbound state although the connection record still indicates that the
connection is bound to a specific user.  This could cause problems for a
subsequent ldap search.  This patch unbinds the connection which will
force it to be reinitialized the next time that it is used.

Brad 


Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> Denis Gervalle <dge@softec.st> Monday, May 10, 2004 3:33:08 PM >>>
Brad,

After a second look to util_ldap patches, I notice that my original
mail 
is already about r1.22 which only solve the problem half way. I have 
tried to held your attention on that problem and you seems to have 
missed the point. Patch 1.22 has still a serious flaw when a wrong 
binddn/bindpw are provided. IMHO, my patch  proposal (Bug 27134), which

delegate all binding operation to the single util_ldap_connection_open

function, avoids all possible trap that could messed up the connection

cache now and in the future. Hopeless, it is now no more applicable 
as-is to the cvs tree.

However, my patch has been test against version 1.24 from the cvs. 
Albert Lunde who did these tests have provided his conclusions under
the 
same bug number (Bug 27134). These results (for which he send me a 
detailled report) shown that version 1.24 (which obviously include 1.22

patch) is entirely unreliable. We are still in close relation about 
these problems, and he recently confirm me that my proposal provide him

good results with a number of connections proportional (as expected) 
with the number of process involved in prefork mode.

Hope that you will have a closer look to my patch this time.
Do not hesitate too contact me if you need more information.
Best regards,

Denis Gervalle
SOFTEC sa
http://www.softec.st 

Brad Nicholes wrote:

>   There are still 2 more patches for util_ldap.c that are proposed
for
>backport to the 2.0 tree.  These are r1.22 and r1.24.  These 2
patches
>should fix the problem that you discribed.  Once the backport
proposal
>recieves a third +1 vote, I will apply them to the 2.0 tree.
>
>Brad
>
>Brad Nicholes
>Senior Software Engineer
>Novell, Inc., the leading provider of Net business solutions
>http://www.novell.com 
>
>  
>
>>>>Denis Gervalle <dge@softec.st> Tuesday, April 20, 2004 4:04:47 PM
>>>>
>>>>        
>>>>
>Brad,
>
>May I suggest you to have a look at bug 27134 that provide a patch 
>regarding the rebinding of the connection during 
>util_ldap_cache_checkuserid().
>The recent correction you provide in the CVS of apache 2.0 has a flaw

>when the binding fails (because of wrong binddn or password). In that

>case, the util_ldap_connection_open will not repair the connection
>since 
>it will appear to be still bound. In my patch proposal, I prefer to 
>delegate the bind to util_ldap_connection_open which make it the only

>place for connection binding. It also take care of retries in case of

>server down.
>Hope this helps,
>Regards,
>
>Denis Gervalle
>SOFTEC
>  
>

Mime
View raw message