Univention Bugzilla – Bug 49842
vfs objects are wrongly quoted in share configuration
Last modified: 2022-04-27 16:11:55 CEST
We received a Pull Request: https://github.com/univention/univention-corporate-server/pull/12 The "vfs objects" configuration of a share is wrongly quoted since Bug #44054.
I think a better patch would be to make the UDM property a multivalue: diff --git a/management/univention-directory-manager-modules/modules/univention/admin/handlers/shares/share.py b/management/univention-directory-manager-modules/modules/univention/admin/handlers/shares/share.py index 211d6a6f3f..800f25038f 100644 --- a/management/univention-directory-manager-modules/modules/univention/admin/handlers/shares/share.py +++ b/management/univention-directory-manager-modules/modules/univention/admin/handlers/shares/share.py @@ -414,6 +414,7 @@ property_descriptions = { short_description=_('VFS objects'), long_description=_('Specifies which VFS Objects to use.'), syntax=univention.admin.syntax.string, + multivalue=True, options=['samba'], ), 'sambaMSDFSRoot': univention.admin.property(
I guess the order matters (would be a case for openldap x-ordered).
I have applied both suggested patches on a running UCS system. I can confirm that the fix from the GitHub pull request fixes the problem. I could not confirm that the problem was fixed applying the patch from Florian Best posted in the comments above. Problem description: Before the fix, setting VFS_Object to catia fruit streams_xattr results in the file /etc/samba/shares.conf.d/Share containing the line vfs objects = acl_xattr "catia fruit streams_xattr" The quotation marks around the added objects lead to the following error message in /var/log/samba/log.smbd Error loading module '/usr/lib/x86_64-linux-gnu/samba/vfs/catia fruit streams_xattr.so': /usr/lib/x86_64-linux-gnu/samba/vfs/catia fruit streams_xattr.so: cannot open shared object file: No such file or directory I confirmed that manually removing the quotation marks from the configuration file restores connectivity. What I did to test the patches: A) GitHub pull request * Apply patch to /usr/lib/univention-directory-listener/system/samba-shares.py * Restart univention-directory-listener -> Result: When adding VFS_Objects, no quotation marks are added. B) Florian Best's patch * Apply patch to /usr/share/pyshared/univention/admin/handlers/shares/share.py * Remove /usr/lib/pymodules/python2.7/univention/admin/handlers/shares/share.py[co] * Restart univention-directory-manager-rest and univention-directory-listener -> Result: When adding VFS_Objects, quotation marks are still added.
Thank you for the feedback. (In reply to Julian Helfferich from comment #3) > B) Florian Best's patch > > * Apply patch to > /usr/share/pyshared/univention/admin/handlers/shares/share.py > * Remove > /usr/lib/pymodules/python2.7/univention/admin/handlers/shares/share.py[co] > * Restart univention-directory-manager-rest and univention-directory-listener > > -> Result: When adding VFS_Objects, quotation marks are still added. Another necessary step after applying the patch is "service restart univention-management-console-server" and "pkill -f univention-cli-server". Then my patch would work. But, as described in comment #2, the order of the attributes matter and therefore we cannot use my patch :-( The Patch from github will re-introduce a security vulnerability because the sanitizing of invalid characters is missing then. We need something based on the github pull request, which does additionally sanitization/validation without adding quotes.
The initial commit/bug description was: With _map_quote the vfs objects are wrongly quoted: with (wrong): vfs objects = acl_xattr "recycle crossrename" without (correct): vfs objects = acl_xattr recycle crossrename
Customer asks for patch of this bug. -> "Would be nice to have this fixed before next school-year starts." Christina already filled in ticket and customer id.
Customer affected and posted in forum. https://help.univention.com/t/samba-share-not-accesible-anymore/16315
Let's just do this and get it released soon: diff --git services/univention-samba4/samba-shares.py services/univention-samba4/samba-shares.py index 3cd34b42e2..b234eda017 100644 --- services/univention-samba4/samba-shares.py +++ services/univention-samba4/samba-shares.py @@ -218 +218 @@ def handler(dn, new, old, command): - print('vfs objects = %s' % (' '.join(_map_quote(vfs_objects)), ), file=fp) + print('vfs objects = %s' % (' '.join(_simple_quote(vfs_object) for vfs_object in vfs_objects), ), file=fp) 1. customers are waiting for it 2. it is really simple 3. we can't do the API change of comment 1 without migrating the existing entries
The quoting has been removed as it is not necessary. Important (from security perspective) is only that the data must not cause a newline to be added. This also affected the option "write list" and for special/complex values of hosts allow/deny. Other attributes are either integers, booleans, strings but not list of tokens. "valid users" and "invalid users" were adjusted/normalized by UDM so they worked for me previously. univention-samba4.yaml 97a5eea125db | Bug #49842: fix breaking VFS objects and other configuration through double quoting tokens univention-samba4 (9.0.6-13) 97a5eea125db | Bug #49842: fix breaking VFS objects and other configuration through double quoting tokens univention-samba.yaml 97a5eea125db | Bug #49842: fix breaking VFS objects and other configuration through double quoting tokens univention-samba (14.0.5-6) 97a5eea125db | Bug #49842: fix breaking VFS objects and other configuration through double quoting tokens
Verified: * Code review * Package update * Functional test * Advisory feeb987697 | Advisory wording
<https://errata.software-univention.de/#/?erratum=5.0x295> <https://errata.software-univention.de/#/?erratum=5.0x296>