Bug 53828 - load_default fails with shares which have consecutive whitespaces
load_default fails with shares which have consecutive whitespaces
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Samba4
UCS 5.0
Other Linux
: P5 normal (vote)
: UCS 5.0-0-errata
Assigned To: Tobias Wenzel
Jürn Brodersen
:
Depends on: 53799
Blocks: 53464
  Show dependency treegraph
 
Reported: 2021-09-23 16:18 CEST by Jürn Brodersen
Modified: 2021-10-06 16:48 CEST (History)
4 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?: 1: Will affect a very 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.057
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jürn Brodersen univentionstaff 2021-09-23 16:18:20 CEST
+++ This bug was initially created as a clone of Bug #53799 +++

This bug was initially filed in ucsschool.
If any share-name includes consecutive whitespaces, this code fails:

from samba.param import LoadParm
lp.load_default()

Reproduce this by creating a class (which will create a share) with the following name:

DEMOSCHOOL-Ch  GK


We think this has to be fixed upstream in samba. 
In school we will write a diagnose module in 53464 and - if needed - prevent classes / workgroups with consecutive whitespaces.
Comment 1 Florian Best univentionstaff 2021-09-23 20:17:13 CEST
services/univention-samba/python/share_restrictions.py|42 col 1 error| 'urllib' imported but unused [F401] [python/flake8]
Comment 2 Florian Best univentionstaff 2021-09-23 20:19:10 CEST
The code change looks wrong.
Quoting/Encoding should be done at the place where values are inserted into a format/protocol/etc.
Comment 3 Florian Best univentionstaff 2021-09-23 20:28:33 CEST
Here is a correct fix:

diff --git services/univention-samba/python/share_restrictions.py services/univention-samba/python/share_restrictions.py
index 9ae457e39e..e2151d6afb 100644
--- services/univention-samba/python/share_restrictions.py
+++ services/univention-samba/python/share_restrictions.py
@@ -39,7 +39,6 @@ from six.moves.urllib_parse import quote
 import os
 import re
 import shlex
-import urllib
 
 # defaults
 ucr = ConfigRegistry()
@@ -174,7 +173,6 @@ class ShareConfiguration(object):
                        except IndexError:
                                continue
 
-                       share.name = quote(share.name, safe='')
                        if cfg.has_option(share.name, Restrictions.INVALID_USERS):
                                share.invalid_users = shlex.split(cfg.get(share.name, Restrictions.INVALID_USERS))
                        if cfg.has_option(share.name, Restrictions.HOSTS_DENY):
@@ -353,7 +351,7 @@ class ShareConfiguration(object):
                        # write share conf only if we have ucr settings
                        if not share.ucr:
                                continue
-                       share_filename = os.path.join(ShareConfiguration.SHARES_DIR, share.name + ShareConfiguration.POSTFIX)
+                       share_filename = os.path.join(ShareConfiguration.SHARES_DIR, quote(share.name, safe='') + ShareConfiguration.POSTFIX)
                        with open(share_filename, "w") as fd:
                                fd.write("[" + share.name + "]\n")
                                for option in share:
Comment 4 Tobias Wenzel univentionstaff 2021-09-24 11:12:53 CEST
Thanks for the comment!

new fix was built with

Package: univention-samba
Version: 14.0.4-6A~5.0.0.202109241043
Branch: ucs_5.0-0
Scope: errata5.0-0
Comment 5 Jürn Brodersen univentionstaff 2021-10-06 15:17:33 CEST
Jenkins -> OK
yaml -> OK

As discussed, this fixes parsing samba share settings using python. It does not fix accessing shares with consecutive whitespaces, see bug 53880.