You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Fritsch <sf...@sfritsch.de> on 2011/06/06 23:17:41 UTC

load order dependency between mod_ldap and mod_authnz_ldap

Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be 
loaded before mod_authnz_ldap. This is somewhat annoying because the 
default httpd.conf tries to load mod_authnz_ldap first. Any ideas how 
to fix this or do we just change the order in the default httpd.conf?

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Thursday 23 June 2011, William A. Rowe Jr. wrote:
> On 6/22/2011 3:13 PM, Stefan Fritsch wrote:
> > On Wednesday 22 June 2011, William A. Rowe Jr. wrote:
> >> On 6/18/2011 7:59 AM, Stefan Fritsch wrote:
> >>> mod_vhost_ldap already exists and is a user of mod_ldap. So -1
> >>> to merging mod_ldap with mod_authnz_ldap.
> >> 
> >> Can anyone do a quick check if it builds against mod_ldap today
> >> with apr 2.0, since the recent changes I made moving apr_ -> ap_
> >> ?
> > 
> > Yes, mod_ldap/mod_authnz_ldap build fine here with apr 2.0.
> 
> Sorry I wasn't so clear... was asking more about mod_vhost_ldap.

That seems to build fine after a search and replace of apr_ldap -> 
ap_ldap.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/22/2011 3:13 PM, Stefan Fritsch wrote:
> On Wednesday 22 June 2011, William A. Rowe Jr. wrote:
>> On 6/18/2011 7:59 AM, Stefan Fritsch wrote:
>>> mod_vhost_ldap already exists and is a user of mod_ldap. So -1 to
>>> merging mod_ldap with mod_authnz_ldap.
>>
>> Can anyone do a quick check if it builds against mod_ldap today
>> with apr 2.0, since the recent changes I made moving apr_ -> ap_ ?
> 
> Yes, mod_ldap/mod_authnz_ldap build fine here with apr 2.0. 

Sorry I wasn't so clear... was asking more about mod_vhost_ldap.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wednesday 22 June 2011, William A. Rowe Jr. wrote:
> On 6/18/2011 7:59 AM, Stefan Fritsch wrote:
> > mod_vhost_ldap already exists and is a user of mod_ldap. So -1 to
> > merging mod_ldap with mod_authnz_ldap.
> 
> Can anyone do a quick check if it builds against mod_ldap today
> with apr 2.0, since the recent changes I made moving apr_ -> ap_ ?

Yes, mod_ldap/mod_authnz_ldap build fine here with apr 2.0. 

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/18/2011 7:59 AM, Stefan Fritsch wrote:
> 
> mod_vhost_ldap already exists and is a user of mod_ldap. So -1 to 
> merging mod_ldap with mod_authnz_ldap.

Can anyone do a quick check if it builds against mod_ldap today with
apr 2.0, since the recent changes I made moving apr_ -> ap_ ?

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Friday 17 June 2011, William A. Rowe Jr. wrote:
> > Is there any remaining benefit from the mod_ldap/mod_authnz_ldap
> > split?   Couldn't we just link it all into one big DSO and stop
> > faffing around with optional functions?  It might be simpler.
> 
> I believe there is.  I remember someone talking about a Novell
> approach to reading the server's config from ldap, which obviously
> doesn't fall into the mod_auth side of the world.
> 
> There are certainly things beyond mod_auth that could be partnered
> with mod_ldap, even mod_userdir comes to mind.

mod_vhost_ldap already exists and is a user of mod_ldap. So -1 to 
merging mod_ldap with mod_authnz_ldap.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/17/2011 7:08 AM, Joe Orton wrote:
> On Mon, Jun 06, 2011 at 04:53:13PM -0500, William Rowe wrote:
>> On 6/6/2011 4:17 PM, Stefan Fritsch wrote:
>>> Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be 
>>> loaded before mod_authnz_ldap. This is somewhat annoying because the 
>>> default httpd.conf tries to load mod_authnz_ldap first. Any ideas how 
>>> to fix this or do we just change the order in the default httpd.conf?
>>
>> I believe the entire fix may be an entry point to apr_ldap_parse_uri
>> (check your own binaries to confirm).  Setting up a single entry point
>> should be trivial, if its appropriate.
> 
> Do you mean arranging for all those functions to be registered as 
> optional function as uldap_* are by mod_ldap, and using them indirectly 
> from mod_authnz_ldap?

That's my first thought, just extending the uldap_* list?

> Is there any remaining benefit from the mod_ldap/mod_authnz_ldap split?  
> Couldn't we just link it all into one big DSO and stop faffing around 
> with optional functions?  It might be simpler.

I believe there is.  I remember someone talking about a Novell approach
to reading the server's config from ldap, which obviously doesn't fall
into the mod_auth side of the world.

There are certainly things beyond mod_auth that could be partnered with
mod_ldap, even mod_userdir comes to mind.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jun 06, 2011 at 04:53:13PM -0500, William Rowe wrote:
> On 6/6/2011 4:17 PM, Stefan Fritsch wrote:
> > Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be 
> > loaded before mod_authnz_ldap. This is somewhat annoying because the 
> > default httpd.conf tries to load mod_authnz_ldap first. Any ideas how 
> > to fix this or do we just change the order in the default httpd.conf?
> 
> I believe the entire fix may be an entry point to apr_ldap_parse_uri
> (check your own binaries to confirm).  Setting up a single entry point
> should be trivial, if its appropriate.

Do you mean arranging for all those functions to be registered as 
optional function as uldap_* are by mod_ldap, and using them indirectly 
from mod_authnz_ldap?

Is there any remaining benefit from the mod_ldap/mod_authnz_ldap split?  
Couldn't we just link it all into one big DSO and stop faffing around 
with optional functions?  It might be simpler.

Regards, Joe

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 27 Jun 2011, at 4:04 PM, William A. Rowe Jr. wrote:

> And I'd nix your definition of mod_ldap... if it an "ldap shared cache
> provider" then it should have been suitably named.  One can omit  
> such a
> feature and still use mod_authnz_ldap.  Of course, that was not  
> possible
> because mod_authnz_ldap required mod_ldap required the ldap helpers
> required an ldap client library.

mod_ldap is an ldap shared memory cache, arguing about what it should  
have been named is pointless, it doesn't make it any less so.

I expect anybody hacking on the LDAP code to know what it is and  
understand how it works. If you don't have even a basic understanding  
of how the existing code works or is structured, it is highly  
irresponsible to be attempting wholesale changes to it. You can add  
this as yet another reason why I veto your commit.

Please revert your code as soon as possible, to ensure that no more of  
this project's time is wasted.

> No.  We didn't agree upon a solution.  We agreed upon a design  
> pattern.

Precisely, and despite this agreement, you have taken it upon yourself  
to ignore this design pattern without any justification or  
explanation, repeatedly suggesting and calling for votes out of the  
blue on all sorts of badly thought out alternatives for no good reason.

In the amount of time we have wasted on pointless deadend design  
discussions, I could have fixed this issue ten times over, and so  
could you. I could have also finished outstanding work on apr_crypto,  
apr_dbd, apr_dbm and mod_cache.

> I have no expectation that it will be fixed,

The only way this statement could possibly be true is if you had no  
intention of fixing the problem yourself, in the way that was agreed.  
There is no two tier system at the ASF where one group gives orders  
and another group follows them, we all collaborate on this project,  
together. Nothing stopped you picking up the draft RFC, learning how  
the LDAP API worked, and filling in the missing part of the API, and  
yet you chose not to. "svn move" does not constitute a meaningful  
contribution towards fixing the issues with the LDAP API.

> and the majority voted at
> APR to evict apr_ldap,

You proposed a vote out of the blue on the APR list to move code to  
httpd, without any prior discussion with either the APR or httpd list.  
You'd obviously discussed this offline, because two people almost  
immediately responded by voting for your move, again without any  
discussion or explanation. Everyone else ignored your thread. The  
httpd project were never notified at all that the vote had taken  
place, never mind their opinions consulted.

As far as procedure is concerned, this vote did not happen.

> therefore it is gone (not be veto or fiat, but
> by the preference of the majority).  I have reacted accordingly  
> because
> httpd would not have been happy if it had abandoned these helpers and
> they were not present for httpd's consumption.

