Since the checkuserid cache does not allow NULL or blank passwords today and its purpose is for authentication, I'm just not completely comfortable allowing another function whose purpose it is to simply lookup user DN's to insert NULL or blank password entries. It seems like it would potentially open up holes where there wasn't a problem before. Adding a separate cache would just eliminate any potential problems.
Brad >>> Jari Ahonen <[EMAIL PROTECTED]> Friday, October 29, 2004 2:51:47 PM >>> Brad Nicholes wrote: > 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. In a way they are both relevant because applying the changes needed for 28253 will cause the behaviour in 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. I think the same search cache should be used. To me it makes no sense to have two caches with more or less the same data in them. Having a record in search cache with bindpw=NULL should be a good enough indicator that the particular record has not been through password verification (In fact the commets in util_ldap_cache.h suggest this might have been planned at some point). It is easy enough to check if bindpw exists in a cache record in util_ldap_cache_checkuserid() to prevent access with blank passwords. And a cache record created by util_ldap_cache_checkuserid() can be used with util_ldap_cache_getuserdn() since that function does not need to care about passwords. I have had a server running with this kind of setup for quite a while now, it was just that the shared cache changes done in 2.0.52 added the extra check that started to crash my server when util_ldap_cache_checkuserid() tries to strcmp() that NULL password. - 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." | ------------------------------------------------------------------
