support for LDAP URLs

Here is a patch to support RFC 2255 LDAP URLs in pg_hba.conf. So,
instead of, say

host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapsearchattribute=uid

you could write

host ... ldap lapurl="ldap://ldap.example.net/dc=example,dc=net?uid?sub"

Apache and probably other software uses the same format, and it's easier
to have a common format for all such configuration instead of having to
translate the information provided by the LDAP admin into each
software's particular configuration spellings.

I'm using the OpenLDAP-provided URL parsing routine, which means this
wouldn't be supported on Windows. But we already support different
authentication settings on different platforms, so this didn't seem such
a big problem.

On Mon, Nov 12, 2012 at 10:38 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> Here is a patch to support RFC 2255 LDAP URLs in pg_hba.conf. So,
> instead of, say
>
> host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapsearchattribute=uid
>
> you could write
>
> host ... ldap lapurl="ldap://ldap.example.net/dc=example,dc=net?uid?sub"
>
> Apache and probably other software uses the same format, and it's easier
> to have a common format for all such configuration instead of having to
> translate the information provided by the LDAP admin into each
> software's particular configuration spellings.
>
> I'm using the OpenLDAP-provided URL parsing routine, which means this
> wouldn't be supported on Windows. But we already support different
> authentication settings on different platforms, so this didn't seem such
> a big problem.

I think this is broadly reasonable, but I'm not sure this part is a good idea:

+#ifdef USE_LDAP
+#ifndef WIN32
+/* We use a deprecated function to keep the codepath the same as win32. */
+#define LDAP_DEPRECATED 1
+#include <ldap.h>
+#else
+#include <winldap.h>
+#endif
+#endif

Presumably if it's deprecated now, it might go away without notice,
and we shouldn't be relying on it to stick around.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, 2012-11-15 at 14:44 -0500, Robert Haas wrote:
> I think this is broadly reasonable, but I'm not sure this part is a
> good idea:
>
> +#ifdef USE_LDAP
> +#ifndef WIN32
> +/* We use a deprecated function to keep the codepath the same as
> win32. */
> +#define LDAP_DEPRECATED 1
> +#include <ldap.h>
> +#else
> +#include <winldap.h>
> +#endif
> +#endif
>
> Presumably if it's deprecated now, it might go away without notice,
> and we shouldn't be relying on it to stick around.

This part was copied from auth.c; it's been like that forever.

Since Windows has no support for URL parsing, this could actually be
simplified as far as hba.c goes, but the underlying issue is a different
battle.

Robert Haas wrote:
>> Here is a patch to support RFC 2255 LDAP URLs in pg_hba.conf.

> I think this is broadly reasonable, but I'm not sure this part is a
good idea:
>
> +#ifdef USE_LDAP
> +#ifndef WIN32
> +/* We use a deprecated function to keep the codepath the same as
win32. */
> +#define LDAP_DEPRECATED 1
> +#include <ldap.h>
> +#else
> +#include <winldap.h>
> +#endif
> +#endif
>
> Presumably if it's deprecated now, it might go away without notice,
> and we shouldn't be relying on it to stick around.

I did the same thing in src/interfaces/libpq/fe-connect.c

I think I remember that problem was that OpenLDAP has deprecated
some API functions, and Windows didn't support the replacements.

Both RFC 1823 and the draft
http://tools.ietf.org/html/draft-ietf-ldapext-ldap-c-api-05
(the latest version I found is from 2001) had these functions
as not deprecated, so I figured it was safe to use it.

Moreover, RFC 1823 didn't contain the replacement functions,
so I didn't feel safe to use them.

I just checked, and the only function I could find that is
deprecated in OpenLDAP, but its replacement is not defined
on Windows, is ldap_unbind.

The alternative to using the deprecated functions would be to
write platform dependent macros that do the right thing.
If ldap_unbind really is the only problem, maybe all LDAP code
should be rewritten to avoid LDAP_DEPRECATED.

What do you think?
Do we feel bound to adhere to RFC 1823?

Yours,
Laurenz Albe

On Fri, Nov 16, 2012 at 4:36 AM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
> What do you think?
> Do we feel bound to adhere to RFC 1823?

Well, I guess if we're already using that piece of ugliness elsewhere
there's not much harm in propagating it here, too. The danger of
course is that these APIs will go away under us and then we'll have to
scramble to come up with a fix, so maybe it would be worth trying to
plan ahead for that, but that's probably a job for a separate patch,
so I'm OK with this one as it is.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Peter Eisentraut wrote:
> Here is a patch to support RFC 2255 LDAP URLs in pg_hba.conf. So,
> instead of, say
>
> host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapsearchattribute=uid
>
> you could write
>
> host ... ldap lapurl="ldap://ldap.example.net/dc=example,dc=net?uid?sub"

Should we be referencing RFC 4516 instead?

I'm not very fond of the way this entry is worded:

