Re: [PATCH] Add an ldapoption to disable chasing LDAP referrals

Hey All,

This patch request grew from this post (of mine) to pgsql-general:

http://www.postgresql.org/message-id/CABUevEzouAe-g1_OejaGujjMem675DNYStwyBp4d_Wz6Om+fxA@mail.gmail.com

The patch adds another available LDAP option (ldapnochaseref) for
search+bind mode in the pg_hba.conf fil. If set to 1 (0 is default) then it
performs a ldap_set_option which disables chasing of any LDAP references
which are returned as part of the search LDIF.

I can think of two use cases for this:

1. (the case which spawned my email) A valid search is performed, but
for some reason a "ref:" with a non responsive LDAP server is returned as
well, which causes the authentication to time out (could be intermittent if
DNS round robin or similar is used and some of the LDAP servers are not
functioning / a packet dropping firewall is in the way).
2. (a case I found when testing with AD) A valid search is performed and
6 "ref:" entries are returned, which all must be chased before
authentication can succeed. Setting ldapnochaseref speeds up authentication
with no negative cost (assuming you understand your LDAP schema).

I think it's work noting that this setting seems to be the default for
ldapsearch on Linux these days.

Hopefully I found all the documentation that I was meant to update, let me
know if not though.

Cheers,

James Sewell
PostgreSQL Team Lead / Solutions Architect
_____________________________________

