You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2005/07/27 22:59:18 UTC
[PATCH] fix util_ldap with older OpenLDAPs
Since 2.0.54, it seems mod_auth_ldap just segfaults on any request if
built against older versions of OpenLDAP, 2.2.20 and earlier (pre-2005).
It looks like this was another regression caused the addition of the
LDAPConnectionTimeout option. (New features, stable branch,
regressions? Hmmm, I spot a pattern)
http://issues.apache.org/bugzilla/show_bug.cgi?id=34618 mentions this
with "upgrade OpenLDAP" as the solution, which isn't really a great way
to give your users that warm fuzzy feeling.
Any objections to this?
Index: modules/experimental/util_ldap.c
===================================================================
--- modules/experimental/util_ldap.c (revision 225591)
+++ modules/experimental/util_ldap.c (working copy)
@@ -50,7 +50,21 @@
#define LDAP_CA_TYPE_BASE64 2
#define LDAP_CA_TYPE_CERT7_DB 3
+#if APR_HAS_OPENLDAP_LDAPSDK
+#include <ldap_features.h>
+/* LDAP_OPT_NETWORK_TIMEOUT is broken in OpenLDAP < 2.2.21, see
+ * OpenLDAP bug "ITS 3487". */
+
+#if LDAP_VENDOR_VERSION_MAJOR < 2 || \
+ (LDAP_VENDOR_VERSION_MAJOR == 2 && LDAP_VENDOR_VERSION_MINOR < 2) || \
+ (LDAP_VENDOR_VERSION_MAJOR == 2 && LDAP_VENDOR_VERSION_MINOR == 2 \
+ && LDAP_VENDOR_VERSION_PATCH < 21)
+#undef LDAP_OPT_NETWORK_TIMEOUT
+#endif
+
+#endif /* APR_HAS_OPENLDAP_LDAPSDK */
+
module AP_MODULE_DECLARE_DATA ldap_module;
int util_ldap_handler(request_rec *r);
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Graham Leggett <mi...@sharp.fm>.
Jess Holle wrote:
> I can't argue with this -- as much as I want multiple-provider support
> right now. [2.0.54's LDAP largely seems to hold up overall -- as
> amazing as that seems...]
Most of the bugs in the LDAP code come about because of bad handling of
timed out connections - meaning you get problems on low or no load,
rather than on high load, which is a very frustrating thing to debug. I
would find a problem, see the server worked, throw a few thousand hits
at it to be sure, then get back the next day to find it was broken again
- connections had timed out in the mean time and broke the server again.
If there is a definite need, walk backwards through the patches to v2.2
and to apr-util to see what is fixed in each patch, I;m sure there are
some segfaults that can be found if someone has the time to walk through it.
> That and the delay from then until the broader audience that suddenly
> turns on it shakes out problems the smaller development community did
> not are the big questions.
>
> I'm still hoping our next product releases can incorporate 2.2...
I've been using v2.1.6 in production for a while (I needed the LDAP
fixes, so it was the only viable option) with no hassles.
If we get v2.2 out the door, I think we could ramp up to a stable and
trusted version pretty quickly.
Regards,
Graham
--
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Jess Holle <je...@ptc.com>.
Graham Leggett wrote:
> Jess Holle wrote:
>
>> Agreed, but some of us need an /officially /stable Apache with good
>> LDAP authentication support.
>
> The problem is that the v2.0.x code was not very defensive, which
> means a simple failure up front causes an obscure and random crash
> somewhere further down in the code. The fixes as a result were
> relatively large and difficult to review for v2.0.x. Some of the fixes
> involved new and changed interfaces to apr-util which were difficult
> to backport.
>
> As a result the v2.0.x code lags far behind v2.2, and backporting
> fixes is a not trivial task. I think the effort is better spent
> getting v2.2 out the door.
I can't argue with this -- as much as I want multiple-provider support
right now. [2.0.54's LDAP largely seems to hold up overall -- as
amazing as that seems...]
>> Apache 2.0.54 is the best thing out there in this regard, but as you
>> note it leaves a lot to be desired. Apache 2.2 seems infinitely far
>> away...
>
> v2.2 has been branched, not sure when the first actual release on this
> branch will be?
That and the delay from then until the broader audience that suddenly
turns on it shakes out problems the smaller development community did
not are the big questions.
I'm still hoping our next product releases can incorporate 2.2...
--
Jess Holle
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Graham Leggett <mi...@sharp.fm>.
Jess Holle wrote:
> Agreed, but some of us need an /officially /stable Apache with good LDAP
> authentication support.
The problem is that the v2.0.x code was not very defensive, which means
a simple failure up front causes an obscure and random crash somewhere
further down in the code. The fixes as a result were relatively large
and difficult to review for v2.0.x. Some of the fixes involved new and
changed interfaces to apr-util which were difficult to backport.
As a result the v2.0.x code lags far behind v2.2, and backporting fixes
is a not trivial task. I think the effort is better spent getting v2.2
out the door.
> Apache 2.0.54 is the best thing out there in this regard, but as you
> note it leaves a lot to be desired. Apache 2.2 seems infinitely far away...
v2.2 has been branched, not sure when the first actual release on this
branch will be?
Regards,
Graham
--
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Jess Holle <je...@ptc.com>.
Graham Leggett wrote:
> None at all - if the v2.0 code can be made more stable this is always
> a good thing, but there are lots more problems in the v2.0 code that
> are fixed in the v2.2 rewrite.
>
> Roll on httpd v2.2.
Agreed, but some of us need an /officially /stable Apache with good LDAP
authentication support.
Apache 2.0.54 is the best thing out there in this regard, but as you
note it leaves a lot to be desired. Apache 2.2 seems infinitely far away...
--
Jess Holle
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Jul 27, 2005 at 11:39:36PM +0200, Graham Leggett wrote:
> Joe Orton wrote:
> >Any objections to this?
>
> None at all - if the v2.0 code can be made more stable this is always a
> good thing, but there are lots more problems in the v2.0 code that are
> fixed in the v2.2 rewrite.
Ah, sorry, I'd presumed this would affect 2.2 as well since the timeout
handling there looked much the same, but it seems not (though I don't
claim to understand why :). I'll just add this to STATUS for 2.0.x in
that case.
joe
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Graham Leggett <mi...@sharp.fm>.
Joe Orton wrote:
> Since 2.0.54, it seems mod_auth_ldap just segfaults on any request if
> built against older versions of OpenLDAP, 2.2.20 and earlier (pre-2005).
> It looks like this was another regression caused the addition of the
> LDAPConnectionTimeout option. (New features, stable branch,
> regressions? Hmmm, I spot a pattern)
The LDAP code in v2.0.x is experimental, so is cannot be expected to be
as stable as the rest of the code.
To be more specific, a lot of the v2.0 code was rewritten and refactored
for v2.2 and was not backported to v2.0 as the new code depended on
APR v1.0.
> http://issues.apache.org/bugzilla/show_bug.cgi?id=34618 mentions this
> with "upgrade OpenLDAP" as the solution, which isn't really a great way
> to give your users that warm fuzzy feeling.
The "experimental" tag on the LDAP code should have dispensed with any
warm fuzzy feelings from the outset.
> Any objections to this?
None at all - if the v2.0 code can be made more stable this is always a
good thing, but there are lots more problems in the v2.0 code that are
fixed in the v2.2 rewrite.
Roll on httpd v2.2.
Regards,
Graham
--
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Jul 27, 2005 at 09:59:18PM +0100, Joe Orton wrote:
> Since 2.0.54, it seems mod_auth_ldap just segfaults on any request if
> built against older versions of OpenLDAP, 2.2.20 and earlier (pre-2005).
I worked this out a little better. It triggers only the *second* time
the LDAP connection is opened for a given process. I think I must have
started testing the 2.0.x code with slapd stopped and saw this being
triggered by the retry-10-times logic for every request.
Brad, can you explain why the ldap_set_option() call is used to change
the *process-global* connection timeout setting in the 2.0.x code,
rather than the connection-specific setting like the trunk code does?
Doing that seems generally undesirable as well as triggering the
OpenLDAP bug. Is it because some SDKs don't handle per-connection
settings, perhaps?
If so, this would be a a simpler, better fix for the issue:
Index: modules/experimental/util_ldap.c
===================================================================
--- modules/experimental/util_ldap.c (revision 227189)
+++ modules/experimental/util_ldap.c (working copy)
@@ -325,7 +325,11 @@
}
if (st->connectionTimeout >= 0) {
+#if APR_HAS_OPENLDAP_LDAPSDK
+ rc = ldap_set_option(ldc->ldap, LDAP_OPT_NETWORK_TIMEOUT, (void *)&timeOut);
+#else
rc = ldap_set_option(NULL, LDAP_OPT_NETWORK_TIMEOUT, (void *)&timeOut);
+#endif
if (APR_SUCCESS != rc) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
"LDAP: Could not set the connection timeout" );
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by Graham Leggett <mi...@sharp.fm>.
William A. Rowe, Jr. said:
> +1 to this fix.
>
> Folks, either agree the code is correct, disagree that it should
> be some other way, identify it's bugs, or hush up. Plenty of
> people ARE using 2.2 ldap auth today - and there is no reason
> to stand in the way of committing obvious bug fixes, especially
> for recently modified code that was just wrong.
I'm not sure where the impression was gained that anybody is standing in
the way of fixing bugs in v2.0? I gave it a +1, Joe a +1 by submitting it,
and you a +1 above. That's three.
Bugs were fixed in v2.2 due to a rewrite that spanned APR v1.1 and httpd,
making the fixes neither atomic nor easy to backport. That said, if people
want to use v2.0, and if people are keen to fix the remaining bugs in
v2.0, then ++1 to that. Patches posted will definitely not be blocked.
Regards,
Graham
--
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 08:21 AM 8/2/2005, William A. Rowe, Jr. wrote:
>+1 to this fix.
>
>Folks, either agree the code is correct, disagree that it should
>be some other way, identify it's bugs, or hush up. Plenty of
>people ARE using 2.2 ldap auth today - and there is no reason
>to stand in the way of committing obvious bug fixes, especially
>for recently modified code that was just wrong.
s/2.2/2.0/ of course :)
<runs off for more Coffee>
Re: [PATCH] fix util_ldap with older OpenLDAPs
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
+1 to this fix.
Folks, either agree the code is correct, disagree that it should
be some other way, identify it's bugs, or hush up. Plenty of
people ARE using 2.2 ldap auth today - and there is no reason
to stand in the way of committing obvious bug fixes, especially
for recently modified code that was just wrong.
Once 2.2 has run around the block a few times, most users will
pick it up to close such bugs. But this involves reconfiguration,
and the users who would appreciate if we would just fix the bug
aren't looking to be beta testers. Holding up segfaults as guns
to their heads, attempting to force them to 2.1-unstable isn't cool.
Bill
At 03:59 PM 7/27/2005, Joe Orton wrote:
>Since 2.0.54, it seems mod_auth_ldap just segfaults on any request if
>built against older versions of OpenLDAP, 2.2.20 and earlier (pre-2005).
>It looks like this was another regression caused the addition of the
>LDAPConnectionTimeout option. (New features, stable branch,
>regressions? Hmmm, I spot a pattern)
>
>http://issues.apache.org/bugzilla/show_bug.cgi?id=34618 mentions this
>with "upgrade OpenLDAP" as the solution, which isn't really a great way
>to give your users that warm fuzzy feeling.
>
>Any objections to this?
>
>Index: modules/experimental/util_ldap.c
>===================================================================
>--- modules/experimental/util_ldap.c (revision 225591)
>+++ modules/experimental/util_ldap.c (working copy)
>@@ -50,7 +50,21 @@
> #define LDAP_CA_TYPE_BASE64 2
> #define LDAP_CA_TYPE_CERT7_DB 3
>
>+#if APR_HAS_OPENLDAP_LDAPSDK
>+#include <ldap_features.h>
>
>+/* LDAP_OPT_NETWORK_TIMEOUT is broken in OpenLDAP < 2.2.21, see
>+ * OpenLDAP bug "ITS 3487". */
>+
>+#if LDAP_VENDOR_VERSION_MAJOR < 2 || \
>+ (LDAP_VENDOR_VERSION_MAJOR == 2 && LDAP_VENDOR_VERSION_MINOR < 2) || \
>+ (LDAP_VENDOR_VERSION_MAJOR == 2 && LDAP_VENDOR_VERSION_MINOR == 2 \
>+ && LDAP_VENDOR_VERSION_PATCH < 21)
>+#undef LDAP_OPT_NETWORK_TIMEOUT
>+#endif
>+
>+#endif /* APR_HAS_OPENLDAP_LDAPSDK */
>+
> module AP_MODULE_DECLARE_DATA ldap_module;
>
> int util_ldap_handler(request_rec *r);