ptclient & ldap changes

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.

Please find the patch attached.  Comments?

:wes

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.

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.

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

-Igor

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.
>
> Please find the patch attached.  Comments?
>
> :wes

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.

Wesley Craig wrote:
> 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.

sasl used to be required for ldap proxy authz, but I do not
think this 
is the case any more.  I suggested that both ldap_sasl and 
ldap_proxy_authz do the same thing.
>
>> 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 suppose you could have both methods, but is that really
needed?   Each 
group should have one entry in DIT.

> 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.

It'd be nice if there is a simpler way to configure group
(membership 
and validation) in pt/ldap.  

>
> 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.
>

Now looking at the code both ldap_member methods are broken.
:( 
- The 'attribute' method stores group DNs in authstate
struct which is 
not correct.  Additional search is needed to get group
names.
- The 'filter' method, as you pointed out, gets one group at
most.  
Changing ldap sitezlimit would fix this issue.

Cyrus does not limit on the number of groups.  I suppose
LDAP_NO_LIMIT 
can be used to match other mechanisms, or implement
configurable limit 
that can be used in other mechanisms.

It'd be nice if all this complexity can be moved to the ldap

configuration, similar to the way sasl simplified things for
user stuff, 
Dynlist/dyngroups looks interesting...

-Igor

> :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.

On 31 May 2008, at 00:06, Igor Brezac wrote:
> sasl used to be required for ldap proxy authz, but I do
not think  
> this is the case any more.  I suggested that both
ldap_sasl and  
> ldap_proxy_authz do the same thing.

Perhaps I misunderstand you.  Since SASL authN and proxy
authZ are  
more or less completely orthogonal, why would you have them
do the  
same thing?  I propose that ldap_sasl control the way bind
is done.   
And ldap_proxy_authz is used to control how user DNs are
obtained.

> I suppose you could have both methods, but is that
really needed?    
> Each group should have one entry in DIT.

If you're using the "attribute" method for group
population, the  
group may (should?) have zero entries in the DIT.  That's
just  
exactly my point.  The code I've added is a (clever) way to
validate  
group names when groups don't have their own DNs -- which is
 
frequently the case when membership is recorded in the
user's entry.

>> ... fixing the current flaw was simpler and more
compatible than  
>> refactoring ptclient/ldap.c entirely.
>
> It'd be nice if there is a simpler way to configure
group  
> (membership and validation) in pt/ldap.  

True.  Feel free to propose something.  I've fixed the
problem I  
have, and improved the code generally.  Further improvements
are  
welcome, of course 

> Now looking at the code both ldap_member methods are
broken. :( -  
> The 'attribute' method stores group DNs in authstate
struct which  
> is not correct.

You're misunderstanding how
ptsmodule_make_authstate_attribute()  
works.  The "attribute" method retrieves the
attribute named in  
ldap_member_attribute from the user's DN.  The syntax of
that  
attribute is assumed to be in a format suitable for
auth_state.  No  
additional search is necessary.

> - The 'filter' method, as you pointed out, gets one
group at most.

Again, not exactly.  If you use the default size limit and
are in  
more than one group, you get no groups -- size limit is
exceeded.   
LDAP_NO_LIMIT might be a useful way to handle this case, but
you're  
still left wondering how to handle the case where the
server's size  
limit is exceeded.  Do I populate what I got back?  Do I
populate  
nothing?

> It'd be nice if all this complexity can be moved to the
ldap  
> configuration, similar to the way sasl simplified
things for user  
> stuff, Dynlist/dyngroups looks interesting...

sasl simplifies what?  IMHO, using sasl makes almost
everything more  
complex.

Cyrus supports two schemes for using groups.  Rather than
have more  
or less every configurable LDAP knob visible in imapd.conf,
I think  
it would be better if it "just worked".  However,
I don't think LDAP  
is very close to that goal as a technology.  A more
achievable goal  
for Cyrus's use of LDAP would be posting recipes for the two
schemes  
in the wiki.

:wes

Wesley Craig wrote:
> On 31 May 2008, at 00:06, Igor Brezac wrote:
>> sasl used to be required for ldap proxy authz, but
I do not think 
>> this is the case any more.  I suggested that both
ldap_sasl and 
>> ldap_proxy_authz do the same thing.
>
> Perhaps I misunderstand you.  Since SASL authN and
proxy authZ are 
> more or less completely orthogonal, why would you have
them do the 
> same thing?  I propose that ldap_sasl control the way
bind is done.  
> And ldap_proxy_authz is used to control how user DNs
are obtained.
Your patch breaks existing configurations, we usually try to
preserve 
configuration compatibility when possible.  Otherwise I am
fine with 
your approach.   Maybe automatically set ldap_proxy_authz to
true when 
ldap_sasl is turned on and when ldap_proxy_authz is not
explicitly 
specified in the config?
>
>> I suppose you could have both methods, but is that
really needed?   
>> Each group should have one entry in DIT.
>
> If you're using the "attribute" method for
group population, the group 
> may (should?) have zero entries in the DIT.  That's
just exactly my 
> point.  The code I've added is a (clever) way to
validate group names 
> when groups don't have their own DNs -- which is
frequently the case 
> when membership is recorded in the user's entry.

OK.  This makes sense.
>
>>> ... fixing the current flaw was simpler and
more compatible than 
>>> refactoring ptclient/ldap.c entirely.
>>
>> It'd be nice if there is a simpler way to configure
group (membership 
>> and validation) in pt/ldap.  
>
> True.  Feel free to propose something.  I've fixed the
problem I have, 
> and improved the code generally.  Further improvements
are welcome, of 
> course 

I was hoping you can make some improvement since I wrote the
original 
code.    
Unfortunately I have not had time to mess with it in years. 
:(

>
>> Now looking at the code both ldap_member methods
are broken. :( - The 
>> 'attribute' method stores group DNs in authstate
struct which is not 
>> correct.
>
> You're misunderstanding how
ptsmodule_make_authstate_attribute() 
> works.  The "attribute" method retrieves the
attribute named in 
> ldap_member_attribute from the user's DN.  The syntax
of that 
> attribute is assumed to be in a format suitable for
auth_state.  No 
> additional search is necessary.

True, although it is broken when values of
ldap_member_attribute are 
represented with DN.  You are probably right, not worth
messing with.

>
>> - The 'filter' method, as you pointed out, gets one
group at most.
>
> Again, not exactly.  If you use the default size limit
and are in more 
> than one group, you get no groups -- size limit is
exceeded.

If you are a member of one group, it works.  We are saying
the same 
thing.  Bottom line and I think you'll agree, the code needs
improvement.

> LDAP_NO_LIMIT might be a useful way to handle this
case, but you're 
> still left wondering how to handle the case where the
server's size 
> limit is exceeded.  Do I populate what I got back?  Do
I populate 
> nothing?
>

I suppose LDAP_NO_LIMIT is lesser of two evils.  It seems
impractical 
for a user to be member of several hundred groups...

>> It'd be nice if all this complexity can be moved to
the ldap 
>> configuration, similar to the way sasl simplified
things for user 
>> stuff, Dynlist/dyngroups looks interesting...
>
> sasl simplifies what?  IMHO, using sasl makes almost
everything more 
> complex.

I respectfully disagree, maybe it is just me.    I suppose
for 
openldap admins it may be more complex. 
In terms of cyrus imap admin, all they should need to know
is ldap 
username/pass.  They should not need to have any knowledge
of DIT, 
filters, bind dn, etc...

>
> Cyrus supports two schemes for using groups.  Rather
than have more or 
> less every configurable LDAP knob visible in
imapd.conf, I think it 
> would be better if it "just worked". 
However, I don't think LDAP is 
> very close to that goal as a technology.  A more
achievable goal for 
> Cyrus's use of LDAP would be posting recipes for the
two schemes in 
> the wiki.
>

Agreed, but I do not think it is far off.

-Igor

On 31 May 2008, at 17:25, Igor Brezac wrote:
> Wesley Craig wrote:
>> On 31 May 2008, at 00:06, Igor Brezac wrote:
>>> sasl used to be required for ldap proxy authz,
but I do not think  
>>> this is the case any more.  I suggested that
both ldap_sasl and  
>>> ldap_proxy_authz do the same thing.
>>
>> Perhaps I misunderstand you.  Since SASL authN and
proxy authZ are  
>> more or less completely orthogonal, why would you
have them do the  
>> same thing?  I propose that ldap_sasl control the
way bind is  
>> done.  And ldap_proxy_authz is used to control how
user DNs are  
>> obtained.
> Your patch breaks existing configurations, we usually
try to  
> preserve configuration compatibility when possible. 
Otherwise I am  
> fine with your approach.   Maybe automatically set
ldap_proxy_authz  
> to true when ldap_sasl is turned on and when
ldap_proxy_authz is  
> not explicitly specified in the config?

Well, that's an issue.  We could make ldap_proxy_authz
tri-valued:  
legacy, on, and off.  Legacy would be the default and would
revert to  
the old behavior.  Of course, that means that it wouldn't
support  
imapd.conf's typical 0/1, on/off, t/f "switch"
syntax.

> True, although it is broken when values of
ldap_member_attribute  
> are represented with DN.  You are probably right, not
worth messing  
> with.

I feel as tho if ldap_member_attribute is a DN, then maybe
the proper  
scheme is ldap_member_method: filter.  Obviously, if the
group entry  
doesn't have a reference to the members, that won't work. 
But I  
think that jibes with my point the pt/ldap supports (more or
less)  
two schemes for group membership.  Adding a variation on the
(already  
kind of broken) ldap_member_method: attribute scheme which 

dereferences ldap_member_attribute's when they are DNs is a
good  
feature.  Someone should need that feature, tho 

>> LDAP_NO_LIMIT might be a useful way to handle this
case, but  
>> you're still left wondering how to handle the case
where the  
>> server's size limit is exceeded.  Do I populate
what I got back?   
>> Do I populate nothing?
>
> I suppose LDAP_NO_LIMIT is lesser of two evils.  It
seems  
> impractical for a user to be member of several hundred
groups...

The complement is not true, tho: it's very practical in the 

ldap_member_method: attribute scheme for there to be a lot
of users  
in a given group.  Perhaps the configurable size limit
option should  
be removed.  If ldap_member_method is filter, there should
be no  
(LDAP client imposed) size limit.  If ldap_member_method is 

attribute, the limit should be 1.

:wes

Wesley Craig wrote:
> On 31 May 2008, at 17:25, Igor Brezac wrote:
>> Wesley Craig wrote:
>>> On 31 May 2008, at 00:06, Igor Brezac wrote:
>>>> sasl used to be required for ldap proxy
authz, but I do not think 
>>>> this is the case any more.  I suggested
that both ldap_sasl and 
>>>> ldap_proxy_authz do the same thing.
>>>
>>> Perhaps I misunderstand you.  Since SASL authN
and proxy authZ are 
>>> more or less completely orthogonal, why would
you have them do the 
>>> same thing?  I propose that ldap_sasl control
the way bind is done.  
>>> And ldap_proxy_authz is used to control how
user DNs are obtained.
>> Your patch breaks existing configurations, we
usually try to preserve 
>> configuration compatibility when possible. 
Otherwise I am fine with 
>> your approach.   Maybe automatically set
ldap_proxy_authz to true 
>> when ldap_sasl is turned on and when
ldap_proxy_authz is not 
>> explicitly specified in the config?
>
> Well, that's an issue.  We could make ldap_proxy_authz
tri-valued: 
> legacy, on, and off.  Legacy would be the default and
would revert to 
> the old behavior.  Of course, that means that it
wouldn't support 
> imapd.conf's typical 0/1, on/off, t/f
"switch" syntax.
>
That'll work.
>>> LDAP_NO_LIMIT might be a useful way to handle
this case, but you're 
>>> still left wondering how to handle the case
where the server's size 
>>> limit is exceeded.  Do I populate what I got
back?  Do I populate 
>>> nothing?
>>
>> I suppose LDAP_NO_LIMIT is lesser of two evils.  It
seems impractical 
>> for a user to be member of several hundred
groups...
>
> The complement is not true, tho: it's very practical in
the 
> ldap_member_method: attribute scheme for there to be a
lot of users in 
> a given group.  Perhaps the configurable size limit
option should be 
> removed.  If ldap_member_method is filter, there should
be no (LDAP 
> client imposed) size limit.  If ldap_member_method is
attribute, the 
> limit should be 1.
>
I like this suggestion.

-Igor

Wesley Craig wrote:
> We could make ldap_proxy_authz tri-valued: legacy, on,
and off.   
> Legacy would be the default and would revert to the old
behavior.   
> Of course, that means that it wouldn't support
imapd.conf's typical  
> 0/1, on/off, t/f "switch" syntax.

> The complement is not true, tho: it's very practical in
the  
> ldap_member_method: attribute scheme for there to be a
lot of users  
> in a given group.  Perhaps the configurable size limit
option  
> should be removed.  If ldap_member_method is filter,
there should  
> be no (LDAP client imposed) size limit.  If
ldap_member_method is  
> attribute, the limit should be 1.

Find the patch implementing the above (and the other stuff
we  
discussed earlier) attached.

:wes