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 <[EMAIL PROTECTED]> 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 | | [EMAIL PROTECTED] | 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." | ------------------------------------------------------------------