Adding a new user DN cache to authnz_ldap...

[Brad Nicholes <BN...@novell.com>]

   I see your point about the req structure being tied to closely to
authentication phase so that authorization will not work without it. 
Now I understand the need for util_ldap_cache_getuserdn() which is the
real patch that needs to be added.  In other words, the relevant bug is
actually 28253 not 31898.  The question now is should the same search
cache used by util_ldap_cache_checkuserid() also be referenced in
util_ldap_cache_getuserdn().  I guess it could be, but I am thinking
that even though some user DN's might be stored in two different caches,
it would be safer to create a new cache just for user DN's.  This way,
util_ldap_cache_checkuserid() can continue to enforce the no NULL or
blank passwords as it was originally designed, as well as avoiding a
potential false positive if util_ldap_cache_getuserdn() were to insert a
NULL password entry thus allowing a user without a password to pass
authentication when the cache is checked by
util_ldap_cache_checkuserid().  

Brad

>>> Jari Ahonen <ja...@progress.com> Friday, October 29, 2004 8:53:41 AM
>>>
Brad,

What I've written below might sound that I'm just arguing that
things should be done my way. Don't take it like that, I'm just
trying to make it easier for you to understand my viewpoint on
this issue.

Brad Nicholes wrote:
> So basically what you did was write a function that puts bad data 
> into the cache that the cache was never designed to handle or allow.

I'm not sure I would agree with that. But then, I haven't seen
the design specifications for the cache. I based my assumptions
on comments in util_ldap_cache.h which I thought implied that
NULL bindpw is allowed in a cache record.

> I would suggest closing the first bug that you wrote and opening 
> another one that states that you would like to add new functionality
>  to util_ldap through a functioned call util_ldap_cache_getuserdn().

That's what I tried some time back with bug 28253 but it didn't
go very far...

> No, there are two other caches that are implemented by util_ldap. 
> They are the compare cache and the comparedn cache.  These caches
are
>  primarily used during the authorization phase rather than the 
> authentication phase.

True. And I understand that these caches exist and what they
are used for. But what is missing in the current util_ldap
implementation is a function that allows you to get the user DN
out of the directory without having to supply the correct user
password. None of the functions defined in include/util_ldap.h
do that, yet this is what is needed in order to do LDAP group
authorizations.

Even the Apache 2.1 mod_authnz_ldap will not be able to authorize
a request if that request is authenticated with some other module.
This is because the user DN (In authn_ldap_request_t *req structure)
is attached to the request in authentication phase and recalled
later in authorization phase. If the request is not authenticated
with mod_authnz_ldap, the req structrure does not exist and with
current util_ldap there is no way to get the DN without knowing
the user password. The end result of all this is that it is
impossible to do authorization based on LDAP groups if you are
authenticating with some other module.

So we need some way of creating that authn_ldap_request_t structure
in the authorization phase of the request. I thought the easiest
way of doing this would be making a copy of
util_ldap_cache_checkuserid() without password checking that would
store the results in the cache with bindpw=NULL. But doing so
caused the problems I reported in bug 31898. Storing bindpw=""
would cause slightly different problem because then the current
util_ldap_cache_checkuserid would allow anyone in with blank
password as long as the user had authenticated once and the cache
record hasn't expired.

Alternative fix for my problem is for myself to store the cache
entries with bindpw="" and move the blank password check that
is now somewhere in the middle of util_ldap_cache_checkuserid()
into the beginning of the function, before the cache check. In
fact maybe that blank password check should be one of the first
checks anyway since it makes no sense to do all that cache and
LDAP searching if blank passwords aren't allowed at all.

- Jari

-- 
------------------------------------------------------------------
| Jari Ahonen               | Progress Software Europe           |
| jah@progress.com          | Schorpioenstraat 67                |
| Tel: +31-10-2865 700      | 3067 GG Rotterdam                  |
| Fax: +31-10-2865 225      | The Netherlands                    |
|----------------------------------------------------------------|
|       "Once you've made the commitment to recreation,          |
|                 there is no turning back."                     |
------------------------------------------------------------------