Bug 11658 - LDAP Filter / DN's aren't escaped in AD Connector
LDAP Filter / DN's aren't escaped in AD Connector
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: AD Connector
UCS 4.2
All Linux
: P4 normal (vote)
: UCS 4.2-2-errata
Assigned To: Lukas Oyen
Felix Botner
:
: 31047 42470 45124 (view as bug list)
Depends on:
Blocks: 49041 45011
  Show dependency treegraph
 
Reported: 2008-07-29 09:44 CEST by Ingo Steuwer
Modified: 2019-03-19 15:58 CET (History)
6 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 3: A User would likely not purchase the product
User Pain: 0.171
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Cleanup, External feedback, Security, Usability
Max CVSS v3 score:
oyen: Patch_Available+


Attachments
Patch-series to fix LDAP (filter/DN) escaping in the adconnector (50.00 KB, application/x-tar)
2017-01-11 10:58 CET, Lukas Oyen
Details
Patch-series to fix LDAP (filter/DN) escaping in the adconnector (updated) (50.00 KB, application/x-tar)
2017-01-24 17:50 CET, Lukas Oyen
Details
Patch-series to fix LDAP (filter/DN) escaping in the adconnector (updated) (50.00 KB, application/x-tar)
2017-02-01 16:42 CET, Lukas Oyen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ingo Steuwer univentionstaff 2008-07-29 09:44:10 CEST
In AD kann man Gruppen mit Klammern setzen. Das unterstützen wir nicht, der Conenctor sollte aber eine sauberere Fehlermeldung liefern. Dazu muss mindestens dieser Suchfilter korrekt die Klammern setzen:

unexpected Error during ad.resync_rejected
Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/univention/connector/ad/__init__.py", line 1511, in resync_rejected
    mapped_object = self._object_mapping(property_key,object)
  File "/usr/lib/python2.4/site-packages/univention/connector/__init__.py", line 1135, in _object_mapping
    object=function(self, object, dn_mapping_stored, isUCSobject=(object_type == 'ucs'))
  File "/usr/lib/python2.4/site-packages/univention/connector/ad/__init__.py", line 288, in group_dn_mapping
    return samaccountname_dn_mapping(connector, given_object, dn_mapping_stored, isUCSobject, 'group', u'cn', u'posixGroup', 'cn', u'group')
  File "/usr/lib/python2.4/site-packages/univention/connector/ad/__init__.py", line 253, in samaccountname_dn_mapping
    base=connector.lo.base, scope='sub', attr=['objectClass'])
  File "/usr/lib/python2.4/site-packages/univention/admin/uldap.py", line 296, in search
    raise univention.admin.uexceptions.ldapError, msg[0]['desc']
