From dev-return-42277-apmail-httpd-dev-archive=httpd.apache.org@httpd.apache.org Wed May 12 21:25:37 2004 Return-Path: Delivered-To: apmail-httpd-dev-archive@www.apache.org Received: (qmail 91835 invoked from network); 12 May 2004 21:25:37 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur-2.apache.org with SMTP; 12 May 2004 21:25:37 -0000 Received: (qmail 461 invoked by uid 500); 12 May 2004 21:25:57 -0000 Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 339 invoked by uid 500); 12 May 2004 21:25:56 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: list-post: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 271 invoked by uid 98); 12 May 2004 21:25:55 -0000 Received: from BNICHOLES@novell.com by hermes.apache.org by uid 82 with qmail-scanner-1.20 (clamuko: 0.70. Clear:RC:0(137.65.81.169):. Processed in 0.07508 secs); 12 May 2004 21:25:55 -0000 X-Qmail-Scanner-Mail-From: BNICHOLES@novell.com via hermes.apache.org X-Qmail-Scanner: 1.20 (Clear:RC:0(137.65.81.169):. Processed in 0.07508 secs) Received: from unknown (HELO sinclair.provo.novell.com) (137.65.81.169) by hermes.apache.org with SMTP; 12 May 2004 21:25:55 -0000 Received: from INET-PRV-MTA by sinclair.provo.novell.com with Novell_GroupWise; Wed, 12 May 2004 15:25:19 -0600 Message-Id: X-Mailer: Novell GroupWise Internet Agent 6.5.2 Beta Date: Wed, 12 May 2004 15:25:06 -0600 From: "Brad Nicholes" To: Cc: , Subject: Re: [util_ldap.c] Your recent correction in util_ldap_cache_checkuserid(). Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part3E1FD122.0__=" X-Spam-Rating: hermes.apache.org 1.6.2 0/1000/N X-Spam-Rating: minotaur-2.apache.org 1.6.2 0/1000/N This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part3E1FD122.0__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline 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 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 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 > > --=__Part3E1FD122.0__= Content-Type: application/octet-stream; name="util_ldap.c.patch1" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="util_ldap.c.patch1" SW5kZXg6IG1vZHVsZXMvZXhwZXJpbWVudGFsL3V0aWxfbGRhcC5jDQo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQpSQ1Mg ZmlsZTogL2hvbWUvY3ZzL2h0dHBkLTIuMC9tb2R1bGVzL2V4cGVyaW1lbnRhbC91dGlsX2xkYXAu Yyx2DQpyZXRyaWV2aW5nIHJldmlzaW9uIDEuNi4yLjEzDQpkaWZmIC11IC1yMS42LjIuMTMgdXRp bF9sZGFwLmMNCi0tLSBtb2R1bGVzL2V4cGVyaW1lbnRhbC91dGlsX2xkYXAuYwkyNiBBcHIgMjAw NCAyMjowNDo1OSAtMDAwMAkxLjYuMi4xMw0KKysrIG1vZHVsZXMvZXhwZXJpbWVudGFsL3V0aWxf bGRhcC5jCTEyIE1heSAyMDA0IDIxOjE5OjM0IC0wMDAwDQpAQCAtMzMzLDkgKzMzMyw2IEBADQog ICAgICAgICAgICAgYnJlYWs7DQogICAgIH0NCiANCi0gICAgbGRjLT5ib3VuZCA9IDE7DQotICAg IGxkYy0+cmVhc29uID0gIkxEQVA6IGNvbm5lY3Rpb24gb3BlbiBzdWNjZXNzZnVsIjsNCi0NCiAg ICAgLyogZnJlZSB0aGUgaGFuZGxlIGlmIHRoZXJlIHdhcyBhbiBlcnJvcg0KICAgICAqLw0KICAg ICBpZiAoTERBUF9TVUNDRVNTICE9IHJlc3VsdCkNCkBAIC0zNDUsNiArMzQyLDEwIEBADQogICAg ICAgICBsZGMtPmJvdW5kID0gMDsNCiAgICAgICAgIGxkYy0+cmVhc29uID0gIkxEQVA6IGxkYXBf c2ltcGxlX2JpbmRfcygpIGZhaWxlZCI7DQogICAgIH0NCisJZWxzZSB7DQorCQlsZGMtPmJvdW5k ID0gMTsNCisJCWxkYy0+cmVhc29uID0gIkxEQVA6IGNvbm5lY3Rpb24gb3BlbiBzdWNjZXNzZnVs IjsNCisJfQ0KIA0KICAgICByZXR1cm4ocmVzdWx0KTsNCiB9DQpAQCAtODc1LDYgKzg3Niw5IEBA DQogICAgIGlmIChyZXN1bHQgIT0gTERBUF9TVUNDRVNTKSB7DQogICAgICAgICBsZGMtPnJlYXNv biA9ICJsZGFwX3NpbXBsZV9iaW5kX3MoKSB0byBjaGVjayB1c2VyIGNyZWRlbnRpYWxzIGZhaWxl ZCI7DQogICAgICAgICBsZGFwX21zZ2ZyZWUocmVzKTsNCisgICAgICAgIGxkYXBfdW5iaW5kX3Mo bGRjLT5sZGFwKTsNCisgICAgICAgIGxkYy0+bGRhcCA9IE5VTEw7DQorICAgICAgICBsZGMtPmJv dW5kID0gMDsNCiAgICAgICAgIHJldHVybiByZXN1bHQ7DQogICAgIH0NCiAgICAgZWxzZSB7DQo= --=__Part3E1FD122.0__=--