> This is the first complaint I can recall hearing about that, so
exactly which ones are "many"?
I've tested a 2 before figuring out about the v3 issue. lldap[0] and the docker image osixia/docker-openldap[1].
- lldap gives the following error message when I attempt to connect without the patch "Service Error: while handling incoming messages: while receiving LDAP op: Bind request version is not equal to 3. This is a serious client bug.". With the attached patch this error message does not appear
- osixia/docker-openlap gives the following error message without the patch "67df745e conn=1001 op=0 RESULT tag=97 err=2 text=historical protocol version requested, use LDAPv3 instead".
"
> Also, are we really sufficiently compliant with v3 that just adding this bit is enough?
I believe that this bit is all that is needed. Per the man page for ldap_set_option [2]: "The protocol version used by the library defaults to LDAPv2 (now historic), which corresponds to the LDAP_VERSION2 macro. Application developers are encouraged to explicitly set LDAP_OPT_PROTOCOL_VERSION to LDAPv3, using the LDAP_VERSION3 macro, or to allow users to select the protocol version."
> src/test/ldap/ doesn't do it for you?
Looking through the tests here it seems like they are all tests for the serverside auth functionality that is configurable in pg_hba.conf. I don't see any tests that test the client side "LDAP Lookup of Connection Parameters" described in [3]
Andrew Jackson <andrewjackson947@gmail.com> writes:
> Currently the LDAP usage in fe-connect.c does not explicitly set the
> protocol version to v3. This causes issues with many LDAP servers as they
> will often require clients to use the v3 protocol and disallow any use of
> the v2 protocol.
This is the first complaint I can recall hearing about that, so
exactly which ones are "many"? Also, are we really sufficiently
compliant with v3 that just adding this bit is enough?
> One further note is that I do not currently see any test coverage over the
> LDAP functionality in `fe-connect.c`. I am happy to add that to this patch
> if needed.
src/test/ldap/ doesn't do it for you?
regards, tom lane
