Bug 40672 - UDM generates broken search filters
UDM generates broken search filters
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.3-2-errata
Assigned To: Johannes Keiser
Dirk Wiesenthal
:
: 32167 47364 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-16 16:40 CET by Florian Best
Modified: 2022-07-27 09:41 CEST (History)
8 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 4: Minor Usability: Impairs usability in secondary scenarios
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.091
Enterprise Customer affected?: Yes
School Customer affected?:
ISV affected?: Yes
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2017110221000119
Bug group (optional): Cleanup, Troubleshooting, Usability
Max CVSS v3 score:
schwardt: Patch_Available+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2016-02-16 16:40:10 CET
All handlers which create search filter in lookup() or in lookup_filter() for a specific property which has a multivalue mapping generate broken LDAP search filters.

One example: 
Call lookup() of dns/srv_record with: filter_s='location=*xen3*'.
This creates the following search filter string:
(&(objectClass=dNSZone)(!(relativeDomainName=@))(!(zoneName=*.in-addr.arpa))(!(zoneName=*.ip6.arpa))(sRVRecord=*)(zoneName=school.local)(sRVRecord=['*', 'x', 'e', 'n', '3', '*']))

The reason is because univention.admin.mapping.mapRewrite() calls mapping.mapValue() which returns a list in these cases instead of a string.
Even worse is that some handlers causes an exception with that logic, see comment #1.

