Fix LDAP TLS client certificate options must be set before ldap_connect() for Google Secure LDAP#945
Open
mikelog wants to merge 1 commit into
Open
Conversation
…r Google Secure LDAP
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
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.
Base information
Symptom (bug)
When using Google Secure LDAP (
ldaps://ldap.google.com:636) with clientcertificates, the first authentication attempt on each new PHP-FPM worker
always fails with:
Subsequent attempts from the same worker succeed. Users need 2-5 login attempts
before being granted access.
Reproduction procedure (bug)
ldaps://ldap.google.com:636)LDAP_OPT_X_TLS_CACERTFILE,LDAP_OPT_X_TLS_CERTFILE,LDAP_OPT_X_TLS_KEYFILEin theoptionsarray inconfig-itop.phppm = ondemandorpm = dynamicCause (bug)
In
CheckCredentials(), TLS options (LDAP_OPT_X_TLS_CACERTFILE,LDAP_OPT_X_TLS_CERTFILE,LDAP_OPT_X_TLS_KEYFILE) are set vialdap_set_option($hDS, ...)afterldap_connect().However, OpenLDAP requires these options to be set globally via
ldap_set_option(null, ...)beforeldap_connect()so that the TLScontext includes the client certificate during the SSL handshake.
Current broken order:
ldap_connect()← TLS handshake starts here, no client cert loaded yetforeach $aOptionswithldap_set_option($hDS, ...)← too late for TLSldap_bind()← fails on first worker attemptProposed solution (bug)
Set TLS options globally via
ldap_set_option(null, ...)beforeldap_connect(),then reinitialize the TLS context with
LDAP_OPT_X_TLS_NEWCTX:Checklist before requesting a review
~20-50% before (first attempt always failed on fresh workers)
digging in the code?
Environment tested
ldap.google.com:636)pm = ondemand)Additional notes
Please consider backporting to
support/3.2as this affects the current stable release (3.2.3).Confirmed and tested on iTop 2.7.3 with the same code path.