ldapError: Bad search filter
Comment 1 Ingo Steuwer univentionstaff 2010-04-30 14:16:52 CEST
Diese Fehlermeldung ist so scheinbar nicht mehr relevant, wenn man manuell Gruppen mit Klammern erlaubt (siehe Bug #18296)
Comment 2 Lukas Walter univentionstaff 2013-02-22 11:30:11 CET
Das Problem tritt in anderer Form noch auf.

Anlegen einer Gruppe "group()group" auf AD Seite führt zu folgendem Traceback im connector.log:

 (PROCESS): sync to ucs:   [         group] [       add] cn=group()group,cn=users,dc=lwa,dc=dev
22.02.2013 11:28:17,324 LDAP        (INFO   ): sync_to_ucs: set position to cn=users,dc=lwa,dc=dev
22.02.2013 11:28:17,324 LDAP        (INFO   ): sync_to_ucs: remove cn=group()group,cn=users,dc=lwa,dc=dev from ucs group cache
22.02.2013 11:28:17,324 LDAP        (INFO   ): __set_values: set attribute, ucs_key: name - value: [u'group()group']
22.02.2013 11:28:17,325 LDAP        (INFO   ): __set_values: module groups/group has custom attributes
22.02.2013 11:28:17,325 LDAP        (INFO   ): set key in ucs-object: name
22.02.2013 11:28:17,325 LDAP        (INFO   ): __set_values: no ldap_attribute defined in <univention.connector.attribute instance at 0x984f72c>, we unset the key description in the ucs-object
22.02.2013 11:28:17,373 LDAP        (ERROR  ): Unknown Exception during sync_to_ucs
22.02.2013 11:28:17,373 LDAP        (ERROR  ): Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/univention/connector/__init__.py", line 1233, in sync_to_ucs
    result = self.add_in_ucs(property_type, object, module, position)
  File "/usr/lib/pymodules/python2.6/univention/connector/__init__.py", line 1101, in add_in_ucs
    return ucs_object.create() and self.__modify_custom_attributes(property_type, object, ucs_object, module, position)
  File "/usr/lib/pymodules/python2.6/univention/admin/handlers/__init__.py", line 331, in create
    return self._create()
  File "/usr/lib/pymodules/python2.6/univention/admin/handlers/__init__.py", line 628, in _create
    al=self._ldap_addlist()
  File "/usr/lib/pymodules/python2.6/univention/admin/handlers/groups/group.py", line 569, in _ldap_addlist
    raise univention.admin.uexceptions.groupNameAlreadyUsed, ': %s' % (name)
groupNameAlreadyUsed: : group()group
Comment 3 Florian Best univentionstaff 2016-09-28 18:11:26 CEST
*** Bug 42470 has been marked as a duplicate of this bug. ***
Comment 4 Florian Best univentionstaff 2016-09-28 18:14:39 CEST
I remove the univentionstaff flag at this bug.
The security impact of not escaping things is obvious and no secret.
This bugs allows users with write-permissions to UDM objects to construct evil filters or move their objects into different subtrees.
Comment 5 Florian Best univentionstaff 2016-09-28 18:19:03 CEST
I guess the code which is responsible for the traceback in Bug #42470 is to simply ignore invalid DN syntax (ldap.INVALID_DN_SYNTAX exception):

services/univention-ad-connector/modules/univention/connector/__init__.py:
 750 »   def get_ucs_ldap_object(self, dn):
 751 »   »   _d=ud.function('ldap.get_ucs_ldap_object')
 752 
 753 »   »   if type(dn) == type(u''):
 754 »   »   »   searchdn = dn
 755 »   »   else:
 756 »   »   »   searchdn = unicode(dn)
 757 »   »   try:
 758 »   »   »   return self.lo.get(searchdn,required=1)
 759 »   »   except ldap.NO_SUCH_OBJECT:
 760 »   »   »   return None
 761 »   »   except ldap.INVALID_DN_SYNTAX:
 762 »   »   »   return None
 763 »   »   except ldap.INVALID_SYNTAX:
 764 »   »   »   return None
Comment 6 Lukas Oyen univentionstaff 2017-01-11 10:58:20 CET
Created attachment 8346 [details]
Patch-series to fix LDAP (filter/DN) escaping in the adconnector

The attached patch-series fixes the LDAP filter-expression/DN escaping in the ad-connector. It also cleans up LDAP operations in several places (in favour of functions from `univention.uldap`).

All ad-connector tests running on a UCS master 4.1-4 connected to a Windows Server 2012 in sync mode are passing.
Comment 7 Lukas Oyen univentionstaff 2017-01-24 11:51:10 CET
(In reply to Lukas Oyen from comment #6)
> Created attachment 8346 [details]
> Patch-series to fix LDAP (filter/DN) escaping in the adconnector
> 
> The attached patch-series fixes the LDAP filter-expression/DN escaping in
> the ad-connector. It also cleans up LDAP operations in several places (in
> favour of functions from `univention.uldap`).
> 
> All ad-connector tests running on a UCS master 4.1-4 connected to a Windows
> Server 2012 in sync mode are passing.

This does not regard the `mapping.py`. I will update it soon.
Comment 8 Lukas Oyen univentionstaff 2017-01-24 17:50:34 CET
Created attachment 8374 [details]
Patch-series to fix LDAP (filter/DN) escaping in the adconnector (updated)

Updated the patch-series to also include the changes to the `mapping.py` file.
Comment 9 Lukas Oyen univentionstaff 2017-02-01 16:42:05 CET
Created attachment 8392 [details]
Patch-series to fix LDAP (filter/DN) escaping in the adconnector (updated)

Updated patchset rebased on current 4.2-0 branch and with fixed indentation.
Comment 10 Lukas Oyen univentionstaff 2017-07-27 14:42:28 CEST
Committed in r81450-81459 (tests r81461-81466, advisory r81468).
Comment 11 Florian Best univentionstaff 2017-07-27 16:11:56 CEST
*** Bug 31047 has been marked as a duplicate of this bug. ***
Comment 12 Florian Best univentionstaff 2017-08-03 17:51:33 CEST
See also Bug #32086 comment 19.
Comment 13 Florian Best univentionstaff 2017-08-04 15:43:50 CEST
*** Bug 45124 has been marked as a duplicate of this bug. ***
Comment 14 Lukas Oyen univentionstaff 2017-08-07 08:54:16 CEST
Committed the fix from bug #45124 as r81807.
Comment 15 Florian Best univentionstaff 2017-08-15 16:11:31 CEST
hmm, svn r81452 makes behavior changes (See Bug #43420 comment 1). One of them at least is still wrong:

modules/univention/connector/ad/__init__.py:»   »   »   default_naming_context = self.lo_ad.getAttr('', 'defaultNamingContext')

→ I think a test case for this would be good, too.
Comment 16 Lukas Oyen univentionstaff 2017-08-16 17:15:03 CEST
(In reply to Florian Best from comment #15)
> modules/univention/connector/ad/__init__.py:»   »   »  
> default_naming_context = self.lo_ad.getAttr('', 'defaultNamingContext')

Fixed in r82202.

Also fixed the issue from bug #18501 comment 17 in r82201.

The issue was, that the `ignore_filter` for "user" was not correctly generated because of a typo. This resulted in "Administrator" beeing _wrongly_ synced.
Comment 17 Lukas Oyen univentionstaff 2017-08-16 17:24:51 CEST
Tests updated to also rename/move users/groups in r82211-r82215.
Comment 18 Felix Botner univentionstaff 2017-10-26 10:35:04 CEST
OK - ad group with parenthesis 

25.10.2017 00:22:56,314 LDAP        (PROCESS): sync to ucs:   [         group] [       
25.10.2017 00:22:56,317 LDAP        (ERROR  ): InvalidSyntax: Name: Value may not contain other than numbers, letters and dots! (cn=g(100),cn=users,dc=four,dc=two)
...

OK - code review
OK - jenkins test
OK - connector with old mapping
OK - YAML
Comment 19 Arvid Requate univentionstaff 2017-11-01 13:49:16 CET
<http://errata.software-univention.de/ucs/4.2/205.html>