Re: [HACKERS] support for LDAP URLs

2012-12-04 Thread Boszormenyi Zoltan

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/



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] support for LDAP URLs

2012-12-04 Thread Andres Freund
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
From 1222c05b6335297e19fd843d3971dab3f5f5a4a7 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Tue, 4 Dec 2012 11:49:38 +0100
Subject: [PATCH] fix build without ldap support after a2fec0a18e4d

---
 src/backend/libpq/hba.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 2bb661c..36ece55 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1385,7 +1385,9 @@ parse_hba_line(List *line, int line_num)
 static bool
 parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
 {
+#ifdef USE_LDAP
 	hbaline-ldapscope = LDAP_SCOPE_SUBTREE;
+#endif
 
 	if (strcmp(name, map) == 0)
 	{
@@ -1448,12 +1450,12 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
 	}
 	else if (strcmp(name, ldapurl) == 0)
 	{
+#ifdef LDAP_API_FEATURE_X_OPENLDAP
 		LDAPURLDesc *urldata;
 		int rc;
 
 		REQUIRE_AUTH_OPTION(uaLDAP, ldapurl, ldap);
 
-#ifdef LDAP_API_FEATURE_X_OPENLDAP
 		rc = ldap_url_parse(val, urldata);
 		if (rc != LDAP_SUCCESS)
 		{
-- 
1.7.10.4


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] support for LDAP URLs

2012-12-03 Thread Peter Eisentraut
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.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] support for LDAP URLs

2012-11-26 Thread Alvaro Herrera
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
 +   termliteralldapurl/literal/term
 +   listitem
 +para
 + You can write most of the LDAP options alternatively using an RFC 
 2255
 + LDAP URL.  The format is
 +synopsis
 +ldap://[replaceableuser/replaceable[:replaceablepassword/replaceable]@]replaceablehost/replaceable[:replaceableport/replaceable]/replaceablebasedn/replaceable[?[replaceableattribute/replaceable][?[replaceablescope/replaceable]]]
 +/synopsis
 + replaceablescope/replaceable must be one
 + of literalbase/literal, literalone/literal, 
 literalsub/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 Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] support for LDAP URLs

2012-11-16 Thread Albe Laurenz
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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] support for LDAP URLs

2012-11-16 Thread Robert Haas
On Fri, Nov 16, 2012 at 4:36 AM, Albe Laurenz laurenz.a...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] support for LDAP URLs

2012-11-15 Thread Robert Haas
On Mon, Nov 12, 2012 at 10:38 PM, Peter Eisentraut pete...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] support for LDAP URLs

2012-11-15 Thread Peter Eisentraut
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.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] support for LDAP URLs

2012-11-12 Thread Peter Eisentraut
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.
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index d053fce..ba6523b 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -1486,6 +1486,39 @@ titleLDAP Authentication/title
 /para
/listitem
   /varlistentry
+  varlistentry
+   termliteralldapurl/literal/term
+   listitem
+para
+ You can write most of the LDAP options alternatively using an RFC 2255
+ LDAP URL.  The format is
+synopsis
+ldap://[replaceableuser/replaceable[:replaceablepassword/replaceable]@]replaceablehost/replaceable[:replaceableport/replaceable]/replaceablebasedn/replaceable[?[replaceableattribute/replaceable][?[replaceablescope/replaceable]]]
+/synopsis
+ replaceablescope/replaceable must be one
+ of literalbase/literal, literalone/literal, literalsub/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
+
+para
+ If you want encrypted LDAP connections, you have to use
+ the literalldaptls/literal option in addition
+ to literalldapurl/literal.  The literalldaps/literal URL
+ scheme (direct SSL connection) is not supported.
+/para
+
+para
+ Some other software that supports authentication against LDAP uses the
+ same URL format, so it will be easier to share the configuration.
+/para
+
+para
+ LDAP URLs are currently only supported with OpenLDAP, not on Windows.
+/para
+   /listitem
+  /varlistentry
 /variablelist
/para
 
@@ -1520,6 +1553,13 @@ titleLDAP Authentication/title
 If that second connection succeeds, the database access is granted.
/para
 
+   para
+Here is the same search+bind configuration written as a URL:
+programlisting
+host ... ldap lapurl=ldap://ldap.example.net/dc=example,dc=net?uid?sub;
+/programlisting
+   /para
+
tip
 para
  Since LDAP often uses commas and spaces to separate the different
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index ca470e1..cc1140d 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2209,7 +2209,7 @@ static int pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
 
 		r = ldap_search_s(ldap,
 		  port-hba-ldapbasedn,
-		  LDAP_SCOPE_SUBTREE,
+		  port-hba-ldapscope,
 		  filter,
 		  attributes,
 		  0,
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 7502e82..ba630a7 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -37,6 +37,16 @@
 #include utils/lsyscache.h
 #include utils/memutils.h
 
+#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
+
 
 #define atooid(x)  ((Oid) strtoul((x), NULL, 10))
 #define atoxid(x)  ((TransactionId) strtoul((x), NULL, 10))
@@ -1336,7 +1346,7 @@ static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 			{
 ereport(LOG,
 		(errcode(ERRCODE_CONFIG_FILE_ERROR),
-		 errmsg(cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, or ldapsearchattribute together with ldapprefix),
+		 errmsg(cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, or ldapurl together with ldapprefix),
 		 errcontext(line %d of configuration file \%s\,
 	line_num, HbaFileName)));
 return NULL;
@@ -1378,6 +1388,8 @@ static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 static bool
 parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
 {
+	hbaline-ldapscope = LDAP_SCOPE_SUBTREE;
+
 	if (strcmp(name, map) == 0)
 	{
 		if (hbaline-auth_method != uaIdent 
@@ -1437,6 +1449,54 @@ static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 		REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
 		hbaline-pamservice = pstrdup(val);
 	}
+	else if (strcmp(name, ldapurl) == 0)
+	{
+		LDAPURLDesc *urldata;
+		int rc;
+
+		REQUIRE_AUTH_OPTION(uaLDAP, ldapurl, ldap);
+
+#ifdef