> + <varlistentry>
> + <term><literal>ldapurl</literal></term>
> + <listitem>
> + <para>
> + You can write most of the LDAP options alternatively using an RFC 2255
> + LDAP URL. The format is
> +<synopsis>
> +ldap://[<replaceable>user</replaceable>[:<replaceable>password</replaceable>](at)]<replaceable>host</replaceable>[:<replaceable>port</replaceable>]/<replaceable>basedn</replaceable>[?[<replaceable>attribute</replaceable>][?[<replaceable>scope</replaceable>]]]
> +</synopsis>
> + <replaceable>scope</replaceable> must be one
> + of <literal>base</literal>, <literal>one</literal>, <literal>sub</literal>,
> + typically the latter. Only one attribute is used, and some other
> + components of standard LDAP URLs such as filters and extensions are
> + not supported.
> + </para>

It seems completely unlike the rest, and it doesn't read like a
reference entry. How about starting with para containing just "An RFC
4516 LDAP URL", or something like that, and then expanding on the
details of the format outside the <varlist>?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, 2012-11-26 at 18:15 -0300, Alvaro Herrera wrote:
> Should we be referencing RFC 4516 instead?

True.

> I'm not very fond of the way this entry is worded:

Good point. I've rewritten it a little bit.

2012-11-13 04:38 keltezéssel, Peter Eisentraut írta:
> Here is a patch to support RFC 2255 LDAP URLs in pg_hba.conf. So,
> instead of, say
>
> host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapsearchattribute=uid
>
> you could write
>
> host ... ldap lapurl="ldap://ldap.example.net/dc=example,dc=net?uid?sub"
>
> Apache and probably other software uses the same format, and it's easier
> to have a common format for all such configuration instead of having to
> translate the information provided by the LDAP admin into each
> software's particular configuration spellings.
>
> I'm using the OpenLDAP-provided URL parsing routine, which means this
> wouldn't be supported on Windows. But we already support different
> authentication settings on different platforms, so this didn't seem such
> a big problem.

This patch was committed today but it fails to compile for non-ldap configs:

$ ./configure --prefix=$HOME/pg93dev --enable-debug --enable-cassert --enable-depend

make[3]: Entering directory
`/home/zozo/crosscolumn/psql-c-relax/postgresql.1/src/backend/libpq'
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement
-Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv
-fexcess-precision=standard -g -I../../../src/include -D_GNU_SOURCE -c -o hba.o hba.c
-MMD -MP -MF .deps/hba.Po
hba.c: In function ‘parse_hba_auth_opt’:
hba.c:1388:23: error: ‘LDAP_SCOPE_SUBTREE’ undeclared (first use in this function)
hba.c:1388:23: note: each undeclared identifier is reported only once for each function it
appears in
hba.c:1451:3: error: unknown type name ‘LDAPURLDesc’
hba.c:1452:7: warning: unused variable ‘rc’ [-Wunused-variable]
hba.c:1451:16: warning: unused variable ‘urldata’ [-Wunused-variable]
make[3]: *** [hba.o] Error 1

The code could use some #ifdef USE_LDAP conditionals.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

On 2012-12-04 10:18:36 +0100, Boszormenyi Zoltan wrote:
> 2012-11-13 04:38 keltezéssel, Peter Eisentraut írta:
> >Here is a patch to support RFC 2255 LDAP URLs in pg_hba.conf. So,
> >instead of, say
> >
> >host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapsearchattribute=uid
> >
> >you could write
> >
> >host ... ldap lapurl="ldap://ldap.example.net/dc=example,dc=net?uid?sub"
> >
> >Apache and probably other software uses the same format, and it's easier
> >to have a common format for all such configuration instead of having to
> >translate the information provided by the LDAP admin into each
> >software's particular configuration spellings.
> >
> >I'm using the OpenLDAP-provided URL parsing routine, which means this
> >wouldn't be supported on Windows. But we already support different
> >authentication settings on different platforms, so this didn't seem such
> >a big problem.
>
> This patch was committed today but it fails to compile for non-ldap configs:
>
> $ ./configure --prefix=$HOME/pg93dev --enable-debug --enable-cassert --enable-depend
>
> make[3]: Entering directory
> `/home/zozo/crosscolumn/psql-c-relax/postgresql.1/src/backend/libpq'
> gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
> -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
> -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard
> -g -I../../../src/include -D_GNU_SOURCE -c -o hba.o hba.c -MMD -MP -MF
> .deps/hba.Po
> hba.c: In function ‘parse_hba_auth_opt’:
> hba.c:1388:23: error: ‘LDAP_SCOPE_SUBTREE’ undeclared (first use in this function)
> hba.c:1388:23: note: each undeclared identifier is reported only once for
> each function it appears in
> hba.c:1451:3: error: unknown type name ‘LDAPURLDesc’
> hba.c:1452:7: warning: unused variable ‘rc’ [-Wunused-variable]
> hba.c:1451:16: warning: unused variable ‘urldata’ [-Wunused-variable]
> make[3]: *** [hba.o] Error 1
>
> The code could use some #ifdef USE_LDAP conditionals.

As I needed to base some stuff on a later commit (5ce108bf3) and I
didn't want to include a revert in the tree, here's what I applied
locally to fix this. Maybe somebody can apply something like that to get
the buildfarm green again?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services