On Thu, Sep 17, 2009 at 11:15 AM, Magnus Hagander <mag...@hagander.net>wrote:
> On Thu, Sep 17, 2009 at 18:02, Robert Fleming <flemi...@gmail.com> wrote:
> > Following a discussion on the pgsql-admin list
> > <http://archives.postgresql.org/pgsql-admin/2009-09/msg00075.php>, I
> have
> > created a patch to (optionally) allow PostgreSQL to do a LDAP search to
> > determine the user's DN (as is done in Apache, MediaWiki, Bugzilla, et
> al.)
> > instead of building the DN from a prefix and suffix.
> > This is necessary for schemas where the login attribute is not in the DN,
> > such as is described here
> > <http://www.ldapman.org/articles/intro_to_ldap.html#individual> (look
> for
> > "name-based"). This patch is against PostgreSQL 8.4.0 from Debian
> > Lenny-backports. If this would be a welcome addition, I can port it
> forward
> > to the latest from postgresql.org.
> > Thanks in advance for your feedback.
>
> This sounds like a very useful feature, and one that I can then remove
> from my personal TODO list without having to do much work :-)
>
> A couple of comments:
>
> First of all, please read up on the PostgreSQL coding style, or at
> least look at the code around yours. This doesn't look anything like
> our standards.
>
Sorry, the 8.1 manual said all contributions go through pgindent so I didn't
spend too much time on that. I see that the 8.4 manual clarifies that
pgindent won't get run till release time. In any case, I've re-formatted
the patch using the Emacs settings from the 8.1 manual (why are they gone
from the 8.4 manual)? It seems like most of the rest of the Coding
Conventions have to do with error reporting. Do please let me know if
there's something else I can do.
Second, this appears to require an anonymous bind to the directory,
> which is something we should not encourage people to enable on their
> LDAP servers. I think we need to also take parameters with a DN and a
> password to bind with in order to do the search, and then re-bind as
> the user when found.
The new patch implements the initial bind using new configuration parameters
"ldapbinddn" and "ldapbindpasswd". I've also added a "ldapsearchattribute"
just to be complete.
Robert
diff -ru postgresql-8.4-8.4.0-original/src/backend/libpq/auth.c postgresql-8.4-8.4.0/src/backend/libpq/auth.c
--- postgresql-8.4-8.4.0-original/src/backend/libpq/auth.c 2009-06-25 04:30:08.000000000 -0700
+++ postgresql-8.4-8.4.0/src/backend/libpq/auth.c 2009-09-17 18:12:57.000000000 -0700
@@ -2150,10 +2150,110 @@
}
}
- snprintf(fulluser, sizeof(fulluser), "%s%s%s",
- port->hba->ldapprefix ? port->hba->ldapprefix : "",
- port->user_name,
- port->hba->ldapsuffix ? port->hba->ldapsuffix : "");
+ if (port->hba->ldapbasedn)
+ {
+ char filter[NAMEDATALEN + 10]; // FIXME: maybe there's a preferred way to pick this size?
+ LDAPMessage* search_message;
+ char* attributes[2];
+ LDAPMessage* entry;
+ char* dn;
+
+ /* bind for searching */
+ r = ldap_simple_bind_s(ldap,
+ port->hba->ldapbinddn ? port->hba->ldapbinddn : "",
+ port->hba->ldapbindpasswd ? port->hba->ldapbindpasswd : "");
+ if (r != LDAP_SUCCESS)
+ {
+ ereport(LOG,
+ (errmsg("LDAP initial bind failed for ldapbinddn \"%s\" on server \"%s\": error code %d",
+ port->hba->ldapbinddn, port->hba->ldapserver, r)));
+ return STATUS_ERROR;
+ }
+
+ /* fetch just one attribute, else /all/ attributes are returned */
+ attributes[0] = "uid";
+ attributes[1] = NULL;
+
+ snprintf(filter, sizeof(filter), "(%s=%s)",
+ port->hba->ldapsearchattribute ? port->hba->ldapsearchattribute : "uid",
+ port->user_name); /* FIXME: sanitize user_name? */
+ filter[sizeof(filter) - 1] = '\0';
+
+ r = ldap_search_s(ldap,
+ port->hba->ldapbasedn,
+ LDAP_SCOPE_SUBTREE,
+ filter,
+ attributes,
+ 0,
+ &search_message);
+
+ if (r != LDAP_SUCCESS)
+ {
+ ereport(LOG,
+ (errmsg("LDAP search failed for filter \"%s\" on server \"%s\": error code %d",
+ filter, port->hba->ldapserver, r)));
+ return STATUS_ERROR;
+ }
+
+ if (ldap_count_entries(ldap, search_message) != 1)
+ {
+ if (ldap_count_entries(ldap, search_message) == 0)
+ ereport(LOG,
+ (errmsg("LDAP search failed for filter \"%s\" on server \"%s\": no such user",
+ filter, port->hba->ldapserver)));
+ else
+ ereport(LOG,
+ (errmsg("LDAP search failed for filter \"%s\" on server \"%s\": user is not unique (%d matches)",
+ filter, port->hba->ldapserver, ldap_count_entries(ldap, search_message))));
+
+ ldap_msgfree(search_message);
+ return STATUS_ERROR;
+ }
+
+ entry = ldap_first_entry(ldap, search_message);
+ dn = ldap_get_dn(ldap, entry);
+ if (dn == NULL)
+ {
+ int error;
+ (void)ldap_get_option(ldap, LDAP_OPT_RESULT_CODE, &error);
+ ereport(LOG,
+ (errmsg("LDAP get_dn() for the first entry matching \"%s\" on server \"%s\": %s",
+ filter, port->hba->ldapserver, ldap_err2string(error))));
+ ldap_msgfree(search_message);
+ return STATUS_ERROR;
+ }
+ strncpy(fulluser, dn, sizeof(fulluser));
+
+ ldap_memfree(dn);
+ ldap_msgfree(search_message);
+
+ /* unbind */
+ r = ldap_unbind_s(ldap);
+ if (r != LDAP_SUCCESS)
+ {
+ int error;
+ (void)ldap_get_option(ldap, LDAP_OPT_RESULT_CODE, &error);
+ ereport(LOG,
+ (errmsg("LDAP unbind failed for user \"%s\" on server \"%s\": %s",
+ fulluser, port->hba->ldapserver, ldap_err2string(error))));
+ return STATUS_ERROR;
+ }
+
+ /* re-init */
+ ldap = ldap_init(port->hba->ldapserver, port->hba->ldapport);
+ if (!ldap)
+ {
+ printf("could not initialize LDAP: error code %d",
+ errno);
+ return STATUS_ERROR;
+ }
+ }
+ else
+ snprintf(fulluser, sizeof(fulluser), "%s%s%s",
+ port->hba->ldapprefix ? port->hba->ldapprefix : "",
+ port->user_name,
+ port->hba->ldapsuffix ? port->hba->ldapsuffix : "");
+
fulluser[sizeof(fulluser) - 1] = '\0';
r = ldap_simple_bind_s(ldap, fulluser, passwd);
diff -ru postgresql-8.4-8.4.0-original/src/backend/libpq/hba.c postgresql-8.4-8.4.0/src/backend/libpq/hba.c
--- postgresql-8.4-8.4.0-original/src/backend/libpq/hba.c 2009-06-24 06:39:42.000000000 -0700
+++ postgresql-8.4-8.4.0/src/backend/libpq/hba.c 2009-09-17 17:46:21.000000000 -0700
@@ -1032,6 +1032,26 @@
return false;
}
}
+ else if (strcmp(token, "ldapbinddn") == 0)
+ {
+ REQUIRE_AUTH_OPTION(uaLDAP, "ldapbinddn", "ldap");
+ parsedline->ldapbinddn = pstrdup(c);
+ }
+ else if (strcmp(token, "ldapbindpasswd") == 0)
+ {
+ REQUIRE_AUTH_OPTION(uaLDAP, "ldapbindpasswd", "ldap");
+ parsedline->ldapbindpasswd = pstrdup(c);
+ }
+ else if (strcmp(token, "ldapsearchattribute") == 0)
+ {
+ REQUIRE_AUTH_OPTION(uaLDAP, "ldapsearchattribute", "ldap");
+ parsedline->ldapsearchattribute = pstrdup(c);
+ }
+ else if (strcmp(token, "ldapbasedn") == 0)
+ {
+ REQUIRE_AUTH_OPTION(uaLDAP, "ldapbasedn", "ldap");
+ parsedline->ldapbasedn = pstrdup(c);
+ }
else if (strcmp(token, "ldapprefix") == 0)
{
REQUIRE_AUTH_OPTION(uaLDAP, "ldapprefix", "ldap");
diff -ru postgresql-8.4-8.4.0-original/src/include/libpq/hba.h postgresql-8.4-8.4.0/src/include/libpq/hba.h
--- postgresql-8.4-8.4.0-original/src/include/libpq/hba.h 2009-06-11 07:49:11.000000000 -0700
+++ postgresql-8.4-8.4.0/src/include/libpq/hba.h 2009-09-17 17:46:00.000000000 -0700
@@ -53,6 +53,10 @@
bool ldaptls;
char *ldapserver;
int ldapport;
+ char *ldapbinddn;
+ char *ldapbindpasswd;
+ char *ldapsearchattribute;
+ char *ldapbasedn;
char *ldapprefix;
char *ldapsuffix;
bool clientcert;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
