ptclient & ldap changes

Wesley Craig wes at umich.edu
Fri May 30 16:16:54 EDT 2008


On 30 May 2008, at 13:50, Igor Brezac wrote:
> 1) I suggest that you keep ldap_sasl for backward configuration  
> compatibility.  ldap_sasl  name is used for sasl vs non sasl  
> binds.   Note that 'ldap_id' needs to have authorization to proxy.

I left ldap_sasl in place, but removed it's ability to control the  
proxy function.  Instead, it continues to control whether sasl bind  
is used.  Are you suggesting that both ldap_sasl & ldap_proxy_authz  
would be required for proxy to be enabled?  I'd be OK with that,  
presuming that ldap_sasl is indeed required for proxy authz to work.   
The code has a dependency on the proxy authz ldap control, I don't  
recall that it requires sasl.  I suspect not, but I haven't actually  
dug into it.

> 2)  I suppose you can have ldap_group_method: attribute, but this  
> is not how ldap data is typically used for groups.  Also, I suggest  
> that ldap_group_attribute be used instead of  
> ldap_member_attribute.  As you correctly described ldap_group and  
> ldap_member do two different things and your implementation would  
> be a bit confusing.   You can possibly default ldap_group_attribute  
> to the value of ldap_member_attribute.
> I personally do not like ldap_group_method: none, mabe Kan can  
> chime in.  This option basically allows for an arbitrary group  
> identifier (potentially non existing one) to be assigned to a mailbox.

There are two ways to obtain the list of groups a user is a member  
of, corresponding to ldap_member_method "filter" and "attribute".

	filter) A search is executed, typically (uid=%u) in an "ou=groups"  
tree.  The list of DNs found correspond to the groups the user is in.
	attribute) Rather than execute a search, the attributes in the users  
entry are used to form the list of groups the user is in.

The problem is that group name validation doesn't have both models.   
I'm not sure how this discrepancy jibes with your understanding of  
how ldap data is typically used for groups.

As you can see from the patch, I haven't added a  
"ldap_group_attribute" and don't use "ldap_member_attribute".  Both  
ldap_group_method's use ldap_group_filter.  The difference is in how  
the result is treated.

Perhaps you're suggesting that ldap_group_* just be dropped, since  
having two distinct configurations for highly related group  
information is confusing and probably unnecessary.  I'd agree with  
that, but I fixing the current flaw was simpler and more compatible  
than refactoring ptclient/ldap.c entirely.

I don't think the "none" method is any more worrisome than permitting  
someone to assign rights to a group they are not in.

> 3) This seems unnecessary, but can you explain a little more?

It's probably not strictly necessary to change the default size  
limit, but the previous error handling treats "size limit exceeded"  
the same as "not found".  That is a problem, since in some cases the  
"size limit exceeded" is not an error at all.  A very common result  
of the current default & code is that when ldap_member_method is  
"filter", the user gets exactly no groups and no error message  
indicating why.  A more sensible solution would be to make the limit  
10 or 100.  Is there a limit on the number of groups Cyrus allows?   
For ldap_member/group_method of "attribute", a limit of 1 is probably  
smart, since it reduces extraneous traffic.

:wes

> Wesley Craig wrote:
>> I have a number of ptclient & ldap bug fixes and improvements to  
>> make:
>>
>>     1) In 2.3.12p2, if ldap_sasl is enabled, user DNs are obtained  
>> through SASL authN/Z proxying.  This assumes that the LDAP server  
>> supports authN/Z proxying and that ptclient/ldap has authorization  
>> to proxy for all users.  I've moved this option under a new  
>> configuration option, ldap_proxy_authz, since the authZ proxying  
>> is more or less orthogonal to using SASL for LDAP authN.
>>
>>     2) Groups have two LDAP configurations, one for populating the  
>> groups a user belongs to and a second for validating a (new) group  
>> name.  In 2.3.12p2, those two configurations suffer from non- 
>> parallel construction.  In particular, ldap_member_method allows  
>> both "attribute" and "filter", while the ldap_group_*  
>> configuration has no "_method" configuration, implicitly assuming  
>> "filter" instead.  I've added a ldap_group_method configuration,  
>> with three options, "filter", "attribute" and "none".  "none"  
>> allows any string that can be canonicalized to be used.  "filter"  
>> works just like ldap_group_* was working -- exactly one DN may be  
>> returned.  "attribute" looks for at least one DN to be returned.   
>> A correct "attribute" configuration searches for the attribute  
>> used in ldap_member_attribute.  The assumption is that if anyone  
>> has the group attribute, it is a valid group name.
>>
>>     3) I changed the default ldap_size_limit to 2.  I also  
>> inserted some additional checks in the code to specifically look  
>> for cases where size limit is exceeded.  These may or may not be  
>> errors, depending on what you're looking for.
>>
>>     4) I fixed two small bugs in ptloader.c, one where unused  
>> memory to syslog'd and another where the error message returned  
>> from the ptloader module isn't null terminated when being passed  
>> to auth_pts.c.


More information about the Cyrus-devel mailing list