Skip to content
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

ldap / ldap 2 level datasource not returning results #785

Closed
xavierba opened this issue Nov 7, 2019 · 12 comments
Closed

ldap / ldap 2 level datasource not returning results #785

xavierba opened this issue Nov 7, 2019 · 12 comments
Labels
bug ready A PR is waiting to be merged. Close to be solved
Milestone

Comments

@xavierba
Copy link
Contributor

xavierba commented Nov 7, 2019

Version

6.2.48

Installation method

RPM

Expected behavior

ldap / ldap 2 level datasource properly returns some mails to include in the list members.

Actual behavior

No member returned.

Additional information

This was working fine with 6.2.44, so I suspect a regression in the datasource rewrite.
Sync'ing the datasource from either the WUI or the CLI doesn't return any error.

# sudo -u sympa sympa.pl -d --sync_include [email protected]
Members of list [email protected] have been successfully update.

Please note the -d switch doesn't actually give more verbosity to the output.

Here's the corresponding log:

Nov  7 11:30:40 sympa-srv sympa[12016]: info main::_load() Configuration file read, log level set using options: 2
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Conf::cookie_changed() Cookie is stable
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::Database::new(Sympa::Database, mysql)
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::Database::connect() Connected to Database Sympa::DatabaseDriver::MySQL <db_name=sympa;db_user=sympa>
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Conf::get_robots_list() Retrieving the list of robots on the server
Nov  7 11:30:40 sympa-srv sympa[12016]: notice main:: Sympa 6.2.48 Started
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::List::new([email protected], , )
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::List::load(Sympa::List <>, some_list, domain.tld, ...)
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::List::sync_include_admin(Sympa::List <[email protected]>)
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::List::get_admins(Sympa::List <[email protected]>, owner,  => )
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::List::sync_include(Sympa::List <[email protected]>, )
Nov  7 11:30:40 sympa-srv sympa[12016]: notice Sympa::Spindle::ProcessRequest::_twist() Processing Sympa::Request <action=include;[email protected];role=member>
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::DataSource::new() ,,,...
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::Database::new(Sympa::Database, LDAP)
Nov  7 11:30:40 sympa-srv sympa[12016]: debug2 Sympa::Database::connect() Connected to Database Sympa::DatabaseDriver::LDAP <ca_file=/etc/pki/tls/certs/ca-bundle.crt;ca_verify=none;host=ldap://ldap.domain.tld;ssl_ciphers=ALL;ssl_version=tlsv1;timeout=30;use_tls=starttls>
Nov  7 11:30:40 sympa-srv sympa[12016]: info Sympa::Request::Handler::include::_twist() Sympa::DataSource::LDAP2 <[email protected];id=02613d60;role=member;name=ldap>: 0 included, 0 deleted, 0 updated, 0 kept
Nov  7 11:30:40 sympa-srv sympa[12016]: info Sympa::Request::Handler::include::_twist() Sympa::Request <action=include;[email protected];role=member>: 0 included, 0 deleted, 0 updated
@ikedas
Copy link
Member

ikedas commented Nov 7, 2019

@xavierba, could you please show you datasource settings?

@xavierba
Copy link
Contributor Author

xavierba commented Nov 7, 2019

Here they are, slightly modified for hiding private details:

include_ldap_query
attrs mail
ca_verify none
filter physicalDeliveryOfficeName=XXXX
ssl_ciphers ALL
name ldap
host ldap.domain.tld
timeout 30
ssl_version tlsv1
use_tls none
scope sub
select first

A similar query run from a simple ldapsearch works.

@ikedas ikedas added the bug label Nov 8, 2019
@ikedas
Copy link
Member

ikedas commented Nov 8, 2019

Hi,

  • suffix parameter seems required.
  • If you are using OpenLDAP or alike, could you please run ldapsearch and tell us what is shown?

@xavierba
Copy link
Contributor Author

xavierba commented Nov 8, 2019

edit: fixed below

@xavierba xavierba closed this as completed Nov 8, 2019
@xavierba
Copy link
Contributor Author

xavierba commented Nov 8, 2019

The backend is openldap

ldapsearch -xZZLLL -H ldap://ldap.domain.tld -b "ou=people,dc=xxxx,dc=xxxxx" "physicaleliveryOfficeName=xxxxx" "mail"
dn: uid=xxxxxt,ou=people,dc=xxxxx,dc=xxxxx
mail: [email protected]

The suffix parameter was not added to the freshly created list (6.2.48), however both the suffix1 and suffix2 parameters were there in the other list which predate the update to 6.2.48.

@xavierba xavierba reopened this Nov 8, 2019
@ikedas ikedas added the wontfix label Nov 8, 2019
@xavierba
Copy link
Contributor Author

xavierba commented Nov 8, 2019

ok, got the ldap 1 level query to work.
I added the missing suffix and also properly set the use_tls parameter to startls.
I would have expected sympa to return an error w/o the use_tls starttls as my ldap server denies non-TLS queries.

Anyway, there is probably still an issue with ldap 2 level query, as a list using this was working before the update from 6.2.44 to 6.2.48.

@ikedas
Copy link
Member

ikedas commented Nov 8, 2019

If you are using OpenSSL, could you please run openssl s_client -starttls ldap ... and tell us what is shown?

@ikedas ikedas removed the wontfix label Nov 8, 2019
@xavierba
Copy link
Contributor Author

xavierba commented Nov 8, 2019

The old EL6 openssl doesn't know about -startls ldap, but this works fine from an EL7 client.
I'm using a private PKI, but the CA is properly deployed in the system-wide location and the ldap server certificate verifies prooperly. Please note the test sympa list for ldap 1 level query was configured to not verify the ldap server certificate anyway. Switching it to ca_verify required is working as well.

At this point, I think the possible bug is only with ldap 2 level query.
However, I think ldap 1 level query should have displayed an error when I improperly configured it (that is, before setting use_tls starttls). Actually, I believe no error is ever displayed. I tried to setup a filter with an invalid syntax and sympa happily accepted it w/o displaying an error (WUI or CLI).

@racke
Copy link
Contributor

racke commented Nov 8, 2019

Yes that is really a shortcoming. Errors don't bubble up to the top.

@xavierba
Copy link
Contributor Author

xavierba commented Nov 8, 2019

About suffix (and possibly suffix1 and suffix2), sympa should probably not accept to define a datasource without a mandatory parameter.

@ikedas
Copy link
Member

ikedas commented Nov 8, 2019

The old EL6 openssl doesn't know about -startls ldap, but this works fine from an EL7 client.

You may also use $SCRIPTDIR/sympa_test_ldap.pl.

@xavierba
Copy link
Contributor Author

xavierba commented Nov 8, 2019

sympa_test_ldap.pl gives me proper results. Tested with both ldap+starttls and ldaps, with mandatory server certificate validation. Also, properly reports error with plain ldap and with invalid filter too. Tested with asking either for a single (mail) or dual attributes (mail,cn).

While I did improperly set my test ldap 1 level datasource when testing, this is now working. I really don't think the error is in the config with the ldap 2 level datasource, as the setup was working before the upgrade from 6.2.44 to 6.2.48.

@ikedas ikedas added the ready A PR is waiting to be merged. Close to be solved label Nov 9, 2019
@ikedas ikedas added this to the 6.2.50 milestone Nov 9, 2019
ikedas added a commit that referenced this issue Nov 15, 2019
Patch for a working include_ldap_2level_query (#785)
@ikedas ikedas closed this as completed Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready A PR is waiting to be merged. Close to be solved
Projects
None yet
Development

No branches or pull requests

3 participants