Did it cross your mind to check with the httpd project first whether  
this assumption was true? I'm sure that the httpd project is overjoyed  
at the fact that the latest beta release failed for every person who  
tried it after you had dumped the code into the tree without any  
warning, any discussion, or any testing. I'm sure we are all  
overwhelmed too at having our builds broken out of the blue on pretty  
much every single platform that httpd runs on because you left the  
build scripts unmodified and untested after the dump.

The httpd project currently depends on and works with the perfectly  
functional apr_ldap interface in the current release of apr-util,  
which supports 7 different LDAP libraries on a host of different  
platforms, and LDAP is not going anywhere in apr-util. There was no  
need to make any panicked rush job import of code into the httpd tree,  
it was entirely unnecessary.

We have an opportunity to complete the missing parts of the apr_ldap  
interface and release apr-util v1.4, and then change mod_ldap and  
friends to use those new interface functions before httpd v2.4 goes out.

This represents the simplest and most efficient solution to the  
original problem, which was people deploying apr linked to one ldap  
library alongside mod_foo_ldap linked to another.

Turning LDAP into a pluggable DBD style API is a desirable addition  
for APR v2.0, but makes no practical difference to httpd v2.4, as APR  
v2.0 is vapourware.

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/27/2011 9:04 AM, William A. Rowe Jr. wrote:
> 
> Of course, following Joe's commit, we no longer have the module load order
> issue (thanks Joe!  I'll review win32 within three days per Gregg's notes).

Just read Jim's post, which sounds great.  I'll get to this tonight.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/27/2011 8:19 AM, Graham Leggett wrote:
> 
> mod_ldap - An LDAP shared memory cache
> mod_authnz_ldap - A user of the LDAP shared memory cache
> 
> The LDAP API exposes way more functionality than mod_ldap exposes, so while you may have
> fixed the problem for the special case that is mod_authnz_ldap, you won't have fixed the
> problem for any other module that makes LDAP calls directly.
> 
> The v1.x LDAP API was removed from APR because it was an incomplete abstraction, and for
> this exact same set of reasons it is inappropriate to add it to httpd.
> 
> Over and above that, given that httpd has no library component common to all modules
> (well, it has, it's called APR), and given that the httpd core is inappropriate for an
> LDAP portability library, the only place to put the LDAP bindings is in mod_ldap, which
> means we either have to accept the module ordering problems that result, or we have to
> create optional functions for every single LDAP API call, and if we do that, we end up
> solving the reason apr_ldap was removed from APR in the first place.

This is precisely the reason that you are wrong.  Only mod_ldap and
mod_authnz_ldap (and apparently mod_vhost_ldap) need functionality of
these helpers (hard to call it an API, since it doesn't actually expose
LDAP functionality).

If no other module of, nor httpd itself, needs ldap then none should bind
to ldap libraries.

Of course, following Joe's commit, we no longer have the module load order
issue (thanks Joe!  I'll review win32 within three days per Gregg's notes).

But you can't veto the status quo.  Today, mod_ldap and mod_authnz_ldap
already must bind to the ldap provider.  They are consumers of the stub
functions

And I'd nix your definition of mod_ldap... if it an "ldap shared cache
provider" then it should have been suitably named.  One can omit such a
feature and still use mod_authnz_ldap.  Of course, that was not possible
because mod_authnz_ldap required mod_ldap required the ldap helpers
required an ldap client library.

> This was an observation, not part of the veto. It's important because there are people who
> are really keen to get v2.4 out the door, and this means practically is that a suboptimal
> API is being rushed unnecessarily into GA, and that is really really bad.

Suboptimal?  Sure.  But IJFW, which has been the critera for httpd for
as long as I've been here.  Not acceptable at APR, but just fine, here.

> The solution we agreed on in the past was to rewrite apr_ldap to fit the pattern of
> apr_dbd, but this effort was held up by a whole lot of problems in the apr_dbd API that
> were identified during review of the apr_crypto API, which was modeled on the apr_dbd API.
> This effort was also held up by the focus on getting httpd v2.4 out the door.

No.  We didn't agree upon a solution.  We agreed upon a design pattern.
But that was never implemented, ergo never solved, ergo it's vapor.

> wrowe got impatient and removed apr_ldap from apr-trunk, which on the face of it isn't a
> problem for httpd because apr-trunk isn't a released entity yet, nor is it likely to be
> until the issues with apr_dbd (and as I understand apr_dbm) are fixed (among other
> issues). This move just forces us to solve the apr_ldap issue before apr v2.0 is released.
> It doesn't however put httpd under any obligation to take any drastic measures for httpd
> v2.4, because the existing apr_ldap library exists and is supported in apr v1.x.

You are absolutely right, httpd is under no obligation to support APR 2.0,
or APR 1.x, or pocore, or any other library.  It is an independent project.

> In short, when httpd v2.4 is released, we will be free to focus our attention on apr_ldap
> and get it fixed.
> 
> There is no need for any temporary hacks before then (where "temporary" is defined as
> "permanently baked into the httpd v2.4 API").

I have no expectation that it will be fixed, and the majority voted at
APR to evict apr_ldap, therefore it is gone (not be veto or fiat, but
by the preference of the majority).  I have reacted accordingly because
httpd would not have been happy if it had abandoned these helpers and
they were not present for httpd's consumption.

On the assumption that APR and httpd are now distinct, that httpd will
not require httpd-deps to run, and that httpd will hopefully consume
apr 1.x or apr 2.0 once it is released, this solution is correct for
the short term health of httpd.

httpd and apr major.minor lifespans have been measured in decades
(.5 .. 1.5) and it would be foolish to ship something another project
has abandoned, which was the basis of questioning if expat was the
right choice going forwards.  With the addition of libxml2 support it
is no longer a question or risk.  apr_ldap was a risk, and this change
proposed to fix it.

I agree with Joe's work to complete this, and it is not a veto'able
change for reasons rehashed on two dev lists.  Call a vote to reject
providing ap_ldap in httpd 2.4, if you like.  Because the technical
solution is identical to the technical solution previously provided
in httpd 2.2 with apr-util 1.x, there isn't a technical basis to your
objection.

Because you are trying to pull an end-run around the eviction of the
apr_ldap API (which would go something like "apr 2.0 didn't ship with
apr_ldap so I veto httpd picking up that version") there is not even
merit to your veto.  A few weeks ago you wanted a dozen apr-util-foo
libraries.

If there is someday merit to adopting a full-fledged ldap provider
in APR with actual source code, and a later version of httpd decides
to use that APR major.minor as a minimum version level, then things
will be great and httpd/mod_[foo_]ldap will never link to an ldap
client library again.  But in terms of carts and horses, ldap is
part of the horse, and you know which end some of us consider it.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 08 Jul 2011, at 6:01 PM, Joe Orton wrote:

>> I have already done so. If you disagree with the objection, or do
>> not understand the objection, engage the objection directly so it
>> can be resolved.
>
> I've done exactly that twice in this thread.  I have not seen any
> attempt to concisely express a specific technical objection, rather  
> than
> this hand-waving stuff about "inappropriateness", and how the API is
> "wrong", how we must necessarily reject code because of the APR vote,
> and how it does not solve a problem which you say will be solved  
> better
> by reimplementing the code in APR.
>
> If you don't have specific technical objections (and hence, any reason
> for veto) then you should just offer up an alternative solution and we
> can have a consensus vote to pick one, and move on.

We have already put the solution to a vote, and this is the outcome:

http://www.mail-archive.com/dev@apr.apache.org/msg22061.html

I completely do not understand one bit why we are wasting yet more  
time voting yet again on a solution. Just pick up a keyboard, a copy  
of the draft LDAP RFC, and finish the work we agreed to do.

If you believe someone else should do the work and not you, ask, and  
as a courtesy, explain why you believe it shouldn't be you.

If you believe this work should be prioritised for inclusion in httpd  
v2.4 and apr-util v1.4, say so.

>> In the case of MacOSX, it breaks for me as follows:
>>
>> checking whether to enable mod_authnz_ldap... checking dependencies
>> checking whether to enable mod_authnz_ldap... configure: error:
>> mod_authnz_ldap has been requested but can not be built due to
>> prerequisite failures
>
> Did you build with --with-ldap as well?  If so, can you post the
> complete config.log?

It builds with --with-ldap, but this should not be necessary - it's  
only if your library is in a non standard location that --with-ldap  
should be necessary.

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jul 07, 2011 at 11:59:20PM +0200, Graham Leggett wrote:
> On 04 Jul 2011, at 6:48 PM, Joe Orton wrote:
> 
> >It's incumbent on you to provide specific technical objections if
> >vetoing code, not this hand-waving "objections must exist because
> >of X".
> 
> I have already done so. If you disagree with the objection, or do
> not understand the objection, engage the objection directly so it
> can be resolved.

I've done exactly that twice in this thread.  I have not seen any 
attempt to concisely express a specific technical objection, rather than 
this hand-waving stuff about "inappropriateness", and how the API is 
"wrong", how we must necessarily reject code because of the APR vote, 
and how it does not solve a problem which you say will be solved better 
by reimplementing the code in APR.

If you don't have specific technical objections (and hence, any reason 
for veto) then you should just offer up an alternative solution and we 
can have a consensus vote to pick one, and move on.

