Thread: 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.
Attachment
On Mon, Nov 12, 2012 at 10:38 PM, Peter Eisentraut <peter_e@gmx.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.albe@wien.gv.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>]@]<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