Bug 39345 - urlencode ldap base in slapd.conf
urlencode ldap base in slapd.conf
Status: RESOLVED MOVED
Product: UCS
Classification: Unclassified
Component: LDAP
UCS 4.4
Other Linux
: P5 normal (vote)
: ---
Assigned To: Nikola Radovanovic
Arvid Requate
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-09-15 08:53 CEST by Florian Best
Modified: 2021-11-01 18:13 CET (History)
4 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): bitesize
Max CVSS v3 score:


Attachments
Additional check at setup time (690 bytes, patch)
2019-01-08 15:09 CET, Martin Castillo
Details | Diff
Add ldap/base check for some templates (8.37 KB, patch)
2019-01-08 15:10 CET, Martin Castillo
Details | Diff
patch (2.53 KB, patch)
2019-06-13 19:33 CEST, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2015-09-15 08:53:39 CEST
The LDAP base is not urlencoded in any of /etc/univention/templates/files/etc/ldap/slapd.conf.d/*.
This makes it impossible to have e.g. a space in it.
See also Bug #29482 comment 7
Comment 1 Florian Best univentionstaff 2015-09-22 13:18:37 CEST
univention.lib.misc.getLDAPURIs() also does no urlencoding.
Comment 2 Stefan Gohmann univentionstaff 2019-01-03 07:16:25 CET
This issue has been filled against UCS 4.0. The maintenance with bug and security fixes for UCS 4.0 has ended on 31st of May 2016.

Customers still on UCS 4.0 are encouraged to update to UCS 4.3. Please contact
your partner or Univention for any questions.

If this issue still occurs in newer UCS versions, please use "Clone this bug" or simply reopen the issue. In this case please provide detailed information on how this issue is affecting you.
Comment 3 Martin Castillo 2019-01-08 15:09:15 CET
Created attachment 9793 [details]
Additional check at setup time
Comment 4 Martin Castillo 2019-01-08 15:10:22 CET
Created attachment 9794 [details]
Add ldap/base check for some templates
Comment 5 Martin Castillo 2019-01-08 15:14:53 CET
The ldap/base is checked at UCS install time to not contain strange characters. The 1st patch adds another check just to be sure.

Therefore the ldap/base in UCR can only contain unexpected characters if something goes really wrong.

One (ugly) way to check that is in the 2nd, UNCOMPLETE patch. There are more templates, that need that check added.

Perhaps someone has a nicer way to check and error out.
Comment 6 Florian Best univentionstaff 2019-01-08 19:14:50 CET
Hello Martin,

the first patch looks unnecessary, as the call to explode_dn() in univention-system-setup/umc/python/setup/util.py:is_ldap_base() already does a is_dn() internally.

The second patch adds a general validation of the LDAP base.
This would be necessary if one changes the ldap base via "ucs set" which hopefully never happens on a installed system. A invalid base will cause the system to not work at all. 

This bug entry is about encoding special characters which would make the "LDAP URI" invalid, if the *valid* ldap base contains special characters. A patch for this would look like the following, for every URI which is inserted into the config files:


diff --git a/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-slave b/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-slave
index 40bb6e3..bfaef0a 100644
--- a/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-slave
+++ b/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-slave
@@ -3 +3,4 @@ authz-regexp
-    ldap:///@%@ldap/base@%@??sub?uid=$1
+@!@
+from urllib import quote
+print '\tldap:///%s??sub?uid=$1' % (quote(configRegistry['ldap/base']),)
+@!@

diff --git a/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-master b/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-master
index 3d7aecd..9849ba0 100644
--- a/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-master
+++ b/management/univention-ldap/conffiles/etc/ldap/slapd.conf.d/60univention-ldap-server_acl-master
@@ -2,0 +3 @@ from univention.lib.misc import custom_username, custom_groupname
+from urllib import quote
@@ -13 +14 @@ print '    uid=([^,]*),cn=(gssapi|saml),cn=auth'
-print '    ldap:///%s??sub?uid=$1' % (ldap_base,)
+print '    ldap:///%s??sub?uid=$1' % (quote(ldap_base),)

diff --git a/base/univention-lib/python/misc.py b/base/univention-lib/python/misc.py
index 88e1e5a..1477baf 100644
--- a/base/univention-lib/python/misc.py
+++ b/base/univention-lib/python/misc.py
@@ -34,0 +35 @@
+from urllib import quote
@@ -83 +84 @@ def getLDAPURIs(configRegistryInstance=None):
-»   »   urilist = ["ldap://%s:%s" % (host, port) for host in ldaphosts]
+»   »   urilist = ["ldap://%s:%s" % (quote(host), quote(port)) for host in ldaphosts]
Comment 7 Florian Best univentionstaff 2019-06-13 19:33:28 CEST
Created attachment 10063 [details]
patch

I found an old untested patch. Maybe this can be used.
Comment 8 Nikola Radovanovic univentionstaff 2021-11-01 17:22:11 CET
https://git.knut.univention.de/univention/ucs/-/merge_requests/157

When/If above MR is approved, I can backport to 4.4 if needed.
Comment 9 Florian Best univentionstaff 2021-11-01 18:10:53 CET
I found an old branch which has several enhancements compared to attachment 10063 [details]:
https://git.knut.univention.de/univention/ucs/-/tree/fbest/39345-quote-urlencode-ldap-base

Can you please adopt or cherry-pick the changes there?
They also add regex-escaping, as not everything in slapd.conf is an URI, there are also regular expressions (in PCRE format).

(In reply to Nikola Radovanovic from comment #8)
> https://git.knut.univention.de/univention/ucs/-/merge_requests/157
> 
> When/If above MR is approved, I can backport to 4.4 if needed.
I don't think a backport will be necessary.
Comment 10 Florian Best univentionstaff 2021-11-01 18:13:42 CET
… as well as string escaping.