You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Graham Leggett <mi...@sharp.fm> on 2001/08/10 00:06:37 UTC

[PATCH] LDAP extension to apr-util (take 2)

Hi all,

Here is another snapshot of the library that incorporates LDAP into
apr-util.

This patch is big, but it's an incorporation of a body of new code
which works as a unit.

The build should work correctly both with the default configuration
(LDAP not included) to the configuration --with-ldap. Please test this
for me to be sure.

The header file apr_ldap.h incorporates some high level access to a
number of LDAP functions, like opening connections, checking userids,
and comparing users. Access is also possible to any of the ldap-*()
routines within the LDAP SDK.

The source code is half a vastly modified version of Dave Carrigan's
auth_ldap module, published under the ASF licence. The other half takes
the form of mod_auth_ldap, which is being posted separately to the
new-httpd list.

Currently the code works for me (Linux/PPC) - would be keen to see what
other people's experiences are. It still needs a lot of work - but at
this point feedback will be valuable.

Comments...?

The files are:

include/apr_ldap.h.in
include/private/apu_ldap_cache.h
ldap/apr_ldap.c
ldap/apr_ldap_cache.c
ldap/apr_ldap_cache_mgr.c
ldap/apr_ldap_compat.c
ldap/Makefile.in

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Aug 10, 2001 at 12:06:37AM +0200, Graham Leggett wrote:
> Hi all,
> 
> Here is another snapshot of the library that incorporates LDAP into
> apr-util.

I plan to commit this later today unless I get railroaded by
work-related things or someone screams about not adding this to
apr-util.  -- justin


Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> I'm all for a new apr-ldap CVS module / library. But its presence in APRUTIL
> feels very questionable to me.

Well, since I feel the same way about everything in apr-util, I'm not sure
if I agree or disagree with Greg.  I think it belongs in httpd-ldap, for
the same reason, but if apr-util is going to contain things like xml and
dbm, then it should also contain ldap.  *shrug*

....Roy


Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Ryan Bloom <rb...@covalent.net>.
Now this I can get behind.

On Monday 13 August 2001 13:37, Graham Leggett wrote:
> Ryan Bloom wrote:
> > Then this is the only thing that should be in apr_ldap.  If we are trying
> > to create a wrapper library to abstract out differences in all of the
> > other LDAP libraries, then I _might_ be able to get behind that.
>
> Ok.
>
> > None of that stuff belongs in apr-util.  This has nothing to do with
> > portability, this has to do with attaching LDAP to a web server.  If this
> > is something that we want to provide as a standard part of Apache, not
> > sure we do (haven't thought about it much), then it should be a part of
> > the module that attaches the two together, not a part of the abstraction
> > library.
>
> So this should be part of Apache - now for the next question - should it
> be an Apache module, or part of the Apache core?
>
> > If these routines are not meant to be an abstraction layer for all LDAP
> > libraries, then they do not belong in APR or APR-util.
>
> Ok, I propose this:
>
> - The linking-to-miriad-of-different-ldap-libraries function, and the
> small bit in apr_ldap_compat.c that smooths out differences between
> functions in LDAP v2 and v3 should go in APR-util (or APR?). (This patch
> is small and easy to review.)

+1

>
> - The LDAP connection-reuse / compare cache should be abstracted into
> their own module in the Apache tree (under modules/ldap/) that provides
> additional LDAP services to modules that need them.

+1

Ryan

______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> Ok, I propose this:
> 
> - The linking-to-miriad-of-different-ldap-libraries function, and the
> small bit in apr_ldap_compat.c that smooths out differences between
> functions in LDAP v2 and v3 should go in APR-util (or APR?). (This patch
> is small and easy to review.)

+1

> - The LDAP connection-reuse / compare cache should be abstracted into
> their own module in the Apache tree (under modules/ldap/) that provides
> additional LDAP services to modules that need them.

+1

....Roy


Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Mon, Aug 13, 2001 at 10:48:36PM +0200, Graham Leggett wrote:
> Justin Erenkrantz wrote:
> 
> > ++1.
> 
> So a +1 and a ++1 makes a +++1?

Yeah.  This is what I would have liked to have seen at the outset
anyway, so it merits the special ++1.  -- justin


Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Graham Leggett <mi...@sharp.fm>.
Justin Erenkrantz wrote:

> ++1.

So a +1 and a ++1 makes a +++1?

:)

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Mon, Aug 13, 2001 at 10:37:33PM +0200, Graham Leggett wrote:
> - The linking-to-miriad-of-different-ldap-libraries function, and the
> small bit in apr_ldap_compat.c that smooths out differences between
> functions in LDAP v2 and v3 should go in APR-util (or APR?). (This patch
> is small and easy to review.)

++1.

> - The LDAP connection-reuse / compare cache should be abstracted into
> their own module in the Apache tree (under modules/ldap/) that provides
> additional LDAP services to modules that need them.

+1.  -- justin


Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Graham Leggett <mi...@sharp.fm>.
Ryan Bloom wrote:

> Then this is the only thing that should be in apr_ldap.  If we are trying to
> create a wrapper library to abstract out differences in all of the other
> LDAP libraries, then I _might_ be able to get behind that.