Here is a list of all broken searches I found using the search filter '*foo*':
FAIL 'dhcp/host' 'hwaddress'                      searchable: 1 filter: (dhcpHWAddress=* f)
FAIL 'dhcp/pool' 'range'                          searchable: 1 filter: (dhcpRange=['*', 'f', 'o', 'o', '*'])
FAIL 'dhcp/sharedsubnet' 'range'                  searchable: 1 filter: (dhcpRange=['*', 'f', 'o', 'o', '*'])
FAIL 'dns/host_record' 'mx'                       searchable: 1 filter: (mXRecord=['*', 'f', 'o', 'o', '*'])
FAIL 'dns/reverse_zone' 'subnet'                  searchable: 1 filter: (zoneName=*foo*.in-addr.arpa)
FAIL 'dns/srv_record' 'location'                  searchable: 1 filter: (sRVRecord=['*', 'f', 'o', 'o', '*'])
FAIL 'networks/network' 'ipRange'                 searchable: 0 filter: (univentionIpRange=['*', 'f', 'o', 'o', '*'])
FAIL 'policies/nfsmounts' 'nfsMounts'             searchable: 1 filter: (univentionNFSMounts=['*', 'f', 'o', 'o', '*'])
FAIL 'policies/print_quota' 'quotaGroupsPerUsers' searchable: 1 filter: (univentionPrintQuotaGroupsPerUsers=['*', 'f', 'o', 'o', '*'])
FAIL 'policies/print_quota' 'quotaGroups'         searchable: 1 filter: (univentionPrintQuotaGroups=['*', 'f', 'o', 'o', '*'])
FAIL 'policies/print_quota' 'quotaUsers'          searchable: 1 filter: (univentionPrintQuotaUsers=['*', 'f', 'o', 'o', '*'])
FAIL 'settings/ldapacl' 'data'                    searchable: 1 filter: (univentionLDAPACLData=)
FAIL 'settings/ldapschema' 'data'                 searchable: 1 filter: (univentionLDAPSchemaData=)
FAIL 'settings/udm_hook' 'data'                   searchable: 1 filter: (univentionUDMHookData=)
FAIL 'settings/udm_module' 'data'                 searchable: 1 filter: (univentionUDMModuleData=)
FAIL 'settings/udm_module' 'icon'                 searchable: 1 filter: (univentionUMCIcon=)
FAIL 'settings/udm_module' 'umcregistration'      searchable: 1 filter: (univentionUMCRegistrationData=)
FAIL 'settings/udm_syntax' 'data'                 searchable: 1 filter: (univentionUDMSyntaxData=)
FAIL 'settings/umc_operationset' 'operation'      searchable: 0 filter: (umcOperationSetCommand=['*', 'f', 'o', 'o', '*'])
FAIL 'shares/share' 'sambaInvalidUsers'           searchable: 1 filter: (univentionShareSambaInvalidUsers=*, f, o, o, *)
FAIL 'shares/share' 'sambaValidUsers'             searchable: 1 filter: (univentionShareSambaValidUsers=*, f, o, o, *)
FAIL 'users/user' 'jpegPhoto'                     searchable: 0 filter: (jpegPhoto=)
FAIL 'users/user' 'sambaUserWorkstations'         searchable: 1 filter: (sambaUserWorkstations=*,f,o,o,*)
FAIL 'users/user' 'userCertificate'               searchable: 0 filter: (userCertificate;binary=)
FAIL 'uvmm/profile' 'bootdev'                     searchable: 1 filter: (univentionVirtualMachineProfileBootDevices=*,f,o,o,*)
EXC 'dns/forward_zone' 'mx' string index out of range
EXC 'settings/printermodel' 'printmodel' string index out of range
EXC 'shares/share' 'sambaCustomSettings' string index out of range
EXC 'users/passwd' 'password' 'module' object has no attribute 'mapping'
EXC 'users/passwd' 'username' 'module' object has no attribute 'mapping'
EXC 'users/user' 'sambaLogonHours' 'in <string>' requires string as left operand, not int
EXC 'users/user' 'umcProperty' string index out of range
EXC 'uvmm/cloudconnection' 'parameter' string index out of range
Comment 1 Florian Best univentionstaff 2016-02-16 16:42:41 CET
When e.g. searching for "Option name in smb.conf and its value" in shares/share the following traceback occurs. We can't get these traceback as external feedback as we are suppressing error pop ups in the search (which should be changed, too).

Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/notifier/threads.py", line 82, in _run
    tmp = self._function()
  File "/usr/lib/pymodules/python2.7/notifier/__init__.py", line 104, in __call__
    return self._function( *tmp, **self._kwargs )
  File "/usr/lib/pymodules/python2.7/univention/management/console/modules/udm/__init__.py", line 532, in _thread
    result = module.search(container, objectProperty, objectPropertyValue, superordinate, scope=scope, hidden=hidden)
  File "/usr/lib/pymodules/python2.7/univention/management/console/modules/udm/udm_ldap.py", line 84, in _decorated
    return method(*args, **kwargs)
  File "/usr/lib/pymodules/python2.7/univention/management/console/ldap.py", line 135, in _decorated
    result = func(*args, **kwargs)
  File "/usr/lib/pymodules/python2.7/univention/management/console/modules/udm/udm_ldap.py", line 473, in search
    result = self.module.lookup(None, ldap_connection, filter_s, base=container, superordinate=superordinate, scope=scope, sizelimit=sizelimit)
  File "/usr/lib/pymodules/python2.7/univention/admin/handlers/shares/share.py", line 977, in lookup
    filter=lookup_filter(filter_s)
  File "/usr/lib/pymodules/python2.7/univention/admin/handlers/shares/share.py", line 973, in lookup_filter
    lookup_filter_obj.append_unmapped_filter_string(filter_s, univention.admin.mapping.mapRewrite, mapping)
  File "/usr/lib/pymodules/python2.7/univention/admin/filter.py", line 88, in append_unmapped_filter_string
    walk(filter_p, rewrite_function, arg=mapping)
  File "/usr/lib/pymodules/python2.7/univention/admin/filter.py", line 231, in walk
    expression_walk_function(filter, arg)
  File "/usr/lib/pymodules/python2.7/univention/admin/mapping.py", line 265, in mapRewrite
    v=mapping.mapValue(filter.variable, filter.value)
  File "/usr/lib/pymodules/python2.7/univention/admin/mapping.py", line 193, in mapValue
    res=self._map[map_name][1](value)
  File "/usr/lib/pymodules/python2.7/univention/admin/handlers/shares/share.py", line 727, in mapKeyAndValue
    lst.append( '%s = %s' % (entry[0], entry[1]) )