> We are not discussing the removal of apr_ldap from APR, we are
> discussing the addition of ap_ldap to httpd.

I only engaged in the discussion about the removal of the code from APR 
because you explicitly called that out as motivation for your veto:

http://www.mail-archive.com/dev@httpd.apache.org/msg51396.html

"I have already stated the basis for the veto: every single apparent 
flaw in the apr_ldap code that caused wrowe to remove it from APR is 
still present in the code that wrowe dumped into httpd."

So if that is not the basis of your veto after all, let's drop it.  But 
I am no closer to understanding the basis for veto.

> In the case of MacOSX, it breaks for me as follows:
> 
> checking whether to enable mod_authnz_ldap... checking dependencies
> checking whether to enable mod_authnz_ldap... configure: error:
> mod_authnz_ldap has been requested but can not be built due to
> prerequisite failures

Did you build with --with-ldap as well?  If so, can you post the 
complete config.log?

Regards, Joe

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 04 Jul 2011, at 6:48 PM, Joe Orton wrote:

> It's incumbent on you to provide specific technical objections if
> vetoing code, not this hand-waving "objections must exist because of  
> X".

I have already done so. If you disagree with the objection, or do not  
understand the objection, engage the objection directly so it can be  
resolved.

> The removal from APR was because the API does not match design of the
> other APIs in APR, because it is an "incomplete abstraction" -  
> mod_ldap
> must use the underlying LDAP API directly as well as the wrapper
> functions.  This is not something which translates directly across to
> httpd; the exported API from mod_ldap was already an incomplete
> abstraction, adding more exports does not change that.  Indeed, it
> explains the motivation for consolidating this stuff in one place.

We are not discussing the removal of apr_ldap from APR, we are  
discussing the addition of ap_ldap to httpd.

Wrowe's reasons for removing apr_ldap from APR are very valid and  
real, the incomplete abstraction is wrong, and sf has already  
explained the problems that are caused when libraries and/or modules  
compiled with different LDAP library are linked to the same server.  
These problems are not in any shape or form solved by moving apr_ldap  
to httpd, the move serves no purpose whatsoever. Over and above this  
zero nett gain, we now have two nett losses:

- We now suffer an abstraction API in httpd, when all other  
abstraction APIs are in APR. I personally don't care whether the  
abstraction libraries are in APR proper, in apr-util, in apr-ldap or  
httpd-helper-libraries, what I do care is that all abstraction APIs  
should be treated the same way. I have not seen any discussion to  
suggest that there are any plans to move DBD, DBM, crypto and XML, nor  
have I seen anyone justify why we are using an inconsistent approach  
in our architecture.

- We currently build against 7 different LDAP APIs on a multitude of  
different platforms. Without the courtesy of any warning or  
discussion, maintainers of various platforms are now expected to spend  
their time and money making this code build again in the httpd  
codebase. What is the payoff for them? Nothing.

Wrowe has attempted to justify his addition of ap_ldap to httpd by  
saying "because httpd would not have been happy if it had abandoned  
these helpers and
they were not present for httpd's consumption", however this statement  
is not true - LDAP is present in apr-util, and will always be present  
as per the versioning rules of APR.

In addition, the versioning rules of apr-util allow us to add to the  
API and in so doing complete the abstraction, solving the problem in  
apr-util v1.4 or v1.5. That makes this entire move unnecessary and a  
huge waste of time.

I am confident that the httpd people will address the APR v2.0 LDAP  
issue, but only after httpd v2.4 is baked, people have time again, and  
APR v2.0 starts becoming something less than vapourware.

> If you have specific concerns with exposing all the ap[r]_ldap_* API
> from httpd, then that is something we can maybe resolve; it looks like
> mod_authnz_ldap and mod_vhost_ldap only require the url_parse hook, so
> maybe we could cut the additional exported API back down to that?

A cut down LDAP API is useless to anybody wishing to bind to an LDAP  
API and use its complete functionality, unless the API is complete, we  
are wasting the end user's time.

Long ago we moved away from this portability nonsense, delegating the  
job to APR. Moving this all back to httpd is ugly in the extreme.

> You allude to build issues without giving details, what's still  
> broken?
> (Thanks Stefan for fixing my own screwup)

See at least the following threads:

- Thread ending with "[VOTE REVOKED] Re: [VOTE] Release Apache  
httpd-2.3.13 as beta"
- httpd LDAP mess

In the case of MacOSX, it breaks for me as follows:

checking whether to enable mod_authnz_ldap... checking dependencies
checking whether to enable mod_authnz_ldap... configure: error:  
mod_authnz_ldap has been requested but can not be built due to  
prerequisite failures

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jul 04, 2011 at 11:43:33AM +0200, Graham Leggett wrote:
> I have already stated the basis for the veto: every single apparent
> flaw in the apr_ldap code that caused wrowe to remove it from APR is
> still present in the code that wrowe dumped into httpd. 

It's incumbent on you to provide specific technical objections if 
vetoing code, not this hand-waving "objections must exist because of X".

The removal from APR was because the API does not match design of the 
other APIs in APR, because it is an "incomplete abstraction" - mod_ldap 
must use the underlying LDAP API directly as well as the wrapper 
functions.  This is not something which translates directly across to 
httpd; the exported API from mod_ldap was already an incomplete 
abstraction, adding more exports does not change that.  Indeed, it 
explains the motivation for consolidating this stuff in one place.

If you have specific concerns with exposing all the ap[r]_ldap_* API 
from httpd, then that is something we can maybe resolve; it looks like 
mod_authnz_ldap and mod_vhost_ldap only require the url_parse hook, so 
maybe we could cut the additional exported API back down to that?

You allude to build issues without giving details, what's still broken? 
(Thanks Stefan for fixing my own screwup)

Regards, Joe


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 07 Jul 2011, at 12:44 AM, Igor Galić wrote:

>> I have already stated the basis for the veto: every single apparent
>> flaw in the apr_ldap code that caused wrowe to remove it from APR is
>> still present in the code that wrowe dumped into httpd. If it's not
>
> It is, fortunately, not in httpd's core. It's in mod_ldap.

It makes no difference in this case whether it's in the core or  
mod_ldap, it is still an incomplete API, and suffers all the flaws we  
suffer right now. What this means practically is that if a module  
wants to limit itself to the caching functionality exposed by mod_ldap  
that module is fine, but if not, that module has to suffer a load  
order problem, and the need to link directly to the LDAP library to  
fill in the blanks (mod_authnz_ldap is forced to do this right now).  
If the end user installs a module binary linked to a different LDAP  
API (mozilla instead of openldap, for example) you get the segfaults  
pointed out by sf.

This change does nothing whatsoever to fix the current problems, and  
performs no purpose.

There is nothing stopping us addressing these problems in apr-util,  
completely within the rules of the APR project, which will fix all our  
problems.

>> good enough for APR, it is not good enough for httpd, period.
>
> As far as we've figured out so far, mod_ldap is the only consumer.
> It might not be good enough for APR or httpd, but it's good enough
> for mod_ldap.

mod_authnz_ldap is a consumer of both mod_ldap and the direct LDAP  
API, as is mod_vhost_ldap, and these are just two modules I know about.

People have been writing modules for httpd and it's derivatives for a  
decade and a half, to make a statement like "mod_ldap is the only  
consumer" is meaningless. People use our code because they trust our  
APIs to remain sensible and stable. This kind of random and badly  
thought out change thrust upon the project at the last minute before a  
GA release is exactly the kind of the change we don't do at the ASF,  
or at httpd.

(Must sleep, more responses over the weekend).

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Igor Galić <i....@brainsware.org>.

----- Original Message -----
> On 04 Jul 2011, at 11:11 AM, Joe Orton wrote:
> 
> >> mod_ldap - An LDAP shared memory cache
> >> mod_authnz_ldap - A user of the LDAP shared memory cache
> >>
> >> The LDAP API exposes way more functionality than mod_ldap exposes,
> >> so while you may have fixed the problem for the special case that
> >> is
> >> mod_authnz_ldap, you won't have fixed the problem for any other
> >> module that makes LDAP calls directly.
> >
> > I don't see how this can be the basis of a veto.  You are stating
> > that
> > there exists a problem which this change does not fix; but that
> > problem
> > existed in the status quo ante, and it *was not the problem this
> > change
> > was intended to fix*.
> 
> I have already stated the basis for the veto: every single apparent
> flaw in the apr_ldap code that caused wrowe to remove it from APR is
> still present in the code that wrowe dumped into httpd. If it's not

It is, fortunately, not in httpd's core. It's in mod_ldap.

> good enough for APR, it is not good enough for httpd, period.

As far as we've figured out so far, mod_ldap is the only consumer.
It might not be good enough for APR or httpd, but it's good enough
for mod_ldap.

> APR-util
> contains abstraction libraries for LDAP, SQL, dbm, XML, and crypto,
> and now you want to move one abstraction library to an httpd module?
> What were you thinking?
> 
> The httpd build is broken on virtually every platform, and is still
> broken more than a month later. That alone is grounds for a veto.
> 
> The httpd project will not be held to ransom by a small group of
> people who break the build in an effort to get others to fix it. This
> code is vetoed, remove the code immediately.
> 
> Regards,
> Graham
> --
> 
> 

-- 
Igor Galić

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 04 Jul 2011, at 11:11 AM, Joe Orton wrote:

>> mod_ldap - An LDAP shared memory cache
>> mod_authnz_ldap - A user of the LDAP shared memory cache
>>
>> The LDAP API exposes way more functionality than mod_ldap exposes,
>> so while you may have fixed the problem for the special case that is
>> mod_authnz_ldap, you won't have fixed the problem for any other
>> module that makes LDAP calls directly.
>
> I don't see how this can be the basis of a veto.  You are stating that
> there exists a problem which this change does not fix; but that  
> problem
> existed in the status quo ante, and it *was not the problem this  
> change
> was intended to fix*.

I have already stated the basis for the veto: every single apparent  
flaw in the apr_ldap code that caused wrowe to remove it from APR is  
still present in the code that wrowe dumped into httpd. If it's not  
good enough for APR, it is not good enough for httpd, period. APR-util  
contains abstraction libraries for LDAP, SQL, dbm, XML, and crypto,  
and now you want to move one abstraction library to an httpd module?  
What were you thinking?

The httpd build is broken on virtually every platform, and is still  
broken more than a month later. That alone is grounds for a veto.

The httpd project will not be held to ransom by a small group of  
people who break the build in an effort to get others to fix it. This  
code is vetoed, remove the code immediately.

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jun 27, 2011 at 03:19:37PM +0200, Graham Leggett wrote:
> mod_ldap - An LDAP shared memory cache
> mod_authnz_ldap - A user of the LDAP shared memory cache
> 
> The LDAP API exposes way more functionality than mod_ldap exposes,
> so while you may have fixed the problem for the special case that is
> mod_authnz_ldap, you won't have fixed the problem for any other
> module that makes LDAP calls directly.

I don't see how this can be the basis of a veto.  You are stating that 
there exists a problem which this change does not fix; but that problem 
existed in the status quo ante, and it *was not the problem this change 
was intended to fix*.

> The v1.x LDAP API was removed from APR because it was an incomplete
> abstraction, and for this exact same set of reasons it is
> inappropriate to add it to httpd.

The API provided by mod_ldap was already an "incomplete abstraction" - 
this is not something that has been changed in this commit to httpd, so 
I cannot see how this can be the basis of a veto, as above.  The 
consensus vote in APR concluded precisely that the code was more 
appropriate for httpd than APR; you cannot twist the logic of that to 
pretend it implies the opposite.

> Over and above that, given that httpd has no library component
> common to all modules (well, it has, it's called APR), and given
> that the httpd core is inappropriate for an LDAP portability
> library, the only place to put the LDAP bindings is in mod_ldap,
> which means we either have to accept the module ordering problems
> that result, or we have to create optional functions for every
> single LDAP API call, and if we do that, we end up solving the
> reason apr_ldap was removed from APR in the first place.

a) "httpd core is inappropriate for an LDAP portability library"

-> this is an opinion, not technical justification for a veto.

b) "we end up solving the reason"

I don't understand that sentence, can you rephrase?

> In addition, we moved all portability code out of httpd in v2.0, it
> is a huge step backwards to go back on that effort. It is really
> ugly to have apr_xml, apr_dbd, apr_crypto and apr_dbm in APR, and
> then ap(r)_ldap somewhere else.

"it's ugly"... sure.  I agree with that, but I also think this change is 
a significant improvement.  Merely stating "it is ugly" is not 
sufficient basis for a veto.

> This is the essence of my veto.

I don't see technical basis for a veto here, unless you can clarify.

Regards, Joe

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 27 Jun 2011, at 12:28 PM, Joe Orton wrote:

>> This is not so, to fix this, you would need to wrap every single
>> LDAP API function call[1] in an optional function, and if you did
>> that, you would solve the problem that caused you to want to remove
>> apr_ldap from APR in the first place, making the whole exercise
>> pointless - you may as well just have fixed apr-ldap in place.
>
> mod_authnz_ldap is an existence proof that we don't need to wrap every
> LDAP API function call for mod_ldap to be useful in its current form,
> surely?

mod_ldap - An LDAP shared memory cache
mod_authnz_ldap - A user of the LDAP shared memory cache

The LDAP API exposes way more functionality than mod_ldap exposes, so  
while you may have fixed the problem for the special case that is  
mod_authnz_ldap, you won't have fixed the problem for any other module  
that makes LDAP calls directly.

The v1.x LDAP API was removed from APR because it was an incomplete  
abstraction, and for this exact same set of reasons it is  
inappropriate to add it to httpd.

