A couple of comments on this patch. I have attached a "fixup" patch on
top of your v4 that should address them.
- I think the bracketing of the LDAP URL synopsis is wrong.
- I have dropped the sentence that LDAP URL extensions are not
supported. That sentence was written mainly to point out that filters
are not supported, which they are now. There is nothing beyond filters
and extensions left in LDAP URLs, AFAIK.
- We have previously used the ldapsearchattribute a bit strangely. We
use it as both the search filter and as the attribute to return from the
search, but we don't actually care about the returned attribute (we only
use the DN (which is not a real attribute)). That hasn't been a real
problem, but now if you specify a filter, as the code stands it will
always request the "uid" attribute, which won't work if there is no such
attribute. I have found that there is an official way to request "no
attribute", so I have fixed the code to do that.
- I was wondering whether we need a way to escape the % (%%?) but it
doesn't seem worth bothering.
I have been looking around the Internet how this functionality compares
to other LDAP authentication systems.
I didn't see the origin of the %u idea in this thread, but perhaps it
came from Dovecot. But there it's a general configuration file syntax,
nothing specific to LDAP. In nginx you use %(username), which again is
general configuration file syntax. I'm OK with using %u.
The original LDAP URL design was adapted from Apache
(https://httpd.apache.org/docs/2.4/mod/mod_authnz_ldap.html#authldapurl).
They combine the attribute filter and the general filter if both are
specified, but I think they got that a bit wrong. The attribute field
in the URL is actually not supposed to be a filter but a return field,
which is also the confusion mentioned above.
The PAM module (https://linux.die.net/man/5/pam_ldap) also has a way to
specify a search attribute and a general filter and combines them.
Neither of these allows substituting the user name into the search filter.
I think there would be a case to be made to allow the searchattribute
and the searchfilter both be specified. One typical use would be to use
the first one to locate the user and the second one to "filter" out
certain things, which I think is the approach the PAM and Apache modules
take. This wouldn't work well, however, if you use the %u placeholder,
because then you'd have to explicitly unset ldapsearchattribute, which
would be annoying. So maybe not.
Please check my patch. I think it's ready to go then.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From cd5536fa70eed74f8b1aa4f856edae8b84d76ef5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 8 Sep 2017 10:58:53 -0400
Subject: [PATCH] fixup! Allow custom search filters to be configured for LDAP
auth.
---
doc/src/sgml/client-auth.sgml | 4 +---
src/backend/libpq/auth.c | 18 ++++++++----------
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 1773ce29a9..1c3db96134 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -1525,7 +1525,7 @@ <title>LDAP Authentication</title>
An RFC 4516 LDAP URL. This is an alternative way to write some of the
other LDAP options in a more compact and standard form. The format is
<synopsis>
-ldap://<replaceable>host</replaceable>[:<replaceable>port</replaceable>]/<replaceable>basedn</replaceable>[[[?[<replaceable>attribute</replaceable>]][?<replaceable>scope</replaceable>]][?<replaceable>filter</replaceable>]]
+ldap://<replaceable>host</replaceable>[:<replaceable>port</replaceable>]/<replaceable>basedn</replaceable>[?[<replaceable>attribute</replaceable>][?[<replaceable>scope</replaceable>][?[<replaceable>filter</replaceable>]]]]
</synopsis>
<replaceable>scope</replaceable> must be one
of <literal>base</literal>, <literal>one</literal>,
<literal>sub</literal>,
@@ -1537,8 +1537,6 @@ <title>LDAP Authentication</title>
<literal>ldapsearchfilter</literal>. When specifying a search filter
as part of a URL, the special token <literal>%u</literal> standing
for the user name must be written as <literal>%25u</literal>.
- Some other components of standard LDAP URLs such as extensions are
- not supported.
</para>
<para>
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 6c96e87d37..c43f203eb3 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2401,8 +2401,8 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
static char *
FormatSearchFilter(const char *pattern, const char *user_name)
{
- Size user_name_size = strlen(user_name);
- Size buffer_size = 0;
+ size_t user_name_size = strlen(user_name);
+ size_t buffer_size = 0;
const char *in;
char *out;
char *result;
@@ -2413,7 +2413,7 @@ FormatSearchFilter(const char *pattern, const char
*user_name)
{
if (in[0] == '%' && in[1] == 'u')
{
- buffer_size += user_name_size;
+ buffer_size += user_name_size - 2;
in += 2;
}
else
@@ -2486,7 +2486,7 @@ CheckLDAPAuth(Port *port)
char *filter;
LDAPMessage *search_message;
LDAPMessage *entry;
- char *attributes[2];
+ char *attributes[] = { LDAP_NO_ATTRS, NULL };
char *dn;
char *c;
int count;
@@ -2528,15 +2528,13 @@ CheckLDAPAuth(Port *port)
return STATUS_ERROR;
}
- /* Fetch just one attribute, else *all* attributes are returned
*/
- attributes[0] = port->hba->ldapsearchattribute ?
port->hba->ldapsearchattribute : "uid";
- attributes[1] = NULL;
-
/* Build a custom filter or a single attribute filter? */
- if (port->hba->ldapsearchfilter != NULL)
+ if (port->hba->ldapsearchfilter)
filter =
FormatSearchFilter(port->hba->ldapsearchfilter, port->user_name);
+ else if (port->hba->ldapsearchattribute)
+ filter = psprintf("(%s=%s)",
port->hba->ldapsearchattribute, port->user_name);
else
- filter = psprintf("(%s=%s)", attributes[0],
port->user_name);
+ filter = psprintf("(uid=%s)", port->user_name);
r = ldap_search_s(ldap,
port->hba->ldapbasedn,
--
2.11.0 (Apple Git-81)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
