Bug 809105

I'm not sure, but I think this comment I found in the openldap sourcode could be related:

openldap-2.4.23/servers/slapd/back-sql/search.c:
#-----------------------------------------------
static int
backsql_process_filter_eq( backsql_srch_info *bsi, backsql_at_map_rec *at,
                int casefold, struct berval *filter_value )
{
        /*
         * maybe we should check type of at->sel_expr here somehow,
         * to know whether upper_func is applicable, but for now
         * upper_func stuff is made for Oracle, where UPPER is
         * safely applicable to NUMBER etc.
         */
#-----------------------------------------------

Unfortunately I'm not knowledgeable enough to fix this myself.

Also related could be:

openldap-2.4.23/servers/slapd/back-sql/back-sql.h: line 343:

#-----------------------------------------------
/* define to uppercase filters only if the matching rule requires it
 * (currently broken) */
/* #define      BACKSQL_UPPERCASE_FILTER */

#define BACKSQL_AT_CANUPPERCASE(at)     ( !BER_BVISNULL( &(at)->bam_sel_expr_u ) )
#-----------------------------------------------

OK, I've had some extra time on my hands and was able to look into it a little bit deeper. From what I saw there are 3 "bugs" in openldap that play together here:

1) It is not checked whether the UPPER function can be applied on a type in the database. Comments in the code itself confirm that this is "currently broken". (see my previous 2 comments)

2) One would assume that the parameter "upper_needs_cast yes" in the slapd.conf would add a string casting statement to every UPPER statement in the SQL queries. In fact, it only does so for one single query (when it queries for "ldap_entries.dn"). All other queries where UPPER is applied still don't use a cast, even though I declare "upper_needs_cast yes" in the main config file.

3) The slapd.conf file lets me declare a "strcast_func" which is the name of the function for a cast to string.  This strcast_func is used at some points in the queries, but not consistently. For the one query that actually pays any attention to the "upper_needs_cast" parameter (which i mentioned before) a hardcoded string cast function is used instead. This function is always "(cast ? as varchar(255))". 

Now these are 3 separate problems in the back-sql code. To fix the problem reported in this bug report I took a closer look at problem number 2. I attached an upstream patch which can be applied to openldap-2.4.23-20.el6. When this patch is applied, the hardcoded strcast function from problem number 3 is at least applied to every use of the UPPER function (or at least everywhere where the upper function is applied properly!), effectively fixing problem number 2.

This may THEORETICALLY lead to the following problem:
IF someone set "upper_needs_cast yes" in their config, with a database backend that does not support the hardcoded strcast function, this patch would break the SQL queries for this user. BUT, in that case using the "upper_needs_cast yes" statement would have been useless - or even causing problems - before this patch anyways - since it was always using the hardcoded function and only at one single point.

I would ask you to please have a look at my patch, and after some good testing it would be great if this could be added to the official repositories. At the current state I cannot update SSSD with our setup. And anyone using a setup of postgresql+openldap+sssd will have the same problem.

It may not be a common setup, but it only uses 3 packages from the offical repositories in the current version and has always worked up to now.

Thanks a lot for your attention to this issue.

Created attachment 575690 [details]
An upstream patch fixing the problem. See comments for more information

Hello Rene,

I've tested your patch and it seems to be working. As you are saying, there are more problems in the back_sql code - especially using upper on incompatible types.

If I omit the 'upper_needs_cast yes' in slapd.conf, using upper on incompatible types just crashes with 'stack smashing detected' error. Question is if we can safely say, that the type we want to use the 'upper' on is incompatible and should be cast.

Back_sql has been declared experimental ever since its creation (see man slapd-sql, comments in code, docs distributed with the source, ...) and is probably a bad idea to use in a production environment.

However, I think this patch effectively fixes at least one sort of problems and should be included upstream as well. Can you propose it upstream? Thanks!

Dear Jan,

Thank you very much for your reply!

Unfortunately in our environment we have to use back-sql, since we have a custom made web-based user management which writes to a database backend. Openldap then connects to the same database and provides authentication and ID providing services. This user management interconnects with most of the webservices we developed. We have used (and polished) this setup for the last 10 years or so. We were always quite satisfied with it :-)

Back to topic: I'm certain there is a way to catch that error and decide when to use upper, but unfortunately I'm not skilled enough to implement this. If someone could do that I would greatly appreciate it.

> Can you propose it upstream?

I'm sorry, I'm not too familiar with the bugreport/patching process. What does it mean to "propose it upstream"?

I can see that there already is an ITS filed for this bug:
http://www.openldap.org/its/index.cgi/Incoming?id=7130

You can post a follow-up with your patch and an explanation.

For more information about the patch posting process, you can refer to http://www.openldap.org/devel/contributing.html .

Created attachment 577697 [details]
Reproducer pack

A reproducer pack.

1) Install packages mentioned in README.
2) As root, run restart.sh
3) As root, run runtest.sh

For more debugging information, run slapd with -d 511 (instead of 256).

Without the patch applied, running the test should cause incorrect behavior (segfaults in my case), because of incorrect application of 'upper' function on a number.

Hello Rene,

if you agree, I can propose this patch upstream for you, mentioning you as the original author.

Jan, this would be really great. I struggled a bit with trying to grasp what is necessary to submit the patch. Thank you so much for your help!

@Jan @Rene: Beware, OpenLDAP upstream does not accept third party contributions. They will reject the patch. It happened to me.

Submitting the patch is easy, you do not need a registration. Create a new report on http://www.openldap.org/its/, upload patch to their FTP server and add a note to the report, that you release the patch under public domain license according to http://www.openldap.org/devel/contributing.html. That's all. (The patch should apply cleanly to their git master branch.)

Ok, I gave it a try:
http://www.openldap.org/its/index.cgi?findid=7248

> Ok, I gave it a try:
Great, thank you!

No response from openldap so far. Is there any chance it could be integrated as a patch into the RHEL package if the openldap community doesn't?

Rene, we would like to have your changes reviewed by upstream before including into RHEL. We do not want to cause any regressions.

It has been over two months and still nothing from the openldap team. What is RedHat's take on situations like these, where the project is no longer maintained by the developer team (like back-sql)?

Rene, it depends on situation. The problem is, that back-sql is very fragile (therefore we provide it as a separate package in Supplementary channel). Your patch looks fine but I'm not sure if it will keep other currently working configurations running. This is the reason why we want to have this reviewed by upstream.


This bug tracking system is not a mechanism for receiving official Red Hat support, and  cannot make any guarantees as to the timeliness or suitability of a resolution. 

If this issue is critical or in any way time sensitive, please raise a ticket through your regular Red Hat support channels to make certain  it receives the proper attention and prioritization to assure a timely resolution. 

For information on how to contact the Red Hat production support team, please visit: https://www.redhat.com/support/process/production/#howto

This has just popped up:

http://www.openldap.org/its/index.cgi?findid=7447

Might be of interest.

This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated
in the current release, Red Hat is unable to address this
request at this time.

Red Hat invites you to ask your support representative to
propose this request, if appropriate, in the next release of
Red Hat Enterprise Linux.

The patch hasn't been touched by upstream and is very unlikely to be. I'm closing this bugzilla.