Ok.

> None of that stuff belongs in apr-util.  This has nothing to do with portability,
> this has to do with attaching LDAP to a web server.  If this is something that
> we want to provide as a standard part of Apache, not sure we do (haven't
> thought about it much), then it should be a part of the module that attaches
> the two together, not a part of the abstraction library.

So this should be part of Apache - now for the next question - should it
be an Apache module, or part of the Apache core?

> If these routines are not meant to be an abstraction layer for all LDAP
> libraries, then they do not belong in APR or APR-util.

Ok, I propose this:

- The linking-to-miriad-of-different-ldap-libraries function, and the
small bit in apr_ldap_compat.c that smooths out differences between
functions in LDAP v2 and v3 should go in APR-util (or APR?). (This patch
is small and easy to review.)

- The LDAP connection-reuse / compare cache should be abstracted into
their own module in the Apache tree (under modules/ldap/) that provides
additional LDAP services to modules that need them.

I'd like to get consensus on this before I leap into more code...
(looking for some +1's)

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Ryan Bloom <rb...@covalent.net>.
On Monday 13 August 2001 13:12, Graham Leggett wrote:
> Ryan Bloom wrote:
> > There are already multiple LDAP libraries, what are we trying to solve by
> > putting an abstraction layer into APR?  Are we sure that the problem
> > needs to be solved, or are we doing work just to do work?
>
> There are multiple LDAP libraries - which is exactly why the LDAP
> library was abstracted. There is the "standard" way of doing things,
> then there is the "netscape" way of doing things, which has just changed
> again to the "iplanet" way of doing things - then throw in the two
> different ways of doing SSL/TLS (netscape|standard) - linking against an
> LDAP library is a real pain in the ass.

Then this is the only thing that should be in apr_ldap.  If we are trying to
create a wrapper library to abstract out differences in all of the other
LDAP libraries, then I _might_ be able to get behind that.

> Then there is the idea that we don't want to
> connect/bind/query/unbind/disconnect many times per connection (which
> happens if auth_ldap and keepalives are used together) or if auth_ldap
> and the LDAP-proxy backend are used together - thus the caching and
> connection reuse abstraction layer to do this.

None of that stuff belongs in apr-util.  This has nothing to do with portability,
this has to do with attaching LDAP to a web server.  If this is something that
we want to provide as a standard part of Apache, not sure we do (haven't
thought about it much), then it should be a part of the module that attaches
the two together, not a part of the abstraction library.

> At all times the core LDAP library is available should the application
> want to use it. The LDAP layer is an addition to rather than an
> interface to LDAP. The code for the layer was sourced from an existing
> Apache module - auth_ldap, which has been around for quite a while and
> is stable.

If these routines are not meant to be an abstraction layer for all LDAP
libraries, then they do not belong in APR or APR-util.

Again, I really want to understand why we need this to be a part of 
APR-util.

Ryan
______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Graham Leggett <mi...@sharp.fm>.
Ryan Bloom wrote:

> There are already multiple LDAP libraries, what are we trying to solve by putting
> an abstraction layer into APR?  Are we sure that the problem needs to be solved,
> or are we doing work just to do work?

There are multiple LDAP libraries - which is exactly why the LDAP
library was abstracted. There is the "standard" way of doing things,
then there is the "netscape" way of doing things, which has just changed
again to the "iplanet" way of doing things - then throw in the two
different ways of doing SSL/TLS (netscape|standard) - linking against an
LDAP library is a real pain in the ass.

Then there is the idea that we don't want to
connect/bind/query/unbind/disconnect many times per connection (which
happens if auth_ldap and keepalives are used together) or if auth_ldap
and the LDAP-proxy backend are used together - thus the caching and
connection reuse abstraction layer to do this.

At all times the core LDAP library is available should the application
want to use it. The LDAP layer is an addition to rather than an
interface to LDAP. The code for the layer was sourced from an existing
Apache module - auth_ldap, which has been around for quite a while and
is stable.

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Ryan Bloom <rb...@covalent.net>.
On Monday 13 August 2001 14:13, Justin Erenkrantz wrote:
> On Mon, Aug 13, 2001 at 03:59:08PM -0500, William A. Rowe, Jr. wrote:
> > apr-util should be stable right out the door ... is this ldap code likely
> > to evolve significantly over time?
>
> Probably not.  I doubt there is much more work to be done for the LDAP
> code.  It is a fairly thin layer (although it isn't as thin as I'd like
> it, but that's me).
>
> As I see it, the problem is that if we add any features in httpd that
> require LDAP (mod_auth_ldap, httpd configuration via LDAP), you now
> need to check out apr-ldap.  Yuck.
>
> Based on the way Ryan tells it, apr-util should be the kitchen sink of
> things that don't belong in apr or in httpd.  I think we agree it
> doesn't belong in APR as it is not adding core portability features.
> I don't think it belongs in httpd as I could definitely use such code
> in other projects not associated with httpd.  apr-util seems the most
> logical place, IMHO.  -- justin

Actually, I have a real problem with apr-util too.  I don't want it to be the
kitchen sink of libraries, but that is what it was scoped to be.  I believe that
is a mistake.

The reason we scoped apr-util to be a kitchen sink, is that APR was adding
some stuff that wasn't strictly for portability.  I believe we would have found it
easier to not add everything if we had kept everything in one library.  Then,
we could have used the size of the library as justification for not including a
large component.

There are already multiple LDAP libraries, what are we trying to solve by putting
an abstraction layer into APR?  Are we sure that the problem needs to be solved,
or are we doing work just to do work?

Ryan

______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Mon, Aug 13, 2001 at 03:59:08PM -0500, William A. Rowe, Jr. wrote:
> apr-util should be stable right out the door ... is this ldap code likely to
> evolve significantly over time?

Probably not.  I doubt there is much more work to be done for the LDAP
code.  It is a fairly thin layer (although it isn't as thin as I'd like 
it, but that's me).  

As I see it, the problem is that if we add any features in httpd that
require LDAP (mod_auth_ldap, httpd configuration via LDAP), you now 
need to check out apr-ldap.  Yuck.

Based on the way Ryan tells it, apr-util should be the kitchen sink of
things that don't belong in apr or in httpd.  I think we agree it
doesn't belong in APR as it is not adding core portability features.
I don't think it belongs in httpd as I could definitely use such code
in other projects not associated with httpd.  apr-util seems the most
logical place, IMHO.  -- justin


Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Greg Stein" <gs...@lyra.org>
Sent: Monday, August 13, 2001 3:03 PM
> 
> I'm all for a new apr-ldap CVS module / library. But its presence in APRUTIL
> feels very questionable to me.

++1

I've hesitated to speak out of my a$$, for lack of a more tactful way to put it,
but I have huge portability concerns and no time to review this code so I know
to what I'm speaking.

But to roll something this large into the apr-util lib (lightweight, portable
and widely useful bits of goodness) just seems to be asking for headaches.

apr-util should be stable right out the door ... is this ldap code likely to
evolve significantly over time?

Just my 2c.

Bill



Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Graham Leggett <mi...@sharp.fm>.
Greg Stein wrote:

> I probably missed some conversation while I was away in July, but this is an
> awfully large chunk of code and functionality to put into APRUTIL. I don't
> see how this is sufficiently generic, or a set of utilities. LDAP is a
> really large item with lots of scariness to get things right. I'm worried
> that this limited functionality is going to grow without bounds over time.

The basic need for LDAP support in APR-util is to provide a common way
of binding LDAP to Apache.

The three pieces of functionality I propose to add to Apache that use
LDAP are:

- auth_ldap (complete - allows accesses to be authenticated against LDAP
users and authorised against LDAP groups)
- config from LDAP (next in line - allows Apache to pull lines of config
from objects on an LDAP server, large configfile-less Apache farms are
possible with this)
- serve LDAP content (allows Apache to embed LDAP attributes within
pages using mod_include)

Having LDAP in APR means that the two modules and the core need not
worry about where and how the LDAP server libraries are installed.

The additional code within the LDAP library is largely focused on
solving the problem of reusing LDAP connections, avoiding rebinding to
the LDAP server where it can be avoided, and avoiding compare queries to
the LDAP server through the use of a normal|shared memory cache.

> I'm all for a new apr-ldap CVS module / library. But its presence in APRUTIL
> feels very questionable to me.

It was suggested in the discussion in July that it go in APR-util and
not APR - though I don't have a reference...

If this code does not belong in APR, would it make sense to add it to
the Apache code directly?

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

Re: [PATCH] LDAP extension to apr-util (take 2)

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 10, 2001 at 12:06:37AM +0200, Graham Leggett wrote:
> Hi all,
> 
> Here is another snapshot of the library that incorporates LDAP into
> apr-util.
> 
> This patch is big, but it's an incorporation of a body of new code
> which works as a unit.
> 
> The build should work correctly both with the default configuration
> (LDAP not included) to the configuration --with-ldap. Please test this
> for me to be sure.
> 
> The header file apr_ldap.h incorporates some high level access to a
> number of LDAP functions, like opening connections, checking userids,
> and comparing users. Access is also possible to any of the ldap-*()
> routines within the LDAP SDK.
> 
> The source code is half a vastly modified version of Dave Carrigan's
> auth_ldap module, published under the ASF licence. The other half takes
> the form of mod_auth_ldap, which is being posted separately to the
> new-httpd list.
> 
> Currently the code works for me (Linux/PPC) - would be keen to see what
> other people's experiences are. It still needs a lot of work - but at
> this point feedback will be valuable.
> 
> Comments...?


I probably missed some conversation while I was away in July, but this is an
awfully large chunk of code and functionality to put into APRUTIL. I don't
see how this is sufficiently generic, or a set of utilities. LDAP is a
really large item with lots of scariness to get things right. I'm worried
that this limited functionality is going to grow without bounds over time.

I'm all for a new apr-ldap CVS module / library. But its presence in APRUTIL
feels very questionable to me.

Cheers,
-g

p.s. if there was a long conversation about this in July, then a reference
would be great, and I'll go read the thread(s)

-- 
Greg Stein, http://www.lyra.org/