IndexError: string index out of range
Comment 2 Florian Best univentionstaff 2016-02-16 18:12:54 CET
We also have another problem:
Shares → select to search for "Samba write access" which gives you a checkbox.
The search will cause a UDM filter of:
sambaWriteable=*False* or sambaWriteable=*True*
Which results in a LDAP filter of:
(univentionShareSambaWriteable=no)

All of the following are therefore also broken:
'dns/alias' 'zonettl'                        searchable: 1 filter: (dNSTTL=0)
'dns/forward_zone' 'zonettl'                 searchable: 1 filter: (dNSTTL=0)
'dns/host_record' 'zonettl'                  searchable: 1 filter: (dNSTTL=0)
'dns/reverse_zone' 'zonettl'                 searchable: 1 filter: (dNSTTL=0)
'dns/srv_record' 'zonettl'                   searchable: 1 filter: (dNSTTL=0)
'dns/txt_record' 'zonettl'                   searchable: 1 filter: (dNSTTL=0)
'policies/dhcp_leasetime' 'lease_time_default'searchable: 1 filter: (univentionDhcpLeaseTimeDefault=0)
'policies/dhcp_leasetime' 'lease_time_max'   searchable: 1 filter: (univentionDhcpLeaseTimeMax=0)
'policies/dhcp_leasetime' 'lease_time_min'   searchable: 1 filter: (univentionDhcpLeaseTimeMin=0)
'settings/sambaconfig' 'disconnectTime'      searchable: 1 filter: (univentionSambaDisconnectTime=0)
'settings/sambaconfig' 'lockoutDuration'     searchable: 1 filter: (univentionSambaLockoutDuration=0)
'settings/sambaconfig' 'logonToChangePW'     searchable: 1 filter: (univentionSambaLogonToChangePW=0)
'settings/sambaconfig' 'maxPasswordAge'      searchable: 1 filter: (univentionSambaMaxPasswordAge=0)
'settings/sambaconfig' 'minPasswordAge'      searchable: 1 filter: (univentionSambaMinPasswordAge=0)
'settings/sambadomain' 'disconnectTime'      searchable: 1 filter: (sambaForceLogoff=0)
'settings/sambadomain' 'lockoutDuration'     searchable: 1 filter: (sambaLockoutDuration=0)
'settings/sambadomain' 'logonToChangePW'     searchable: 1 filter: (sambaLogonToChgPwd=0)
'settings/sambadomain' 'maxPasswordAge'      searchable: 1 filter: (sambaMaxPwdAge=0)
'settings/sambadomain' 'minPasswordAge'      searchable: 1 filter: (sambaMinPwdAge=0)
'shares/share' 'root_squash'                 searchable: 1 filter: (univentionShareNFSRootSquash=no)
'shares/share' 'sambaBrowseable'             searchable: 1 filter: (univentionShareSambaBrowseable=no)
'shares/share' 'sambaDosFilemode'            searchable: 1 filter: (univentionShareSambaDosFilemode=no)
'shares/share' 'sambaHideUnreadable'         searchable: 1 filter: (univentionShareSambaHideUnreadable=no)
'shares/share' 'sambaInheritOwner'           searchable: 1 filter: (univentionShareSambaInheritOwner=no)
'shares/share' 'sambaInheritPermissions'     searchable: 1 filter: (univentionShareSambaInheritPermissions=no)
'shares/share' 'sambaMSDFSRoot'              searchable: 1 filter: (univentionShareSambaMSDFS=no)
'shares/share' 'sambaPublic'                 searchable: 1 filter: (univentionShareSambaPublic=no)
'shares/share' 'sambaWriteable'              searchable: 1 filter: (univentionShareSambaWriteable=no)
'shares/share' 'subtree_checking'            searchable: 1 filter: (univentionShareNFSSubTree=no)
'shares/share' 'writeable'                   searchable: 1 filter: (univentionShareWriteable=no)
Comment 3 Philipp Hahn univentionstaff 2016-10-29 17:13:38 CEST
It is also wrong for "single-valued" properties using a "complex syntax" like the MAC address for dhcp/hosts.
handlers/dhcp/host.py # mapHWAddress is broken - here the output of my extended version printing the arguments:

># /usr/share/pyshared/univention/admincli/admin.py dhcp/host list --superordinate="$service" --filter hwaddress='ethernet d6:d3:29:68:f4:e5'
>map=<univention.admin.mapping.mapping instance at 0x288a488> filt=expression('hwaddress', 'ethernet d6:d3:29:68:f4:e5', '=')
>map old='ethernet d6:d3:29:68:f4:e5'
>(&(objectClass=univentionDhcpHost)(dhcpHWAddress=e t))

Broken:
def mapHWAddress(old):
        univention.debug.debug(univention.debug.ADMIN, univention.debug.INFO, 'host.py: mapHWAddress: old: %s' % old)
        print "map old=%r" % (old,) # udm -> ldap
        if not old[0]:
                return ''
        else:
                if len (old) > 1:
                        return '%s %s' % (old[0], old[1])
                else:
                        return old

Fixed:
def mapHWAddress(old):
        univention.debug.debug(univention.debug.ADMIN, univention.debug.INFO, 'host.py: mapHWAddress: old: %s' % old)
        print "map old=%r" % (old,) # udm -> ldap
        return old if isinstance(old, basestring) else ' '.join(old)


The documentation on <http://wiki.univention.de/index.php?title=Entwicklung_und_Integration_eigener_Module_in_Univention_Directory_Manager#mapping> is also unclear, if the map function should map a single UDM-proterty-value to a single LDAP-attribute-value (and vis-versa), or if it must map a "list of LDAP-attributes-values" to either a "single UDM-property-value (if the property is single-valued), or to a list of UDM-property-values (if it is multi-valued).
As the same mapping function is used to build the filter string, it must magically have a completely different behavior then:

# /usr/share/pyshared/univention/admincli/admin.py dhcp/host create --superordinate="$service" --set hwaddress='ethernet d6:d3:29:68:f4:ff' --set host=ff
map old=['ethernet', 'd6:d3:29:68:f4:ff']

# /usr/share/pyshared/univention/admincli/admin.py dhcp/host list --superordinate="$service" --filter hwaddress='ethernet d6:d3:29:68:f4:ff'
map=<univention.admin.mapping.mapping instance at 0x30330e0> filt=expression('hwaddress', 'ethernet d6:d3:29:68:f4:ff', '=')
map old='ethernet d6:d3:29:68:f4:ff'
filter=conjunction('&', [expression('objectClass', 'univentionDhcpHost', '='), expression('dhcpHWAddress', 'ethernet d6:d3:29:68:f4:ff', '=')])
unmap old=['ethernet d6:d3:29:68:f4:ff']
hwaddress=ethernet d6:d3:29:68:f4:ff
Comment 4 Philipp Hahn univentionstaff 2017-11-02 10:47:15 CET
Customer probably used "--append" instead of "--set" and now "udm networks/network list" crashes:
> INVALID_DN_SYNTAX: {'info': 'invalid DN', 'desc': 'Invalid DN syntax'}

# udm computers/domaincontroller_slave modify --dn cn=yyy,$lb --append network="cn=default,cn=networks,$lb"
...
  File "/usr/lib/pymodules/python2.7/univention/uldap.py", line 260, in get
    result = self.lo.search_s(dn, ldap.SCOPE_BASE, '(objectClass=*)', attr)

with extra debugging:
  self.lo.search_s (['c'], 0, '(objectClass=*)', [])

FYI: network is 'single-valued', but due the use of '--append' it was handled as 'multi-valued'?
Comment 5 Sönke Schwardt-Krummrich univentionstaff 2018-02-06 11:59:05 CET
Florian provided a fix via pull request #1:
https://github.com/univention/univention-corporate-server/pull/1
Comment 6 Johannes Keiser univentionstaff 2018-08-27 15:05:52 CEST
from comment #5)
> Florian provided a fix via pull request #1:
> https://github.com/univention/univention-corporate-server/pull/1

