Bug 809105 - Openldap 2.4.23 back_sql incompatible with SSSD 1.5.1_66.el6_2.3
Openldap 2.4.23 back_sql incompatible with SSSD 1.5.1_66.el6_2.3
Status: CLOSED WONTFIX
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: openldap (Show other bugs)
6.2
x86_64 Linux
unspecified Severity medium
: rc
: ---
Assigned To: Jan Synacek
David Spurek
:
Depends On:
Blocks: 1022999
  Show dependency treegraph
 
Reported: 2012-04-02 09:51 EDT by Rene Snajder
Modified: 2015-03-02 00:26 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1022999 (view as bug list)
Environment:
Last Closed: 2014-03-25 05:47:45 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
An upstream patch fixing the problem. See comments for more information (1.35 KB, patch)
2012-04-06 06:00 EDT, Rene Snajder
no flags Details | Diff
Reproducer pack (7.68 KB, application/x-gzip)
2012-04-16 08:23 EDT, Jan Synacek
no flags Details

  None (edit)
Description Rene Snajder 2012-04-02 09:51:39 EDT
I posted this on the OpenLDAP mailing lists since I think it is based on an OpenLDAP bug, but my posts never came through. When I posted it to the its mailing list (which is wrong) I only got the information that back_sql is not maintained anymore.

Description of problem:
I'm using the Openldap 2.4.23, with back-sql and postgresql as a backend.
I have SSSD which uses this openldap server for ID providing and authentication.
The update in SSSD to version 1.5.1_66 changed the filter used to retrieve
groupids of users, which surfaced what seems to be a bug in backsql.

From my investigation it seems to me that when constructing the search
query, openldap tries to use the UPPER function on every criteria in
the WHERE clause, no matter which type it is. This causes an error in
postgresql, as the gidNumber that is supposed to be filtered is of
type "bigint".

I could simply remove "upper_func" from my slapd.conf, but I need it
for other queries. Changing the field gidNumber in the database to
text would also be a workaround, but I'd rather prefer this fixed.

Version-Release number of selected component (if applicable):
Openldap 2.4.23 
with 
SSSD 1.5.1_66.el6_2.3

How reproducible:

Setup openldap with back_sql and postgres as a backend. Setup SSSD so that it uses openldap for ID providing.  Configure upper_func in the openldap config.

Here is the important part of my slapd.conf:

#-----------------------------------------------
database sql
suffix "dc=mydomain,dc=at"
rootdn "cn=Manager,dc=mydomain,dc=at"
rootpw mysecret
dbname mydbname
dbuser mydbuser
dbpasswd myuserpw
subtree_cond "UPPER(ldap_entries.dn) LIKE  UPPER('%'||?)"
insentry_stmt "insert into ldap_entries
(id,dn,oc_map_id,parent,keyval) values ((select max(id)+1 from
ldap_entries),?,?,?,?)"
upper_func "upper"
upper_needs_cast "yes"
strcast_func "text"
concat_pattern "?||?"
has_ldapinfo_dn_ru no
sizelimit       -1
#-----------------------------------------------
  

And here the important part of my sssd.conf:

#-----------------------------------------------
[domain/default]
ldap_id_use_start_tls = True
ldap_tls_reqcert = never 
ldap_tls_cacert = /path/to/my/cert
ldap_search_base = dc=mydomain,dc=at
ldap_user_search_base = ou=users,ou=myserver,dc=mydomain,dc=at
ldap_group_search_base = ou=groups,ou=myserver,dc=mydomain,dc=at
id_provider = ldap
ldap_uri = ldaps://127.0.0.1/
ldap_default_bind_dn = UID=nssldap,OU=dsa,OU=myserver,DC=mydomain,DC=at
ldap_default_authtok_type = password
ldap_default_authtok = secret
chpass_provider = ldap
enumerate = true
#------------------------------------------------

Then run the following command:

#-----------------------------------------------
ldapsearch -H ldaps://127.0.0.1/ -b ou=groups,ou=myserver,dc=mydomain,dc=at -D "uid=myuser,ou=users,ou=myserver,dc=mydomain,DC=at" '(gidNumber=512)' -W
#-----------------------------------------------

Actual results:

#-----------------------------------------------
# extended LDIF
#
# LDAPv3
# base <ou=groups,ou=myserver,dc=mydomain,dc=at> with scope subtree
# filter: (gidNumber=512)
# requesting: ALL
#

# search result
search: 2
result: 80 Other (e.g., implementation specific) error
#-----------------------------------------------

Expected results:

The user I searched for.

Additional info:

Below the openldap log output at log level 255:

#-----------------------------------------------
<==backsql_oc_get_candidates(): 0
==>backsql_oc_get_candidates(): oc="sambaDomain"
==>backsql_srch_query()
==>backsql_process_filter()
<==backsql_process_filter() succeeded
<==backsql_srch_query() returns SELECT DISTINCT
ldap_entries.id,samba_domain_ldap.id,text('sambaDomain') AS
objectClass,ldap_entries.dn AS dn FROM ldap_entries,samba_domain_ldap
WHERE samba_domain_ldap.id=ldap_entries.keyval AND
ldap_entries.oc_map_id=? AND UPPER(ldap_entries.dn) LIKE
UPPER('%'||?) AND 7=7
Constructed query: SELECT DISTINCT
ldap_entries.id,samba_domain_ldap.id,text('sambaDomain') AS
objectClass,ldap_entries.dn AS dn FROM ldap_entries,samba_domain_ldap
WHERE samba_domain_ldap.id=ldap_entries.keyval AND
ldap_entries.oc_map_id=? AND UPPER(ldap_entries.dn) LIKE
UPPER('%'||?) AND 7=7
id: '6'
(sub)dn: "%OU=GROUPS,OU=MYSERVER,DC=MYDOMAIN,DC=AT"
<==backsql_oc_get_candidates(): 0
==>backsql_oc_get_candidates(): oc="inetOrgPerson"
==>backsql_srch_query()
==>backsql_process_filter()
==>backsql_process_filter_attr(gidNumber)
<==backsql_process_filter_attr(gidNumber)
<==backsql_process_filter() succeeded
<==backsql_srch_query() returns SELECT DISTINCT
ldap_entries.id,OS_USER.id,text('inetOrgPerson') AS
objectClass,ldap_entries.dn AS dn FROM ldap_entries,OS_USER WHERE
OS_USER.id=ldap_entries.keyval AND ldap_entries.oc_map_id=? AND
UPPER(ldap_entries.dn) LIKE  UPPER('%'||?) AND
(upper(OS_USER.gidnumber)='512')
Constructed query: SELECT DISTINCT
ldap_entries.id,OS_USER.id,text('inetOrgPerson') AS
objectClass,ldap_entries.dn AS dn FROM ldap_entries,OS_USER WHERE
OS_USER.id=ldap_entries.keyval AND ldap_entries.oc_map_id=? AND
UPPER(ldap_entries.dn) LIKE  UPPER('%'||?) AND
(upper(OS_USER.gidnumber)='512')
id: '8'
(sub)dn: "%OU=GROUPS,OU=MYSERVER,DC=MYDOMAIN,DC=AT"
backsql_oc_get_candidates(): error executing query
Return code: -1
  nativeErrCode=7 SQLengineState=S1000 msg="[unixODBC]ERROR:
function upper(bigint) does not exist at character 288;#012Error while
executing the query"
#-----------------------------------------------

In the log one can clearly see that the upper function is used incorrectly in the SQL query. There are several queries that can be set in the back_sql database itself, but from my research and from skimming through the back_sql sourcecode it seems to just use the upper function for everything when filtering like this.

Thanks a lot
Comment 2 Rene Snajder 2012-04-04 06:46:44 EDT
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.
Comment 3 Rene Snajder 2012-04-04 07:51:45 EDT
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 ) )
#-----------------------------------------------
Comment 4 Rene Snajder 2012-04-06 05:58:22 EDT
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.
Comment 5 Rene Snajder 2012-04-06 06:00:01 EDT
Created attachment 575690 [details]
An upstream patch fixing the problem. See comments for more information
Comment 6 Jan Synacek 2012-04-13 04:29:06 EDT
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!
Comment 7 Rene Snajder 2012-04-13 04:41:22 EDT
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"?
Comment 8 Jan Synacek 2012-04-13 05:00:01 EDT
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 .
Comment 10 Jan Synacek 2012-04-16 08:23:49 EDT
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.
Comment 11 Jan Synacek 2012-04-16 08:29:36 EDT
Hello Rene,

if you agree, I can propose this patch upstream for you, mentioning you as the original author.
Comment 12 Rene Snajder 2012-04-16 08:35:46 EDT
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!
Comment 13 Jan Vcelak 2012-04-16 10:09:06 EDT
@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.)
Comment 14 Rene Snajder 2012-04-17 04:02:50 EDT
Ok, I gave it a try:
http://www.openldap.org/its/index.cgi?findid=7248
Comment 15 Jan Synacek 2012-04-17 05:33:27 EDT
> Ok, I gave it a try:
Great, thank you!
Comment 17 Rene Snajder 2012-04-30 10:18:43 EDT
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?
Comment 18 Jan Vcelak 2012-04-30 13:33:28 EDT
Rene, we would like to have your changes reviewed by upstream before including into RHEL. We do not want to cause any regressions.
Comment 19 Rene Snajder 2012-06-25 07:20:39 EDT
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)?
Comment 20 Jan Vcelak 2012-06-26 04:35:49 EDT
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
Comment 21 Jan Synacek 2012-11-20 08:20:31 EST
This has just popped up:

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

Might be of interest.
Comment 23 RHEL Product and Program Management 2013-10-13 20:44:03 EDT
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.
Comment 24 Jan Synacek 2014-03-25 05:47:45 EDT
The patch hasn't been touched by upstream and is very unlikely to be. I'm closing this bugzilla.

Note You need to log in before you can comment on or make changes to this bug.