Over and above that, given that httpd has no library component common  
to all modules (well, it has, it's called APR), and given that the  
httpd core is inappropriate for an LDAP portability library, the only  
place to put the LDAP bindings is in mod_ldap, which means we either  
have to accept the module ordering problems that result, or we have to  
create optional functions for every single LDAP API call, and if we do  
that, we end up solving the reason apr_ldap was removed from APR in  
the first place.

In addition, we moved all portability code out of httpd in v2.0, it is  
a huge step backwards to go back on that effort. It is really ugly to  
have apr_xml, apr_dbd, apr_crypto and apr_dbm in APR, and then  
ap(r)_ldap somewhere else.

This is the essence of my veto.

>> The timing cannot be worse
>
> This is probably true, but it doesn't seem like technical  
> justification
> for a veto.  Release schedules can move at the pace we want them to.

This was an observation, not part of the veto. It's important because  
there are people who are really keen to get v2.4 out the door, and  
this means practically is that a suboptimal API is being rushed  
unnecessarily into GA, and that is really really bad.

>> There is no need for this move at all
>
> This is strictly true, in the sense that there may be other ways to
> solve the problems fixed by moving the code.  If other solutions are
> presented, we can have a consensus vote on which to choose.  In the
> absence of other solutions being implemented, asserting that "better
> solutions exist" can never be sufficient reason to veto a change.

The solution we agreed on in the past was to rewrite apr_ldap to fit  
the pattern of apr_dbd, but this effort was held up by a whole lot of  
problems in the apr_dbd API that were identified during review of the  
apr_crypto API, which was modeled on the apr_dbd API. This effort was  
also held up by the focus on getting httpd v2.4 out the door.

wrowe got impatient and removed apr_ldap from apr-trunk, which on the  
face of it isn't a problem for httpd because apr-trunk isn't a  
released entity yet, nor is it likely to be until the issues with  
apr_dbd (and as I understand apr_dbm) are fixed (among other issues).  
This move just forces us to solve the apr_ldap issue before apr v2.0  
is released. It doesn't however put httpd under any obligation to take  
any drastic measures for httpd v2.4, because the existing apr_ldap  
library exists and is supported in apr v1.x.

In short, when httpd v2.4 is released, we will be free to focus our  
attention on apr_ldap and get it fixed.

There is no need for any temporary hacks before then (where  
"temporary" is defined as "permanently baked into the httpd v2.4 API").

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Joe Orton <jo...@redhat.com>.
On Sat, Jun 25, 2011 at 10:11:20PM +0200, Graham Leggett wrote:
> On 06 Jun 2011, at 11:53 PM, William A. Rowe Jr. wrote:
> >>Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be
> >>loaded before mod_authnz_ldap. This is somewhat annoying because the
> >>default httpd.conf tries to load mod_authnz_ldap first. Any ideas how
> >>to fix this or do we just change the order in the default httpd.conf?
> >
> >I believe the entire fix may be an entry point to apr_ldap_parse_uri
> >(check your own binaries to confirm).  Setting up a single entry point
> >should be trivial, if its appropriate.
> 
> This is not so, to fix this, you would need to wrap every single
> LDAP API function call[1] in an optional function, and if you did
> that, you would solve the problem that caused you to want to remove
> apr_ldap from APR in the first place, making the whole exercise
> pointless - you may as well just have fixed apr-ldap in place.

mod_authnz_ldap is an existence proof that we don't need to wrap every 
LDAP API function call for mod_ldap to be useful in its current form, 
surely?

> In it's current form, this change introduces module ordering bugs to
> httpd that we haven't suffered for a decade.

Fixed in r1140069.

> In addition, the autoconf build is currently broken against apr v1.x
> on MacOSX, and this is probably broken on other platforms as well.

Can you give details on this?

> The timing cannot be worse

This is probably true, but it doesn't seem like technical justification 
for a veto.  Release schedules can move at the pace we want them to.

> There is no need for this move at all

This is strictly true, in the sense that there may be other ways to 
solve the problems fixed by moving the code.  If other solutions are 
presented, we can have a consensus vote on which to choose.  In the 
absence of other solutions being implemented, asserting that "better 
solutions exist" can never be sufficient reason to veto a change.

Regards, Joe

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Thursday 30 June 2011, Graham Leggett wrote:
> On 27 Jun 2011, at 8:29 PM, Stefan Fritsch wrote:
> >> This is fixed by calling the ldap_get_option() function
> >> described in section 9.2 of
> >> http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ld
> >> ap ext-ldap-c-api-05.txt . There is no need to move the code to
> >> support this, this can be supported today by adding the
> >> appropriate sanity check on init and failing as appropriate.
> > 
> > This does not help us. We need the information encoded in the
> > package dependencies so that the package manager refuses to
> > install a version of httpd together with a version of apr that
> > don't work together.
> > 
> > And I don't think you suggestion would work, at least not without
> > a new API. The problem is this: If apr uses libldap.A and httpd
> > uses libldap.B, httpd will call apr_ldap_init() which makes apr
> > call libldap.A's ldap_init(). Apr then passes the resulting LDAP
> > * pointer to httpd. But httpd will use libldap.B's other ldap_*
> > functions on the object created by libldap.A. This usually
> > causes segfaults, but it may also cause more subtle bugs. To
> > detect this issue, both httpd and apr would need to call
> > ldap_get_option() and httpd would need to compare the versions
> > returned by both calls.
> 
> To fix this, wrap the rest of the LDAP API. This is specified in
> http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldap
> ext-ldap-c-api-05.txt , and should be a straightforward task.

Maybe straightforward but still a lot of work. I don't want to do it 
and I think going this way would delay httpd 2.4 further than the 
alternatives.

> Moving ap(r)_ldap to mod_ldap just means you suffer this problem
> with other packages, like mod_vhost_ldap, you don't solve this
> problem at all.

It's not a solution, but it helps quite a bit if only httpd related 
packages are affected. So, it's at least some improvement.


> > The problem is that we also need to modify mod_ldap. If we want
> > true isolation, we would also get #ifdef APR_HAS_SOLARIS_LDAPSDK
> > sections in mod_ldap, which is what what apr_ldap tried to avoid
> > in the first place.
> 
> This is the wrong way to solve this, here is the full history
> behind it.
> 
> In the early days of LDAP libraries, SSL support was an extremely
> basic affair. Implementers hacked the ldap_init() call in various
> ugly ways to pass a flag that caused ldap_init() to make a simple
> and immediate SSL connection, based on some server wide notion of
> CA certificates and other SSL information. The SSL connection was
> made immediately, and success or failure returned from the
> ldap_init() directly. Call this "simple mode".
> 
> Over time, LDAP library implementations realised that SSL support
> is configured per connection, do you want SSL or do you want TLS?
> Do you want a specific client or CA certificate for this
> connection? Instead of a single call to a hacked up ldap_init(),
> now you had a multi-stage process, first you ldap_init, then you
> set any CA and/or client certificates with ldap_set_option, and
> then you set whether the connection will be SSL or TLS with
> ldap_set_option again. At this point the connection is completed,
> and you get success or failure after the three steps are complete.
> Call this "multi-stage mode".
> 
> Modern LDAP libraries like openldap and mozilla support
> "multi-stage mode", while legacy LDAP libraries like the Solaris
> library only support "simple mode".
> 
> These are two separate and distinct connection patterns that have
> to be allowed for separately.
> 
> The apr_ldap API supports both modes. In the apr_ldap API, if you
> want "simple mode", you pass the "secure" parameter to
> apr_ldap_init(). If you want "multi-stage mode", you pass the
> "secure" parameter via an apr_ldap_set_option() call.
>
> Right now, mod_ldap supports "multi-stage" mode only, it does not
> yet support "simple mode", simply because nobody has ever asked
> mod_ldap to support "simple mode".

It did work with 2.0.63. But maybe 2.0 just did not use some buggy 
feature in Solaris LDAP while 2.2 does.

I think a problem is when to call ldap_set_option(LDAP_OPT_SSL). The 
Solaris ldap libs will segfault if SSL is enabled more than once. So 
mod_ldap cannot call apr_ldap_set_option(AP_LDAP_OPT_TLS) if it has 
already passed the "secure" parameter to apr_ldap_init(). But no one 
can say if this change will break other ldap sdks.

There is some documentation at 
http://web.archive.org/web/20090207112215/http://docs.sun.com/source/817-6707/ssl.html 
I think there were a few more caveats to actually make it work. But 
right now, I can't remember what exactly I have tried last year. 
Unfortunately, I only have occasional access to a Solaris machine and 
can't do any tests at the moment.
 
> The simplest way to tackle this is to teach mod_ldap to autodetect
> "simple mode", in other words if no per connection certificates are
> present, and if "secure" is "SSL" (not "TLS"), use apr_ldap_init()
> in "simple mode", otherwise use the existing "multi-stage mode".
> 
> Any solution that attempts to detect APR_HAS_SOLARIS_LDAPSDK in
> mod_ldap is both unnecessary and wrong, for the reasons you've
> already described, just use the apr_ldap API as it was designed to
> be used.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 27 Jun 2011, at 8:29 PM, Stefan Fritsch wrote:

>> This is fixed by calling the ldap_get_option() function described
>> in section 9.2 of
>> http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldap
>> ext-ldap-c-api-05.txt . There is no need to move the code to
>> support this, this can be supported today by adding the
>> appropriate sanity check on init and failing as appropriate.
>
> This does not help us. We need the information encoded in the package
> dependencies so that the package manager refuses to install a version
> of httpd together with a version of apr that don't work together.

> And I don't think you suggestion would work, at least not without a
> new API. The problem is this: If apr uses libldap.A and httpd uses
> libldap.B, httpd will call apr_ldap_init() which makes apr call
> libldap.A's ldap_init(). Apr then passes the resulting LDAP * pointer
> to httpd. But httpd will use libldap.B's other ldap_* functions on the
> object created by libldap.A. This usually causes segfaults, but it may
> also cause more subtle bugs. To detect this issue, both httpd and apr
> would need to call ldap_get_option() and httpd would need to compare
> the versions returned by both calls.

To fix this, wrap the rest of the LDAP API. This is specified in http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldapext-ldap-c-api-05.txt 
, and should be a straightforward task.

Moving ap(r)_ldap to mod_ldap just means you suffer this problem with  
other packages, like mod_vhost_ldap, you don't solve this problem at  
all.

> The problem is that we also need to modify mod_ldap. If we want true
> isolation, we would also get #ifdef APR_HAS_SOLARIS_LDAPSDK sections
> in mod_ldap, which is what what apr_ldap tried to avoid in the first
> place.


This is the wrong way to solve this, here is the full history behind it.

In the early days of LDAP libraries, SSL support was an extremely  
basic affair. Implementers hacked the ldap_init() call in various ugly  
ways to pass a flag that caused ldap_init() to make a simple and  
immediate SSL connection, based on some server wide notion of CA  
certificates and other SSL information. The SSL connection was made  
immediately, and success or failure returned from the ldap_init()  
directly. Call this "simple mode".

Over time, LDAP library implementations realised that SSL support is  
configured per connection, do you want SSL or do you want TLS? Do you  
want a specific client or CA certificate for this connection? Instead  
of a single call to a hacked up ldap_init(), now you had a multi-stage  
process, first you ldap_init, then you set any CA and/or client  
certificates with ldap_set_option, and then you set whether the  
connection will be SSL or TLS with ldap_set_option again. At this  
point the connection is completed, and you get success or failure  
after the three steps are complete. Call this "multi-stage mode".

Modern LDAP libraries like openldap and mozilla support "multi-stage  
mode", while legacy LDAP libraries like the Solaris library only  
support "simple mode".

These are two separate and distinct connection patterns that have to  
be allowed for separately.

The apr_ldap API supports both modes. In the apr_ldap API, if you want  
"simple mode", you pass the "secure" parameter to apr_ldap_init(). If  
you want "multi-stage mode", you pass the "secure" parameter via an  
apr_ldap_set_option() call.

Right now, mod_ldap supports "multi-stage" mode only, it does not yet  
support "simple mode", simply because nobody has ever asked mod_ldap  
to support "simple mode".

The simplest way to tackle this is to teach mod_ldap to autodetect  
"simple mode", in other words if no per connection certificates are  
present, and if "secure" is "SSL" (not "TLS"), use apr_ldap_init() in  
"simple mode", otherwise use the existing "multi-stage mode".

Any solution that attempts to detect APR_HAS_SOLARIS_LDAPSDK in  
mod_ldap is both unnecessary and wrong, for the reasons you've already  
described, just use the apr_ldap API as it was designed to be used.

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sunday 26 June 2011, Graham Leggett wrote:
> On 26 Jun 2011, at 3:16 PM, Stefan Fritsch wrote:
> > Nobody said that this would be magically fixed by moving the
> > stuff to HTTPD. But it means that the apr libraries are no
> > longer involved in the mess, which is already very helpful for
> > distributions like Debian. With the apr 1.x situation, an ABI
> > break (soname change) in openldap means we have to jump through
> > hoops to prevent crashes when httpd uses one version of openldap
> > and apr uses a different version.
> 
> This is fixed by calling the ldap_get_option() function described
> in section 9.2 of
> http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldap
> ext-ldap-c-api-05.txt . There is no need to move the code to
> support this, this can be supported today by adding the
> appropriate sanity check on init and failing as appropriate.

This does not help us. We need the information encoded in the package 
dependencies so that the package manager refuses to install a version 
of httpd together with a version of apr that don't work together.

And I don't think you suggestion would work, at least not without a 
new API. The problem is this: If apr uses libldap.A and httpd uses 
libldap.B, httpd will call apr_ldap_init() which makes apr call 
libldap.A's ldap_init(). Apr then passes the resulting LDAP * pointer 
to httpd. But httpd will use libldap.B's other ldap_* functions on the 
object created by libldap.A. This usually causes segfaults, but it may 
also cause more subtle bugs. To detect this issue, both httpd and apr 
would need to call ldap_get_option() and httpd would need to compare 
the versions returned by both calls.

> > I think you misread that comment. The patch attached to the PR
> > makes it work with Solaris LDAP but breaks with OpenLDAP,
> > therefore it is not acceptable.
> 
> Looking at the patch in more detail, the patch makes indiscriminate
> attempts to change the behaviour of all the platforms at the same
> time, instead of targeting the Solaris platform only. OpenLDAP will
> break by definition in this case, as will other platforms.
> 
> What the patch needs to do is properly isolate the functions being
> used on Solaris (ldapssl_init()? ldap_sslint()? something else?),
> and modify them only, using #ifdef where appropriate.

The problem is that we also need to modify mod_ldap. If we want true 
isolation, we would also get #ifdef APR_HAS_SOLARIS_LDAPSDK sections 
in mod_ldap, which is what what apr_ldap tried to avoid in the first 
place.


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 26 Jun 2011, at 3:16 PM, Stefan Fritsch wrote:

> Nobody said that this would be magically fixed by moving the stuff  
> to HTTPD. But it means that the apr libraries are no longer involved  
> in the mess, which is already very helpful for distributions like  
> Debian. With the apr 1.x situation, an ABI break (soname change) in  
> openldap means we have to jump through hoops to prevent crashes when  
> httpd uses one version of openldap and apr uses a different version.

This is fixed by calling the ldap_get_option() function described in  
section 9.2 of http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldapext-ldap-c-api-05.txt 
. There is no need to move the code to support this, this can be  
supported today by adding the appropriate sanity check on init and  
failing as appropriate.

> I think you misread that comment. The patch attached to the PR makes  
> it work with Solaris LDAP but breaks with OpenLDAP, therefore it is  
> not acceptable.

Looking at the patch in more detail, the patch makes indiscriminate  
attempts to change the behaviour of all the platforms at the same  
time, instead of targeting the Solaris platform only. OpenLDAP will  
break by definition in this case, as will other platforms.

What the patch needs to do is properly isolate the functions being  
used on Solaris (ldapssl_init()? ldap_sslint()? something else?), and  
modify them only, using #ifdef where appropriate.

> But this highlights another problem with how we deal ldap: There is  
> no way a developer can test changes on all or even on a significant  
> subset of the supported toolkits. This makes it very hard to do any  
> changes at all. Ideally we would only support toolkits/plattforms  
> that are available at the ASF for testing.

The apr_dbd and apr_crypto APIs already demonstrate how to solve this  
problem, and the plans for apr_ldap in v2.0 are to implement an API  
that follows the same pattern.

The apr_dbd and apr_crypto APIs have had various shortcomings that  
need to be fixed in APR v2.0, and this has held up work on apr_ldap.

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sun, 26 Jun 2011, Graham Leggett wrote:

> On 25 Jun 2011, at 11:24 PM, Stefan Fritsch wrote:
>
>>> This is not so, to fix this, you would need to wrap every single
>>> LDAP API function call[1] in an optional function, and if you did
>>> that, you would solve the problem that caused you to want to
>>> remove apr_ldap from APR in the first place, making the whole
>>> exercise pointless - you may as well just have fixed apr-ldap in
>>> place.
>> 
>> No, that is not correct. You only need to wrap all ap_ldap_* calls and
>> make the depending modules (mod_authnz_ldap, mod_vhost_ldap, ...) link
>> to the ldap libraries themselves.
>
> This is one of the key reasons that got apr_ldap evicted from APR in the 
> first place.

Nobody said that this would be magically fixed by moving the stuff to 
HTTPD. But it means that the apr libraries are no longer involved in the 
mess, which is already very helpful for distributions like Debian. With 
the apr 1.x situation, an ABI break (soname change) in openldap means we 
have to jump through hoops to prevent crashes when httpd uses one version 
of openldap and apr uses a different version.

> If the API is not good enough for APR, then the API is not good enough for 
> httpd. We must fix this problem, not shift it around.
>
>> Moving the ldap_init code from mod_ldap into apr with httpd 2.2.x
>> broke support for Solaris LDAP (PR 42682), which is a regression from
>> 2.0.x. Fixing that would require changes both in mod_ldap and in apr-
>> ldap. It says a lot that mod_ldap in 2.2.x does not use
>> apr_ldap_init() in the way it is recommended in the documentation
>> (setting secure if ldaps is wanted without client certificates).
>> Because if it did, it would not work, because the apr_ldap code is
>> buggy.
>
> Looking at this, the fix involves teaching mod_ldap to pass the correct 
> parameter to apr_ldap_ssl_init(), it doesn't require a change to the API that 
> I can see.

Changing mod_ldap in this simple way breaks with OpenLDAP. So it's not 
that simple, unfortunately. If we make any changes to apr_ldap that 
require a different calling order of the functions or change the semantics 
in every other way, that's a change in the API. Even if all function 
prototypes stay the same.

> The long term fix is to hide the LDAP structure within something like 
> apr_ldap_t, and then lazy init when necessary. This is why 
> apr_ldap_ssl_init() is marked deprecated in v1.x.
>
> To address the comment in the PR about only supporting openssl, we support 
> the following distinct LDAP toolkits across a myriad of platforms:

I think you misread that comment. The patch attached to the PR makes it 
work with Solaris LDAP but breaks with OpenLDAP, therefore it is not 
acceptable.

> - OpenLDAP
> - Solaris
> - Novell
> - Microsoft
> - Netscape
> - Mozilla
> - Tivoli
> - z/OS
>
> Do we still work after the API was moved? For MacOSX at least, no, for the 
> others, particularly many of the smaller platforms, probably not.

But if we have ap_ldap_init in HTTPD, we can work around changes in 
behaviour by modifying how mod_ldap calls the API. This is not possible 
with apr_ldap_init because it must continue to work with the unchanged, 
released versions of 2.2.x's mod_ldap. Therefore moving the ldap code into 
HTTPD gives more flexibility.

If MacOSX is broken, MacOSX users/devs should complain and provide more 
info. I couldn't find a mail about this.

But this highlights another problem with how we deal ldap: There is no way 
a developer can test changes on all or even on a significant subset of the 
supported toolkits. This makes it very hard to do any changes at all. 
Ideally we would only support toolkits/plattforms that are available at 
the ASF for testing.

>> Doing these changes in APR 1.x in a way that does not break an
>> unmodified 2.2.x mod_ldap with some LDAP library is next to
>> impossible. So moving the apr_ldap stuff as ap_ldap into httpd and
>> therefore making it possible to change the API is an advantage.
>
> That's incorrect, it's not possible to change the API - when we release httpd 
> v2.4.0, that's the API baked, and you don't get a chance fix this.

But 2.4.0 is not yet released. Until then we can change things. And if 
something breaks, we can fix it in 2.4.1. This is much less severe than if 
we break APR and affect all HTTPD 2.2.x user, too.

> It is not fair on end users who have waited ages for httpd v2.4.0 to suddenly 
> force them to wait even longer while we fix the APIs in APR. It is even worse 
> to expect end users to deal with problems caused by APIs dumped in at the 
> last minute without a proper stabilisation process before we make a major 
> release.

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 25 Jun 2011, at 11:24 PM, Stefan Fritsch wrote:

>> This is not so, to fix this, you would need to wrap every single
>> LDAP API function call[1] in an optional function, and if you did
>> that, you would solve the problem that caused you to want to
>> remove apr_ldap from APR in the first place, making the whole
>> exercise pointless - you may as well just have fixed apr-ldap in
>> place.
>
> No, that is not correct. You only need to wrap all ap_ldap_* calls and
> make the depending modules (mod_authnz_ldap, mod_vhost_ldap, ...) link
> to the ldap libraries themselves.

This is one of the key reasons that got apr_ldap evicted from APR in  
the first place.

If the API is not good enough for APR, then the API is not good enough  
for httpd. We must fix this problem, not shift it around.

> Moving the ldap_init code from mod_ldap into apr with httpd 2.2.x
> broke support for Solaris LDAP (PR 42682), which is a regression from
> 2.0.x. Fixing that would require changes both in mod_ldap and in apr-
> ldap. It says a lot that mod_ldap in 2.2.x does not use
> apr_ldap_init() in the way it is recommended in the documentation
> (setting secure if ldaps is wanted without client certificates).
> Because if it did, it would not work, because the apr_ldap code is
> buggy.

Looking at this, the fix involves teaching mod_ldap to pass the  
correct parameter to apr_ldap_ssl_init(), it doesn't require a change  
to the API that I can see.

The long term fix is to hide the LDAP structure within something like  
apr_ldap_t, and then lazy init when necessary. This is why  
apr_ldap_ssl_init() is marked deprecated in v1.x.

To address the comment in the PR about only supporting openssl, we  
support the following distinct LDAP toolkits across a myriad of  
platforms:

- OpenLDAP
- Solaris
- Novell
- Microsoft
- Netscape
- Mozilla
- Tivoli
- z/OS

Do we still work after the API was moved? For MacOSX at least, no, for  
the others, particularly many of the smaller platforms, probably not.

> Doing these changes in APR 1.x in a way that does not break an
> unmodified 2.2.x mod_ldap with some LDAP library is next to
> impossible. So moving the apr_ldap stuff as ap_ldap into httpd and
> therefore making it possible to change the API is an advantage.

That's incorrect, it's not possible to change the API - when we  
release httpd v2.4.0, that's the API baked, and you don't get a chance  
fix this.

It is not fair on end users who have waited ages for httpd v2.4.0 to  
suddenly force them to wait even longer while we fix the APIs in APR.  
It is even worse to expect end users to deal with problems caused by  
APIs dumped in at the last minute without a proper stabilisation  
process before we make a major release.

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Saturday 25 June 2011, Graham Leggett wrote:
> On 06 Jun 2011, at 11:53 PM, William A. Rowe Jr. wrote:
> > I believe the entire fix may be an entry point to
> > apr_ldap_parse_uri (check your own binaries to confirm). 
> > Setting up a single entry point should be trivial, if its
> > appropriate.
> 
> This is not so, to fix this, you would need to wrap every single
> LDAP API function call[1] in an optional function, and if you did
> that, you would solve the problem that caused you to want to
> remove apr_ldap from APR in the first place, making the whole
> exercise pointless - you may as well just have fixed apr-ldap in
> place.

No, that is not correct. You only need to wrap all ap_ldap_* calls and 
make the depending modules (mod_authnz_ldap, mod_vhost_ldap, ...) link 
to the ldap libraries themselves.

> In it's current form, this change introduces module ordering bugs
> to httpd that we haven't suffered for a decade.
> 
> In addition, the autoconf build is currently broken against apr
> v1.x on MacOSX, and this is probably broken on other platforms as
> well. This introduces serious inconvenience for vendors who have
> to mess about trying to make all of this build all over again on
> all sorts of platforms.
> 
> The timing cannot be worse - an already suboptimal API plus these
> new bugs are being dumped into httpd in the final stages of trying
> to bake the final version of httpd v2.4.0, which means we will be
> stuck with this brokenness through the life of httpd v2.4.
> 
> There is no need for this move at all, as httpd works perfectly
> against APR v1.x (or did until this change). APR v2.x hasn't gone
> through any kind of stabilisation phase, never mind seen an alpha
> or beta release, and so httpd v2.4.x being compatible with
> apr-trunk at this stage is not necessary, especially seeing that
> when httpd v2.4 is released, it's API is set in stone, but APR
> v2.0's API remains in flux. Or to put it another way, the fact
> that apr_ldap is missing from apr-trunk is not a problem for httpd
> v2.4, and can be solved after httpd v2.4.

Moving the ldap_init code from mod_ldap into apr with httpd 2.2.x 
broke support for Solaris LDAP (PR 42682), which is a regression from 
2.0.x. Fixing that would require changes both in mod_ldap and in apr-
ldap. It says a lot that mod_ldap in 2.2.x does not use 
apr_ldap_init() in the way it is recommended in the documentation 
(setting secure if ldaps is wanted without client certificates). 
Because if it did, it would not work, because the apr_ldap code is 
buggy.

Doing these changes in APR 1.x in a way that does not break an 
unmodified 2.2.x mod_ldap with some LDAP library is next to 
impossible. So moving the apr_ldap stuff as ap_ldap into httpd and 
therefore making it possible to change the API is an advantage.

> I am therefore vetoing this move of apr_ldap from APR to httpd.

Please don't.

> We need to focus on getting httpd v2.4 out the door before worrying
> about some future version of APR.
> 
> [1]
> http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldap
> ext-ldap-c-api-05.txt

Cheers,
Stefan

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 7/7/2011 9:10 PM, Nick Kew wrote:
> 
>> I am therefore vetoing this move of apr_ldap from APR to httpd.
> 
> Hang on!  How long ago did this move happen, and when did you
> first raise concerns?

His first attempt at vetoing this change was in 2009, accompanied
by an assertion that he would address the issues.  Of course, nobody
expects any individual to provide specific code to do anything, but that
doesn't excuse the promise of action or requests for unending dialog
without code to slow a project for two years;

http://mail-archives.apache.org/mod_mbox/apr-dev/200903.mbox/%3C5c902b9e0903240326r3222ac90k15dcb7f34d2d1587@mail.gmail.com%3E

[One will note Graham isn't arguing against my proposal, but Justin's.]

That that time, Graham's response was that decisions should not be made
offlist at a face to face, which was patently absurd on the face of it,
since Justin brought a decision to the list, rather than act on the
conversations he had with individual committers.

Following inaction on the [endless] discussion in this thread and others,
I finally proposed an absolute eviction, to which Graham did not respond;

http://mail-archives.apache.org/mod_mbox/apr-dev/201004.mbox/%3C4BD46614.6010503@rowe-clan.net%3E
[and following on in May...]
http://mail-archives.apache.org/mod_mbox/apr-dev/201005.mbox/%3CAANLkTinKzmDYbxo2VggZLOxPY72mH0OReK-kUeSGJSa3@mail.gmail.com%3E

Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
[answering this backwards]

On 08 Jul 2011, at 4:10 AM, Nick Kew wrote:

>> I am therefore vetoing this move of apr_ldap from APR to httpd.
>
> Hang on!  How long ago did this move happen, and when did you
> first raise concerns?

The move happened on the 31st of May, and my concerns were first  
raised that same day:

http://mail-archives.apache.org/mod_mbox/httpd-cvs/201105.mbox/%3C20110531171012.DFC8123888E4@eris.apache.org%3E

The announcement that the move was to take place was declared on the  
dev@apr mailing list:

http://www.mail-archive.com/dev%40apr.apache.org/msg24081.html

In reply I asked "Can you point out for me the thread on the dev@httpd  
list when this was decided? Why are we discussing changes to httpd on  
the dev@apr list?". This particular question was ignored.

Further on in the thread I made the following statement:

http://www.mail-archive.com/dev%40apr.apache.org/msg24086.html

"Leaving end users to discover that whole APIs have mysteriously  
disappeared without warning or explanation, without those APIs having  
been marked as deprecated, is grounds for a veto. So is the idea that  
another project will tolerate a code dump from one project to another,  
with the corresponding disruption to vendors that will result. In  
addition, APR has been a standalone library, depended on by many  
projects external to the ASF for many years, APR is not part of httpd,  
and code isn't interchangeable between them."

This statement was not challenged.

I eventually formally vetoed the move on 26 June as I warned I would  
do after the disastrous attempt to release v2.3.13 of httpd:

http://www.mail-archive.com/dev%40httpd.apache.org/msg51301.html

This veto has to date been ignored, and the grounds for the veto as  
stated remain unchallenged by wrowe.

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Nick Kew <ni...@webthing.com>.
On 25 Jun 2011, at 21:11, Graham Leggett wrote:

> This is not so, to fix this, you would need to wrap every single LDAP API function call[1] in an optional function, and if you did that, you would solve the problem that caused you to want to remove apr_ldap from APR in the first place, making the whole exercise pointless - you may as well just have fixed apr-ldap in place.

In view of the escalation of this argument, I'm trying to review the history and see
if I can get to grips with the underlying problem.  It seems this is where your
objection starts, right?  If it matters, I'm trying not to take sides here, and I too
find it uncomfortable when Bill gets abrasive in a public list.

But isn't the above a slight misstatement of the issue?  ap_ldap (and apr_ldap before)
wraps only a subset of the LDAP API, that subset is what in fact has to be wrapped in
optional functions.  I don't see a problem with that: more optional functions can be
introduced as and when more APIs are exposed.  And optional functions can exist
alongside regular API functions, as in mod_dbd.h, as and when you or anyone do it.

> The timing cannot be worse - an already suboptimal API plus these new bugs are being dumped into httpd in the final stages of trying to bake the final version of httpd v2.4.0, which means we will be stuck with this brokenness through the life of httpd v2.4.

Up to a point.  We're full of compromises, some nicer, some uglier.
But what *new* bugs are you referring to that aren't fixed by optional functions?
Would it make sense to introduce a mod_dbd-like parallel API, and does it break
anything that would prevent you hacking it within the lifetime of 2.4?

> There is no need for this move at all, as httpd works perfectly against APR v1.x (or did until this change). APR v2.x hasn't gone through any kind of stabilisation phase, never mind seen an alpha or beta release, and so httpd v2.4.x being compatible with apr-trunk at this stage is not necessary, especially seeing that when httpd v2.4 is released, it's API is set in stone, but APR v2.0's API remains in flux. Or to put it another way, the fact that apr_ldap is missing from apr-trunk is not a problem for httpd v2.4, and can be solved after httpd v2.4.

That's a fair point.  If you can convince me there are NEW unsolved bugs,
I may support it.  But I wouldn't agree with excluding APR2 use if these
new bugs are (no more than) what optional functions solve.  Nor do I
think now is the right time to make an issue of defects that are equally
present in ap_ldap and in apr_ldap before.

> I am therefore vetoing this move of apr_ldap from APR to httpd.

Hang on!  How long ago did this move happen, and when did you
first raise concerns?

-- 
Nick Kew

Available for work, contract or permanent
http://www.webthing.com/~nick/cv.html


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by Graham Leggett <mi...@sharp.fm>.
On 06 Jun 2011, at 11:53 PM, William A. Rowe Jr. wrote:

>> Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be
>> loaded before mod_authnz_ldap. This is somewhat annoying because the
>> default httpd.conf tries to load mod_authnz_ldap first. Any ideas how
>> to fix this or do we just change the order in the default httpd.conf?
>
> I believe the entire fix may be an entry point to apr_ldap_parse_uri
> (check your own binaries to confirm).  Setting up a single entry point
> should be trivial, if its appropriate.

This is not so, to fix this, you would need to wrap every single LDAP  
API function call[1] in an optional function, and if you did that, you  
would solve the problem that caused you to want to remove apr_ldap  
from APR in the first place, making the whole exercise pointless - you  
may as well just have fixed apr-ldap in place.

In it's current form, this change introduces module ordering bugs to  
httpd that we haven't suffered for a decade.

In addition, the autoconf build is currently broken against apr v1.x  
on MacOSX, and this is probably broken on other platforms as well.  
This introduces serious inconvenience for vendors who have to mess  
about trying to make all of this build all over again on all sorts of  
platforms.

The timing cannot be worse - an already suboptimal API plus these new  
bugs are being dumped into httpd in the final stages of trying to bake  
the final version of httpd v2.4.0, which means we will be stuck with  
this brokenness through the life of httpd v2.4.

There is no need for this move at all, as httpd works perfectly  
against APR v1.x (or did until this change). APR v2.x hasn't gone  
through any kind of stabilisation phase, never mind seen an alpha or  
beta release, and so httpd v2.4.x being compatible with apr-trunk at  
this stage is not necessary, especially seeing that when httpd v2.4 is  
released, it's API is set in stone, but APR v2.0's API remains in  
flux. Or to put it another way, the fact that apr_ldap is missing from  
apr-trunk is not a problem for httpd v2.4, and can be solved after  
httpd v2.4.

I am therefore vetoing this move of apr_ldap from APR to httpd.

We need to focus on getting httpd v2.4 out the door before worrying  
about some future version of APR.

[1] http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldapext-ldap-c-api-05.txt

Regards,
Graham
--


Re: load order dependency between mod_ldap and mod_authnz_ldap

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/6/2011 4:17 PM, Stefan Fritsch wrote:
> Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be 
> loaded before mod_authnz_ldap. This is somewhat annoying because the 
> default httpd.conf tries to load mod_authnz_ldap first. Any ideas how 
> to fix this or do we just change the order in the default httpd.conf?

I believe the entire fix may be an entry point to apr_ldap_parse_uri
(check your own binaries to confirm).  Setting up a single entry point
should be trivial, if its appropriate.