The pull request was implemented and adapted to include all boolean
syntax classes


89db814 Bug #40672: rewrite boolean/chechbox search filter
c0135d4 Bug #40672: fix search filter creation for multivalue and complex syntax properties
a90aae0 Bug #40672: use generic lookup method in all handlers
0dd4a17 Bug #40672: implement lookup_filter_superordinate()
fb548a4 Bug #40672: some fixes to build the package
31f894e Bug #40672: adapt mapping to include all syntax classes that are shown as checkboxes
e45415b Bug #40672: Debian changelog entries
86dcb2d Bug #40672: YAML entries
bbc1e75 Bug #40672: Merge branch 'jkeiser/40672__udm_generates_broken_search_filter' into 4.3-1
0134e4f Bug #40672: YAML - update version(In reply to Sönke Schwardt-Krummrich 

Successful build
Package: univention-directory-manager-modules
Version: 13.0.21-33A~4.3.0.201808271442

Successful build
Package: univention-management-console-module-udm
Version: 8.0.5-12A~4.3.0.201808271444
Comment 7 Felix Botner univentionstaff 2018-08-28 10:37:56 CEST
i think (not sure 100%) this breaks all jenkins test 

[master090] 2018-08-27T23:16:54.773981	Failed adding the DNS host record master090 (10.210.110.135) to zone autotest090.local.nCommand failed with 1:nTraceback (most recent call last):
[master090] 2018-08-27T23:16:54.773981	File "/usr/share/univention-admin-tools/univention-dnsedit", line 460, in <module>
[master090] 2018-08-27T23:16:54.773981	main()
[master090] 2018-08-27T23:16:54.773981	File "/usr/share/univention-admin-tools/univention-dnsedit", line 411, in main
[master090] 2018-08-27T23:16:54.773981	zone = lookup_zone(zone_name)
[master090] 2018-08-27T23:16:54.773981	File "/usr/share/univention-admin-tools/univention-dnsedit", line 168, in lookup_zone
[master090] 2018-08-27T23:16:54.773981	scope='domain', base=position.getDomain(), unique=True)
[master090] 2018-08-27T23:16:54.773981	File "/usr/lib/pymodules/python2.7/univention/admin/handlers/__init__.py", line 1563, in lookup
[master090] 2018-08-27T23:16:54.774682	filter_s = cls.lookup_filter(filter_s, lo)
[master090] 2018-08-27T23:16:54.774682	File "/usr/lib/pymodules/python2.7/univention/admin/handlers/__init__.py", line 1589, in lookup_filter
[master090] 2018-08-27T23:16:54.774682	filter_p.append_unmapped_filter_string(filter_s, cls.rewrite_filter, univention.admin.modules.get(cls.module).mapping)
[master090] 2018-08-27T23:16:54.774682	AttributeError: 'NoneType' object has no attribute 'mapping'
[master090] 2018-08-27T23:16:54.774682	__JOINERR__:FAILED: /usr/lib/univention-install/05univention-bind.inst
[master090] 2018-08-27T23:16:54.774682	__JOINERR__:FAILED: /usr/lib/univention-install/05univention-bind.inst

we have to fix this asap or revert,
Comment 8 Johannes Keiser univentionstaff 2018-08-28 14:14:17 CEST
I reverted the changes for now and will implement the fix for Comment #7
after the next release.

959c98f Revert "Bug #40672: adapt mapping to include all syntax classes that are shown"
44feeb2 Revert "Bug #40672: some fixes to build the package"
c828aa4 Revert "Bug #40672: implement lookup_filter_superordinate()"
55786bf Revert "Bug #40672: use generic lookup method in all handlers"
8e1d73a Revert "Bug #40672: fix search filter creation for multivalue and complex syntax properties"
36f2736 Revert "Bug #40672: rewrite boolean/chechbox search filter"
17b9a0f Bug #40672: Debian changelog entries
fe97183 Bug #40672: YAML - remove entries
0d5ac17 Bug #40672: Merge branch 'jkeiser/40672__udm_generates_broken_search_filter' into 4.3-1
ef5d5aa Bug #40672: YAML - update version
Comment 9 Johannes Keiser univentionstaff 2018-08-28 16:27:23 CEST
99f81d3 Bug #40672: YAML
I added this bugnumber to the YAMl file again for the errata validation.

