Re: LDAP: bugfix and deprecated OpenLDAP API

Magnus Hagander wrote:
> On Tue, Feb 5, 2013 at 10:39 AM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
>> I found a small bug in the implementation of LDAP connection
>> parameter lookup.

[...]

>> As coded now, the timeout won't work - if the LDAP server
>> is down, ldap_simple_bind will wait for the network
>> timeout, which will be quite longer than 2 seconds.
>>
>> The attached patch ldap-bug.patch fixes this problem;
>> unfortunately I found no way that works both with OpenLDAP
>> and Windows LDAP, so I had to add an #ifdef.
>>
>> I think that this patch should be applied and backpatched.
>
> So just to be clear - the difference is we're going from implicit
> anonymous bind, to an explicit one? We're not actually causing an
> extra bind compared to previous versions?

No, it was an explicit bind before as well.
The bug I discovered is that if you ldap_simple_bind, the
timeout set in ldap_result does not work for network timeouts.

It always waits until the TCP connection is established or fails,
which can take much longer than PGLDAP_TIMEOUT seconds.

With OpenLDAP you can set the LDAP_OPT_NETWORK_TIMEOUT option
to enforce a timeout, but on Windows you have to use the
nonstandard function ldap_connect.

>> I also tried to fix the problem mentioned in
>> http://www.postgresql.org/message-id/CA+TgmoYnj=Es3L_0Q8+ijR4tVhvztW1fb=7C9K9gEmZWqhpwuQ@mail.gmail.com
>> that we use deprecated OpenLDAP functions, see the attached
>> ldap-undeprecate.patch.
>>
>> I added a file ldap.c in src/port with my own implementation
>> of some of the functions that OpenLDAP has deprecated.
>> With that, the code changes necessary are pretty minimal.
>
> Doesn't this need a version check against OpenSSL at some point, or a
> configure check? Are we just assuming that all versions that people
> ever use have the function deprecated? (That's probably not entirely
> unreasonable, just double checking)

I checked, and it should work fine since OpenLDAP 2.0.0, released in 2000.
The current version is 2.4.35.
In 2.0.0 the old API was not yet deprecated, but the new functions are
already there so that PostgreSQL should work.

I don't know if that would require a check, but it should be simple
to add a configure test if we are sufficiently paranoid.

I'll be on vacation from Wednesday on until July 20th.

Yours,
Laurenz Albe