PSMDB-1998 Add timeouts to LDAP connect/bind operations#1756
Merged
Conversation
Apply ldapTimeoutMS to LDAP connection establishment and bind operations, not just search queries. Without these timeouts, auth requests hang indefinitely when the LDAP server is unreachable, causing connection pile-up and eventual file descriptor exhaustion. Changes: - Set LDAP_OPT_NETWORK_TIMEOUT and LDAP_OPT_TIMEOUT on all LDAP handles (both pool and per-auth connections) using ldapTimeoutMS - Check LDAPbind() return value in borrow_search_connection() to avoid returning broken connections - Add timeout to connection pool condvar wait to prevent threads from blocking indefinitely when all pool slots are occupied - Fix LDAP handle leak in create_connection() error paths using ScopeGuard
Backport Policy Overview@igorsol, This branch ( Next step: Please re-open this PR against Please reach out in #server-release if you have any questions. |
Add ldapauthz_ldap_timeouts.js with two test scenarios: - Auth against a non-listening LDAP port fails gracefully and the server recovers when ldapServers is switched to a valid server - Auth fails with zero ldapTimeoutMS, proving LDAP_OPT_TIMEOUT is applied to bind operations Convert _setup.js to an ES module exporting createAdminUser(conn) and remove dead load() calls from existing tests that never used the admin user created by the old setup.
Replace the _ld class member with a local variable and ScopeGuard to ensure the LDAP connection is released immediately after the single-step auth completes, rather than lingering until the mechanism object is destroyed.
Move LDAPbind() from borrow_search_connection() into borrow_or_create() so pool connections are bound once when created rather than on every borrow. This eliminates a round-trip to the LDAP server on each auth or role query. Add a destroy parameter to return_ldap_connection() so callers can signal that a connection should be removed from the pool rather than recycled. Use this in execQuery() retry and error paths to discard connections after search failures.
Add a destroy flag to LDAPConnInfo so connections can be marked for destruction when returned to the pool. Add invalidate_connections() to ConnectionPoller which destroys idle connections immediately and marks borrowed ones for destruction on return. Expose invalidateConnections() through the LDAPManager interface. Convert ldapQueryUser and ldapQueryPassword IDL parameters to use custom cpp_class implementations that call invalidateConnections() when values change at runtime. Also invalidate connections when ldapServers changes, since existing connections point to the old server.
Extend ldapauthz_ldap_timeouts.js Test 1 to verify that switching ldapServers back to a non-listening port invalidates the pool and causes auth to fail again. Add ldapauthz_pool_invalidation.js to test that changing ldapQueryPassword at runtime invalidates pooled connections: auth fails with wrong credentials and succeeds again after restoring the correct password.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Apply ldapTimeoutMS to LDAP connection establishment and bind operations, not just search queries. Without these timeouts, auth requests hang indefinitely when the LDAP server is unreachable, causing connection pile-up and eventual file descriptor exhaustion.
Changes:
Add ldapauthz_ldap_timeouts.js test with two test scenarios:
Add ldapauthz_pool_invalidation.js to test that changing
ldapQueryPassword at runtime invalidates pooled connections: auth
fails with wrong credentials and succeeds again after restoring
the correct password.
Convert _setup.js to an ES module exporting createAdminUser(conn) and remove dead load() calls from existing tests that never used the admin user created by the old setup.