Horton

Thread

Commits

  1. Remove unnecessary unbind in LDAP search+bind mode

  1. [PATCH] Remove unnecessary unbind in LDAP search+bind mode

    Anatoly Zaretsky <anatoly.zaretsky@gmail.com> — 2023-03-23T01:45:17Z

    Hi!

Comments in src/backend/libpq/auth.c [1] say:
(after successfully finding the final DN to check the user-supplied
password against)
/* Unbind and disconnect from the LDAP server */
and later
/*
* Need to re-initialize the LDAP connection, so that we can bind to
* it with a different username.
*/

But the protocol actually permits multiple subsequent authentications
("binds" in LDAP parlance) over a single connection [2].
Moreover, inspection of the code revision history of mod_authnz_ldap,
pam_ldap, Bugzilla, and MediaWiki LDAP authentication plugin, shows that
they've been doing this bind-after-search over the same LDAP connection for
~20 years without any evidence of interoperability troubles.

(mod_authnz_ldap and pam_ldap are listed in the PostgreSQL documentation as
examples of other software implementing this scheme. Bugzilla and MediaWiki
are the original patch author's motivating examples [3])

Also it might be interesting to consider this note from the current
revision of the protocol RFC [4]:
"The Unbind operation is not the antithesis of the Bind operation as the
name implies. The naming of these operations are historical. The Unbind
operation should be thought of as the "quit" operation."

So, it seems like the whole connection re-initialization thing was just a
confusion caused by this very unfortunate "historical" naming, and can be
safely removed, thus saving quite a few network round-trips, especially for
the case of ldaps/starttls.

[1]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/libpq/auth.c;h=bc0cf26b122a1b28c20fe037ec851c0e99b1ffb6;hb=HEAD#l2603
[2] https://www.rfc-editor.org/rfc/rfc4511#section-4.2.1
[3]
https://www.postgresql.org/message-id/4c0112730909141334n201cadf3x2e288528a97883ca%40mail.gmail.com
[4] https://www.rfc-editor.org/rfc/rfc4511#section-4.3
-- 
Best regards,
Anatoly Zaretsky

  2. Re: [PATCH] Remove unnecessary unbind in LDAP search+bind mode

    Peter Eisentraut <peter@eisentraut.org> — 2023-07-03T09:53:03Z

    On 23.03.23 02:45, Anatoly Zaretsky wrote:
> Comments in src/backend/libpq/auth.c [1] say:
> (after successfully finding the final DN to check the user-supplied 
> password against)
> /* Unbind and disconnect from the LDAP server */
> and later
> /*
> * Need to re-initialize the LDAP connection, so that we can bind to
> * it with a different username.
> */
> 
> But the protocol actually permits multiple subsequent authentications 
> ("binds" in LDAP parlance) over a single connection [2].
> Moreover, inspection of the code revision history of mod_authnz_ldap, 
> pam_ldap, Bugzilla, and MediaWiki LDAP authentication plugin, shows that 
> they've been doing this bind-after-search over the same LDAP connection 
> for ~20 years without any evidence of interoperability troubles.

> So, it seems like the whole connection re-initialization thing was just 
> a confusion caused by this very unfortunate "historical" naming, and can 
> be safely removed, thus saving quite a few network round-trips, 
> especially for the case of ldaps/starttls.

Your reasoning and your patch look correct to me.

  3. Re: [PATCH] Remove unnecessary unbind in LDAP search+bind mode

    Peter Eisentraut <peter@eisentraut.org> — 2023-07-09T06:57:37Z

    On 03.07.23 11:53, Peter Eisentraut wrote:
> On 23.03.23 02:45, Anatoly Zaretsky wrote:
>> Comments in src/backend/libpq/auth.c [1] say:
>> (after successfully finding the final DN to check the user-supplied 
>> password against)
>> /* Unbind and disconnect from the LDAP server */
>> and later
>> /*
>> * Need to re-initialize the LDAP connection, so that we can bind to
>> * it with a different username.
>> */
>>
>> But the protocol actually permits multiple subsequent authentications 
>> ("binds" in LDAP parlance) over a single connection [2].
>> Moreover, inspection of the code revision history of mod_authnz_ldap, 
>> pam_ldap, Bugzilla, and MediaWiki LDAP authentication plugin, shows 
>> that they've been doing this bind-after-search over the same LDAP 
>> connection for ~20 years without any evidence of interoperability 
>> troubles.
> 
>> So, it seems like the whole connection re-initialization thing was 
>> just a confusion caused by this very unfortunate "historical" 
>> naming, and can be safely removed, thus saving quite a few 
>> network round-trips, especially for the case of ldaps/starttls.
> 
> Your reasoning and your patch look correct to me.

committed

  4. Re: [PATCH] Remove unnecessary unbind in LDAP search+bind mode

    Anatoly Zaretsky <anatoly.zaretsky@gmail.com> — 2023-07-09T18:11:53Z

    On Sun, Jul 9, 2023 at 9:57 AM Peter Eisentraut <peter@eisentraut.org>
wrote:

> committed
>
Thanks!

-- 
Best regards,
Anatoly Zaretsky