[image:
http://www.lisasoft.com/sites/lisasoft/files/u1/2013hieghtslogan_0.png]

Level 2, 50 Queen St,
Melbourne, VIC, 3000

P: 03 8370 8000 F: 03 8370 8099 W: www.lisasoft.com

--

------------------------------
The contents of this email are confidential and may be subject to legal or
professional privilege and copyright. No representation is made that this
email is free of viruses or other defects. If you have received this
communication in error, you may not copy or distribute any part of it or
otherwise disclose its contents to anyone. Please advise the sender of your
incorrect receipt of this correspondence.

On 7/2/13 12:20 AM, James Sewell wrote:
> Hey All,
>
> This patch request grew from this post (of mine) to pgsql-general:
>
> http://www.postgresql.org/message-id/CABUevEzouAe-g1_OejaGujjMem675DNYStwyBp4d_Wz6Om+fxA@mail.gmail.com
>
> The patch adds another available LDAP option (ldapnochaseref) for
> search+bind mode in the pg_hba.conf fil. If set to 1 (0 is default) then
> it performs a ldap_set_option which disables chasing of any LDAP
> references which are returned as part of the search LDIF.

This appears to be the same as the "referrals" option in pam_ldap
(http://linux.die.net/man/5/pam_ldap). So it seems legitimate.

For consistency, I would name the option ldapreferrals={0|1}. I prefer
avoiding double negatives.

Do you know of a standard way to represent this option in an LDAP URL,
perhaps as an extension?

Hey Peter,

You are correct, it is the same as the referrals option in pam_ldap. It's
also the -C (sometimes -R - it seems ldapsearch options are pretty
non-standard) option in ldapsearch.

As far as I'm aware you can't pass this in an LDAP URL, primarily because
this never gets sent to the LDAP server. The server always returns an LDIF
with inline references, this just determines if you chase them client side
or just list them as is.

I could be missing something here, but using:

ldapreferrals={0|1}

Would require a three state type, as we need a way of not interfering with
the library defaults? To 'enable' the new behavior here using a boolean you
would need to set ldapreferrals=false - which with the normal way of
dealing with config booleans would alter the default behavior if the option
was not specified.

How do you feel about:

ldapdisablereferrals=(0|1)

Cheers,
James Sewell

James Sewell
PostgreSQL Team Lead / Solutions Architect
_____________________________________

[image:
http://www.lisasoft.com/sites/lisasoft/files/u1/2013hieghtslogan_0.png]

Level 2, 50 Queen St,
Melbourne, VIC, 3000

P: 03 8370 8000 F: 03 8370 8099 W: www.lisasoft.com

On Tue, Jul 2, 2013 at 10:46 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:

> On 7/2/13 12:20 AM, James Sewell wrote:
> > Hey All,
> >
> > This patch request grew from this post (of mine) to pgsql-general:
> >
> >
> http://www.postgresql.org/message-id/CABUevEzouAe-g1_OejaGujjMem675DNYStwyBp4d_Wz6Om+fxA@mail.gmail.com
> >
> > The patch adds another available LDAP option (ldapnochaseref) for
> > search+bind mode in the pg_hba.conf fil. If set to 1 (0 is default) then
> > it performs a ldap_set_option which disables chasing of any LDAP
> > references which are returned as part of the search LDIF.
>
> This appears to be the same as the "referrals" option in pam_ldap
> (http://linux.die.net/man/5/pam_ldap). So it seems legitimate.
>
> For consistency, I would name the option ldapreferrals={0|1}. I prefer
> avoiding double negatives.
>
> Do you know of a standard way to represent this option in an LDAP URL,
> perhaps as an extension?
>
>

--

------------------------------
The contents of this email are confidential and may be subject to legal or
professional privilege and copyright. No representation is made that this
email is free of viruses or other defects. If you have received this
communication in error, you may not copy or distribute any part of it or
otherwise disclose its contents to anyone. Please advise the sender of your
incorrect receipt of this correspondence.

On Wed, Jul 3, 2013 at 3:04 AM, James Sewell <james(dot)sewell(at)lisasoft(dot)com>wrote:

> Hey Peter,
>
> You are correct, it is the same as the referrals option in pam_ldap. It's
> also the -C (sometimes -R - it seems ldapsearch options are pretty
> non-standard) option in ldapsearch.
>
> As far as I'm aware you can't pass this in an LDAP URL, primarily because
> this never gets sent to the LDAP server. The server always returns an LDIF
> with inline references, this just determines if you chase them client side
> or just list them as is.
>
> I could be missing something here, but using:
>
> ldapreferrals={0|1}
>
> Would require a three state type, as we need a way of not interfering with
> the library defaults? To 'enable' the new behavior here using a boolean you
> would need to set ldapreferrals=false - which with the normal way of
> dealing with config booleans would alter the default behavior if the option
> was not specified.
>
> How do you feel about:
>
> ldapdisablereferrals=(0|1)
>
>
I agree with Peter that the negative thing is bad. l don't see the problem,
really. If you don't specify it, you rely on library defaults. If you do
specify it, we lock it to that setting. I don't see the need to
specifically have a setting to rely on library defaults - just remove it
from the line and you get that.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Heya,

I see what you are saying, the problem as I see it is that the action we
are taking here is "disable chasing ldap referrals". If the name is
ldapreferrals and we use a boolean then setting it to 1 reads in a counter
intuitive manner:

"set ldapreferals=true to disable chasing LDAP referrals."

Perhaps you are fine with this though if it's documented? It does work in
the inverse way to pam_ldap, where setting to true enables referral
chasing. pam_ldap works like so:

not set : library default
set to 0 : disable referral chasing
set to 1 : enable referral chasing

The other option would be to have the default value (of the parameter) be
true and set the boolean to false to disable it. I can't find any other
examples of this though - I assume having a one off like this in the code
is a bad thing also?

I'm happy to let you guys decide.

Cheers,
James

James Sewell
PostgreSQL Team Lead / Solutions Architect
_____________________________________

[image:
http://www.lisasoft.com/sites/lisasoft/files/u1/2013hieghtslogan_0.png]

Level 2, 50 Queen St,
Melbourne, VIC, 3000

P: 03 8370 8000 F: 03 8370 8099 W: www.lisasoft.com

On Wed, Jul 3, 2013 at 6:12 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:

>
> On Wed, Jul 3, 2013 at 3:04 AM, James Sewell <james(dot)sewell(at)lisasoft(dot)com>wrote:
>
>> Hey Peter,
>>
>> You are correct, it is the same as the referrals option in pam_ldap.
>> It's also the -C (sometimes -R - it seems ldapsearch options are pretty
>> non-standard) option in ldapsearch.
>>
>> As far as I'm aware you can't pass this in an LDAP URL, primarily because
>> this never gets sent to the LDAP server. The server always returns an LDIF
>> with inline references, this just determines if you chase them client side
>> or just list them as is.
>>
>> I could be missing something here, but using:
>>
>> ldapreferrals={0|1}
>>
>> Would require a three state type, as we need a way of not interfering
>> with the library defaults? To 'enable' the new behavior here using a
>> boolean you would need to set ldapreferrals=false - which with the normal
>> way of dealing with config booleans would alter the default behavior if the
>> option was not specified.
>>
>> How do you feel about:
>>
>> ldapdisablereferrals=(0|1)
>>
>>
> I agree with Peter that the negative thing is bad. l don't see the
> problem, really. If you don't specify it, you rely on library defaults. If
> you do specify it, we lock it to that setting. I don't see the need to
> specifically have a setting to rely on library defaults - just remove it
> from the line and you get that.
>
> --
> Magnus Hagander
> Me: http://www.hagander.net/
> Work: http://www.redpill-linpro.com/
>

--

------------------------------
The contents of this email are confidential and may be subject to legal or
professional privilege and copyright. No representation is made that this
email is free of viruses or other defects. If you have received this
communication in error, you may not copy or distribute any part of it or
otherwise disclose its contents to anyone. Please advise the sender of your
incorrect receipt of this correspondence.

On Thu, Jul 4, 2013 at 2:30 AM, James Sewell <james(dot)sewell(at)lisasoft(dot)com>wrote:

> Heya,
>
> I see what you are saying, the problem as I see it is that the action we
> are taking here is "disable chasing ldap referrals". If the name is
> ldapreferrals and we use a boolean then setting it to 1 reads in a counter
> intuitive manner:
>

That assumes that the default in the ldap library is always going to be to
chase them. Does the standard somehow mandate that it should be?

"set ldapreferals=true to disable chasing LDAP referrals."
>

You'd obviously reverse the meaning as well. "set ldapreferals=false to
disable chasing LDAP referrals."

Perhaps you are fine with this though if it's documented? It does work in
> the inverse way to pam_ldap, where setting to true enables referral
> chasing. pam_ldap works like so:
>
> not set : library default
> set to 0 : disable referral chasing
> set to 1 : enable referral chasing
>
>
That is exactly what I'm suggesting it should do, and I'm pretty sure
that's what Peter suggested as well.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Hey,

New patch attached. I've moved from using a boolean to an enum trivalue.

Let me know what you think.

Cheers,
James

James Sewell
PostgreSQL Team Lead / Solutions Architect
_____________________________________

[image:
http://www.lisasoft.com/sites/lisasoft/files/u1/2013hieghtslogan_0.png]

Level 2, 50 Queen St,
Melbourne, VIC, 3000

P: 03 8370 8000 F: 03 8370 8099 W: www.lisasoft.com

On Thu, Jul 4, 2013 at 8:23 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:

>
> On Thu, Jul 4, 2013 at 2:30 AM, James Sewell <james(dot)sewell(at)lisasoft(dot)com>wrote:
>
>> Heya,
>>
>> I see what you are saying, the problem as I see it is that the action we
>> are taking here is "disable chasing ldap referrals". If the name is
>> ldapreferrals and we use a boolean then setting it to 1 reads in a counter
>> intuitive manner:
>>
>
> That assumes that the default in the ldap library is always going to be to
> chase them. Does the standard somehow mandate that it should be?
>
>
> "set ldapreferals=true to disable chasing LDAP referrals."
>>
>
> You'd obviously reverse the meaning as well. "set ldapreferals=false to
> disable chasing LDAP referrals."
>
>
> Perhaps you are fine with this though if it's documented? It does work in
>> the inverse way to pam_ldap, where setting to true enables referral
>> chasing. pam_ldap works like so:
>>
>> not set : library default
>> set to 0 : disable referral chasing
>> set to 1 : enable referral chasing
>>
>>
> That is exactly what I'm suggesting it should do, and I'm pretty sure
> that's what Peter suggested as well.
>
>
>
> --
> Magnus Hagander
> Me: http://www.hagander.net/
> Work: http://www.redpill-linpro.com/
>

--

------------------------------
The contents of this email are confidential and may be subject to legal or
professional privilege and copyright. No representation is made that this
email is free of viruses or other defects. If you have received this
communication in error, you may not copy or distribute any part of it or
otherwise disclose its contents to anyone. Please advise the sender of your
incorrect receipt of this correspondence.

On Tue, Jul 9, 2013 at 3:33 AM, James Sewell <james(dot)sewell(at)lisasoft(dot)com>wrote:

> Hey,
>
> New patch attached. I've moved from using a boolean to an enum trivalue.
>
> Let me know what you think.
>
>
Please add your patch at
https://commitfest.postgresql.org/action/commitfest_view?id=19 so it's not
missed in the next commitfest!

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, 2013-07-09 at 11:33 +1000, James Sewell wrote:
> New patch attached. I've moved from using a boolean to an enum
> trivalue.

You have updated the documentation to say that the ldareferrals option
only applies in search+bind mode. But looking over the code I think it
applies in both modes. Check please.

On 7/8/13 9:33 PM, James Sewell wrote:
> New patch attached. I've moved from using a boolean to an enum trivalue.

When ldapreferrals is set to something other than "0" or "1" exactly, it
just ignores the option. That's not good, I think. It should probably
be an error.

Hey All,

I had missed these emails, sorry.

The search+bind mode issue is one of documentation location, I have fixed
it by moving the section to the applied to both list. As the patch is to do
with post-auth response this is correct.

As far as the issue when something other than 0 or 1 is set I am happy
throw an error (although this doesn't seem to be how option such as LDAPTLS
work: 1 if 1 else 0). I assume I would use the ereport() function to do
this (using the second example from this page
http://www.postgresql.org/docs/9.2/static/error-message-reporting.html)?

Cheers,
James

James Sewell,
PostgreSQL Team Lead / Solutions Architect
______________________________________

Level 2, 50 Queen St, Melbourne VIC 3000

*P *(+61) 3 8370 8000 * **W* www.lisasoft.com *F *(+61) 3 8370 8099

On Thu, Sep 19, 2013 at 12:56 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:

> On 7/8/13 9:33 PM, James Sewell wrote:
> > New patch attached. I've moved from using a boolean to an enum trivalue.
>
> When ldapreferrals is set to something other than "0" or "1" exactly, it
> just ignores the option. That's not good, I think. It should probably
> be an error.
>
>

--

------------------------------
The contents of this email are confidential and may be subject to legal or
professional privilege and copyright. No representation is made that this
email is free of viruses or other defects. If you have received this
communication in error, you may not copy or distribute any part of it or
otherwise disclose its contents to anyone. Please advise the sender of your
incorrect receipt of this correspondence.

On Thu, 2013-10-17 at 13:49 +1100, James Sewell wrote:

> The search+bind mode issue is one of documentation location, I have
> fixed it by moving the section to the applied to both list. As the
> patch is to do with post-auth response this is correct.
>
Makes sense.

> As far as the issue when something other than 0 or 1 is set I am happy
> throw an error (although this doesn't seem to be how option such as
> LDAPTLS work: 1 if 1 else 0).

Right, that's how ldapreferrals ought to work as well.