-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ldap no longer respects TLS_CACERT from ldaprc in ldap_start_tls() #18529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
|
BTW, I cannot reproduce, using PHP 8.3.21 or 8.4.7 (on Fedora 41 with openldap 2.6.9 or RHEL 9.5 with openldap 2.6.6 / openssl 3.2.2 from official RH channels, PHP from my repo) Using a With TLS_REQCERT=never and TLS_REQSAN=never
With TLS_REQCERT=demand and TLS_REQSAN=demand
|
Sure, but the whole point of this ticket is, that verifying a certificate chain during start_tls fails although it is valid and the needed CA certificate is provided. When you disable the verification the test is no longer valid for this. |
I'm wondering if it could be because ldaprc creates a global context which is now getting reset before the first use and it gets lost. If it's the case, it might be a bit tricky to get fixed. We would probably need to try to parse ldaprc ourselves (unless there is some API way to do that) or we could get the |
I just had a bit deeper look to the docs and code and it looks like it's really getting reset. This is all actually quite well visible in https://github.com/openldap/openldap/blob/e460f00874352c3d2cdbca8a00a4112c7bc633ab/libraries/libldap/init.c . The core initialization function is The problem is that So unless I missed something and my analysis of the code is incorrect, I think it might be best to to revert 389de7c and just add |
The goal of this test is to ensure values from "ldaprc" are properly used. |
I misunderstood your comment and thought you wanted to say "no problem", but now I got it. |
It would be good to document under which circumstances values from ldaprc are picked up respectively get reset to default values. |
I did a bit more checking and the global options are really preserved. In this case it's kept in Previous behaviour was that as soon as the context (SSL *) created, it was cached and couldn't be changed. This was also kept for all requests that use the same worker. It means for example when FPM (or apache prefork) child started and got the first request that initializes / starts tls session, it created a TLS context that was cached for all subsequent requests and it was not possible to change those values using So there are few options what can be happening:
@rainerjung it would be great if you could do a bit more checking how this gets resets. If you can run it under gdb, it might be useful to add a break point to |
Thanks for the details to look for. gdb show four calls to ldap_pvt_tls_set_option(). The first three come from the three functional lines in ldaprc and set TLS_REQCERT, TLS_REQSAN and TLS_CACERT correctly as given in ldaprc. The stacks for these are mostly the same, namely
Then there's exactly one additional call to ldap_pvt_tls_set_option(). It comes from zif_ldap_start_tls():
So the first three calls operate on the global LDAP config and the third one on the config with address 0xd63190. This brings me to the following code in ldap.c, that was added by the commit in question:
I have not yet understood that construct, but it comments "ensure all pending TLS options are applied in a new context" and runs only when an ldaps URL is used. In our case, we have an ldap url but use start_tls afterwards. So I think the code does not "apply all pending TLS options to the new context" and thus the CACERT option gets lost. I can not judge, whether the code could and should also run for a plain ldap URL so that a followup start_tls call would succeed. |
Sorry, I see, that start_tls seems to do something similar:
I checked, that LDAP_OPT_X_TLS_NEWCTX is defined in my include/ldap.h:
(from openldap 2.6.9). |
We're hit by this issue as well, where the Only by explicitly calling For me, this setting should be kept for both context types. |
I have investigated the OpenLDAP source code. My understanding is as follows:
a) direct items in ld->ld_options aka ld->ldc->ldc_options Examples are the flags
which determine if and how certificates get checked. b) indirect items in ld->ld_options->ldo_tls_info aka ld->ldc->ldc_options->ldo_tls_info Examples are:
Some macro abbreviations exist like eg.
c) the SSL context in ld->ld_options->ldo_tls_ctx aka ld->ldc->ldc_options->ldo_tls_ctx
Implementation specific, eg. for OpenSSL etc. How do the three parts behave with respect to inheriting settings from the global context to an LDAP instance? First: once the SSL context c) is initialized, any change to settings in a) or b) will no longer influence that context. It gets created from a) and b) but from then on it is independent of them, except when it gets freshly created. We'll come back to this in a minute. That means setting TLS config items after the SSL context got initialized does no longer change it, except if being recreated. The first SSL context seems always to get initialized from the global settings. When a non-global LDAP instance is created in ldap_create( LDAP **ldp ), eg. via ldap_initialize(LDAP **ldp, LDAP_CONST char *url), the config items in a) get copied from the global settings via memcpy. The settings in b) get reset (!) and the ssl ctx is NULL: int The SSL context gets initialized either during ldap_int_tls_connect(...). If it is not yet initialized it gets always initialized from the global settings. Or it gets initialized or recreated via ldap_set_option for option LDAP_OPT_X_TLS_NEWCTX. In that case it gets initialized from a) and b) of the LDAP instance. Remember the LDAP instance always starts with b) being zeroed. So how does that relate to the observed behavior in PHP land:
TLS_REQCERT demand in ldaprc goes into part a) in the global options and always gets inherited to any LDAP instance unless explicitly overwritten with ldap_set_option. Setting TLS_CACERT in ldaprc goes into b). It ends up in the first SSL context, but whenever a new SSL context is created from and LDAP instance, because b) gets zeroed when creating the instance, it gets lost in the SSL context. The only way to create a new SSL context is ldap_set_option with LDAP_OPT_X_TLS_NEWCTX. When one wants to overwrite global settings via ldap_set_option on an LDAP instance before the SSL context is created, one has to make sure, that the new SSL context is not simply the global one. IMHO therefore one needs to use LDAP_OPT_X_TLS_NEWCTX. But then all global b) configs are lost unless recreated with explicit calls to ldap_set_option. I think the memset of ldo_tls_info to 0 in ldap_create() is not a good idea. Of course one would have to do a deep copy instead of just a memcpy in order to get an independent copy of strings in the created LDAP instead of a copied pointer. That way the new LDAP would inherit the configuration and be adjustable without also changing the global config. As a result I would say there is no good solution. But at least one should document, that using start_tls destroys the global config for
(the names in the global config ldaprc or env vbars differ, but you get the idea). And one should decide, whether the change in behavior is OK for a patch release like 8.4.7 or not. I myself am undecided and fine with it each way. |
Note that this is also an issue with PHP 8.3.21 (using the Sury Debian packages). For me the only choice would be to keep the global settings when using |
@rainerjung thanks for the detailed diag. @bobvandevijver no, I really thinks this should be reported upstream (openldap), string options should be inherited like other. A possible workaround is proposed in PR #18547 (draft for now) Other eyes / tests / feedback welcome |
Description
The following code:
using an ldaprc file like
Resulted in this output (using 8.4.7):
But I expected this output instead (using 8.4.6):
The connection works with 8.4.7, if I add
but it fails for 8.4.6, if I use to define the bundle that way and remove it from ldaprc.
So 8.4.6 need to have it in ldaprc, 8.4.7 needs to have it set via LDAP_OPT_X_TLS_CACERTFILE.
The problem shows up when using start_tls. When I connect directly to an ldaps URL, the bundle from ldaprc is used in 8.4.6 and 8.4.7 and the connection succeeds.
I guess this is related to #17776 and commit 389de7c .
PHP Version
Operating System
RHEL 9
The text was updated successfully, but these errors were encountered: