Re: DNS SRV support for LDAP authentication

Поиск
Список
Период
Сортировка
От Daniel Gustafsson
Тема Re: DNS SRV support for LDAP authentication
Дата
Msg-id 07897B8D-2667-490D-86C6-D46CB215224B@yesql.se
обсуждение исходный текст
Ответ на DNS SRV support for LDAP authentication  (Thomas Munro <thomas.munro@enterprisedb.com>)
Ответы Re: DNS SRV support for LDAP authentication  (Thomas Munro <thomas.munro@enterprisedb.com>)
Список pgsql-hackers
> On 25 Sep 2018, at 04:09, Thomas Munro <thomas.munro@enterprisedb.com> wrote:

> Some people like to use DNS SRV records to advertise LDAP servers on
> their network.  Microsoft Active Directory is usually (always?) set up
> that way.  Here is a patch to allow our LDAP auth module to support
> that kind of discovery.  It copies the convention of the OpenLDAP
> command line tools: if you give it a URL that has no hostname, it'll
> try to extract a domain name from the bind DN, and then ask your DNS
> server for a SRV record for LDAP-over-TCP at that domain.  The
> OpenLDAP version of libldap.so exports the magic to do that, so the
> patch is very small (but the infrastructure set-up to test it is a bit
> of a schlep, see below).  I'll add this to the next Commitfest.

Sounds like a reasonable feature.

> Testing instructions for (paths and commands given for FreeBSD, adjust
> as appropriate):

Trying this quickly on macOS while at a conference didn’t yield much success,
will do another attempt when I’m on a more reliable connection.

> This is a first draft.  Not tested much yet.  I wonder if
> HAVE_LDAP_INITIALIZE is a reasonable way to detact OpenLDAP.  The
> documentation was written in about 7 seconds so probably needs work.
> There is probably a Windowsy way to do this too but I didn't look into
> that.

Reading through the patch, and related OpenLDAP code, this seems like a good
approach.  A few small comments:

+     If <productname>PostgreSQL</productname> was compiled with OpenLDAP as

Should OpenLDAP be wrapped in <productname> tags as well?  If so, there is
another “bare” instance in client-auth.sgml which perhaps can be wrapped into
this patch while at it.

+               ereport(LOG,
+                       (errmsg("could not look up a hostlist for %s",
+                               domain)));

Should this be \”%s\”?

+               new_uris = psprintf("%s%s%s://%s:%d",

While this construction isn't introduced in this patch, would it not make sense
to convert uris to StringInfo instead to improve readability?

+               /* Step over this hostname and any spaces. */

Nitpicking on a moved hunk, but single-line comments shouldn’t end in a period
I believe.

cheers ./daniel



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alvaro Herrera
Дата:
Сообщение: Re: [HACKERS] [PATCH] Generic type subscripting
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: [HACKERS] [PATCH] Generic type subscripting