Bug 53799 - 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 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-8-errata
Assigned To: Tobias Wenzel
Jürn Brodersen
:
Depends on:
Blocks: 53464 53828
  Show dependency treegraph
 
Reported: 2021-09-16 10:59 CEST by Tobias Wenzel
Modified: 2021-10-06 17:05 CEST (History)
3 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 Tobias Wenzel univentionstaff 2021-09-16 10:59:22 CEST
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 Tobias Wenzel univentionstaff 2021-09-23 16:50:59 CEST
We managed to find a very simple solution:

In Bug 38688 this was already fixed for shares, which are created. The problem in school exams arose, because the UCR-V ... was set, which triggered the samba shares ucr modules - which did not do this.

so do simply do

share.name = urllib.quote(share.name, safe='')

fix is already pushed to 4.4/5.0 (Bug 53828)

With the new version, nothing else has to be done.


Package: univention-samba
Version: 13.0.0-13A~4.4.0.202109231648
Branch: ucs_4.4-0
Scope: errata4.4-8

Package: univention-samba
Version: 14.0.4-5A~5.0.0.202109231647
Branch: ucs_5.0-0
Scope: errata5.0-0
Comment 2 Jürn Brodersen univentionstaff 2021-09-23 17:16:54 CEST
What I tested:

Starting an exam with a class that has consecutive spaces in its name -> config filename is escaped (E.g DEMOSCHOOL-c-two%20%20space.local.config.conf) -> No more error when starting the exam -> OK

samba-tool works on the exam users share and returns the right permissions -> OK
E.g
"""
samba-tool ntacl get --as-sddl  /home/DEMOSCHOOL/schueler/exam-homes/exam-noch.einer.20210923-093622/
""" ->
O:S-1-5-21-1509474621-3342046244-139893009-5320G:S-1-5-21-1509474621-3342046244-139893009-11173D:PAI(D;OICI;WOWD;;;S-1-5-21-1509474621-3342046244-139893009-5320)(A;;0x001201ff;;;S-1-5-21-1509474621-3342046244-139893009-5320)(A;;;;;S-1-5-21-1509474621-3342046244-139893009-11173)(A;;;;;WD)(A;OICI;0x001301bf;;;S-1-3-4)(A;OICI;0x001301bf;;;S-1-5-21-1509474621-3342046244-139893009-5320)

yaml -> OK

Wating for jenkins
Comment 3 Florian Best univentionstaff 2021-09-24 08:58:51 CEST
(In reply to Florian Best from Bug #53828 comment #1 #2 and #3)
> services/univention-samba/python/share_restrictions.py|42 col 1 error|
> 'urllib' imported but unused [F401] [python/flake8]

> The code change looks wrong.
> Quoting/Encoding should be done at the place where values are inserted into
> a format/protocol/etc.

> 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:13:31 CEST
Thanks for the comment!

The new fix was built with

Package: univention-samba
Version: 13.0.0-14A~4.4.0.202109241105
Branch: ucs_4.4-0
Scope: errata4.4-8
Comment 5 Jürn Brodersen univentionstaff 2021-10-06 15:16:11 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.