If the revert is OK please update the YAML file and reopen this bug
Comment 10 Dirk Wiesenthal univentionstaff 2018-08-28 16:39:30 CEST
Revert: OK
Comment 11 Johannes Keiser univentionstaff 2018-08-30 09:46:30 CEST
f486253 Bug #40672: rewrite boolean/chechbox search filter
31f0d7a Bug #40672: fix search filter creation for multivalue and complex syntax properties
24eee82 Bug #40672: use generic lookup method in all handlers
f81612b Bug #40672: implement lookup_filter_superordinate()
b5e5a6b Bug #40672: some fixes to build the package
85b28e3 Bug #40672: adapt mapping to include all syntax classes that are shown as checkboxes
a09aede Bug #40672: fix lookup call before modules.update()
f898b54 Bug #40672: Debian changelog entries
409c274 Bug #40672: YAML - add entries
3c087e0 Bug #40672: Merge branch 'jkeiser/40672__udm_generates_broken_search_filter' into 4.3-1
069f14b Bug #40672: YAML - update version

Successful build
Package: univention-management-console-module-udm
Version: 8.0.5-14A~4.3.0.201808300941

Successful build
Package: univention-directory-manager-modules
Version: 13.0.21-35A~4.3.0.201808300938
Comment 12 Stefan Gohmann univentionstaff 2018-09-18 07:53:56 CEST
The target milestone is 4.3-1-errata. I think it should be 4.3-2-errata and it shouldn't be released as erratum for 4.3-1.
Comment 13 Stefan Gohmann univentionstaff 2018-09-18 07:55:13 CEST
(In reply to Stefan Gohmann from comment #12)
> The target milestone is 4.3-1-errata. I think it should be 4.3-2-errata and
> it shouldn't be released as erratum for 4.3-1.

Please check
Comment 14 Johannes Keiser univentionstaff 2018-09-20 09:59:45 CEST
Resolved:

The PR request also touched Bug #32167
The PR request has been extended as mentioned in Comment  9 of Bug #32167 



2b4be8adfd Bug #40672: rewrite boolean/chechbox search filter
b18bcff310 Bug #40672: fix search filter creation for multivalue and complex syntax properties
6548faf211 Bug #40672: use generic lookup method in all handlers
5dd2b968b0 Bug #40672: implement lookup_filter_superordinate()
0f7b641d97 Bug #40672: some fixes to build the package
1587325369 Bug #40672: adapt mapping to include all syntax classes that are shown as checkboxes
e3d9551f18 Bug #40672: fix lookup call before modules.update()
27cefa845f Bug #40672: Debian changelog entries
59446dfb6c Bug #40672: YAML - add entries
7e02523c12 Bug #40672: Merge branch 'jkeiser/4.3-2/40672/udm_generates_broken_search_filter' into 4.3-2
4014470ed9 Bug #40672: YAML - update versions


Successful build
Package: univention-directory-manager-modules
Version: 13.0.23-5A~4.3.0.201809200950

Successful build
Package: univention-management-console-module-udm
Version: 8.0.5-23A~4.3.0.201809200953
Comment 15 Dirk Wiesenthal univentionstaff 2018-09-25 22:09:21 CEST
Ok, I checked many of the cases in CLI and UMC-UDM and did a code review of everything. Looks better now.
Comment 17 Dirk Wiesenthal univentionstaff 2018-10-04 10:04:04 CEST
*** Bug 32167 has been marked as a duplicate of this bug. ***
Comment 18 Johannes Keiser univentionstaff 2018-10-04 12:38:03 CEST
*** Bug 47364 has been marked as a duplicate of this bug. ***