The branch, master has been updated
via 47ff4223204 python:gp_cert_auto_enrol: fix GUID stringification
via 6107656ebc8 samba-tool gpo: better entities check copes with new
lines
via 65751f2562f samba-tool gpo backup fix --generalize
via 6b619b568f6 pytest: samba-tool gpo: fix
has_difference(sortlines=True)
via e87e20c04d9 python:netcmd:gpo: fix crash when updating an MOTD GPO
via 969cb41e062 pytest: check we can set GPO more than once
via d2b246fc1f4 pytest: samba-tool gpo: close opened files
via 9d3a0ffa8a5 samba-tool gpo: close opened files
from 20129d16dc3 python:ntacls: pull allow list out of loop
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 47ff42232048c008a7b361a948e5ac79311b5458
Author: Douglas Bagnall <[email protected]>
Date: Mon Mar 24 22:26:12 2025 +0000
python:gp_cert_auto_enrol: fix GUID stringification
We were using some broken ad-hoc unpacking to do what the ndr
unpacker does perfectly well.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15839
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
Autobuild-User(master): Douglas Bagnall <[email protected]>
Autobuild-Date(master): Tue Mar 25 05:21:49 UTC 2025 on atb-devel-224
commit 6107656ebc8d092b2c1907940b2486ab0265aad9
Author: Douglas Bagnall <[email protected]>
Date: Fri Mar 14 17:45:18 2025 +1300
samba-tool gpo: better entities check copes with new lines
Per https://www.w3.org/TR/xml/#sec-entity-decl (and MS references)
there is always some whitespace between '<!ENTITY' and the name, and
between the name and whatever is next. Also, it is valid XML to have
newlines inside entity declarations, like this:
<!ENTITY
bubble
"*S-1-5-113"
>
We used to create such files, so we should allow them.
There is a kind of entity that has '%' before the name, and there are
non-ascii names, which we continue not to support.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15829
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
commit 65751f2562f98bd7fd0734dc00784e6395d76322
Author: Douglas Bagnall <[email protected]>
Date: Fri Mar 14 21:55:29 2025 +1300
samba-tool gpo backup fix --generalize
This was broken with commit ce56d336f234febfd4cb3da11dd584842c24ce1d
but we didn't notice because the test was already broken.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15829
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
commit 6b619b568f6661d3a5f0701cdfaf1e1e4943ff6f
Author: Douglas Bagnall <[email protected]>
Date: Fri Mar 14 19:52:57 2025 +1300
pytest: samba-tool gpo: fix has_difference(sortlines=True)
We had
file1 = open(path1).readlines()
file1.sort()
file2 = open(path1).readlines()
file2.sort()
which is opening path1 in both cases.
This meant we were testing nothing because the assertions are all that
the files are the same -- though the only affected check is one in
test_backup_restore_generalize().
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15829
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
commit e87e20c04d90292e3a5caac8ea3105b16f948ed3
Author: Andreas Hasenack <[email protected]>
Date: Tue Feb 18 12:43:46 2025 -0300
python:netcmd:gpo: fix crash when updating an MOTD GPO
When the policy exists already, there is no exception and the code
tries to use the "data" variable, but it doesn't exist because it was
only defined in the exception handling.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15774
Signed-off-by: Andreas Hasenack <[email protected]>
Reviewed-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
commit 969cb41e06247949c3992cab25e824795204e31e
Author: Douglas Bagnall <[email protected]>
Date: Fri Mar 14 18:22:53 2025 +1300
pytest: check we can set GPO more than once
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15774
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
commit d2b246fc1f4a092af3e0e3ca6ca45d0259dec211
Author: Douglas Bagnall <[email protected]>
Date: Fri Mar 14 19:48:54 2025 +1300
pytest: samba-tool gpo: close opened files
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
commit 9d3a0ffa8a5a5a9438b5b7686d55985c04b1b566
Author: Douglas Bagnall <[email protected]>
Date: Fri Mar 14 19:47:53 2025 +1300
samba-tool gpo: close opened files
It is almost certain that we are not going to run out of files, as
they get garbage collected anyway, but in some circumstances these can
fill your screen with "ResourceWarning: unclosed file" messages, which
hides the real messages.
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
python/samba/gp/gp_cert_auto_enroll_ext.py | 13 ++----
python/samba/netcmd/gpo.py | 36 ++++++++------
python/samba/tests/gpo.py | 6 ++-
python/samba/tests/samba_tool/gpo.py | 75 ++++++++++++++++++++++++------
4 files changed, 91 insertions(+), 39 deletions(-)
Changeset truncated at 500 lines:
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py
b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 9b743cb7f9b..877659b043e 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -19,6 +19,9 @@ import operator
import requests
from samba.gp.gpclass import gp_pol_ext, gp_applier, GPOSTATE
from samba import Ldb
+from samba.dcerpc import misc
+from samba.ndr import ndr_unpack
+
from ldb import SCOPE_SUBTREE, SCOPE_BASE
from samba.auth import system_session
from samba.gp.gpclass import get_dc_hostname
@@ -52,14 +55,6 @@ global_trust_dirs = ['/etc/pki/trust/anchors', #
SUSE
'/etc/pki/ca-trust/source/anchors', # RHEL/Fedora
'/usr/local/share/ca-certificates'] # Debian/Ubuntu
-def octet_string_to_objectGUID(data):
- """Convert an octet string to an objectGUID."""
- return '%s-%s-%s-%s-%s' % ('%02x' % struct.unpack('<L', data[0:4])[0],
- '%02x' % struct.unpack('<H', data[4:6])[0],
- '%02x' % struct.unpack('<H', data[6:8])[0],
- '%02x' % struct.unpack('>H', data[8:10])[0],
- '%02x%02x' % struct.unpack('>HL', data[10:]))
-
def group_and_sort_end_point_information(end_point_information):
"""Group and Sort End Point Information.
@@ -480,7 +475,7 @@ class gp_cert_auto_enroll_ext(gp_pol_ext, gp_applier):
# instance. If the values do not match, continue with the next
# group.
objectGUID = '{%s}' % \
-
octet_string_to_objectGUID(res2[0]['objectGUID'][0]).upper()
+ str(ndr_unpack(misc.GUID,
res2[0]['objectGUID'][0])).upper()
if objectGUID != e['PolicyID']:
continue
diff --git a/python/samba/netcmd/gpo.py b/python/samba/netcmd/gpo.py
index d22404dd851..ada5f142a6f 100644
--- a/python/samba/netcmd/gpo.py
+++ b/python/samba/netcmd/gpo.py
@@ -342,7 +342,8 @@ def copy_directory_remote_to_local(conn, remotedir,
localdir):
os.mkdir(l_name)
else:
data = conn.loadfile(r_name)
- open(l_name, 'wb').write(data)
+ with open(l_name, 'wb') as f:
+ f.write(data)
def copy_directory_local_to_remote(conn, localdir, remotedir,
@@ -378,8 +379,8 @@ def copy_directory_local_to_remote(conn, localdir,
remotedir,
except NTSTATUSError:
pass
- data = open(l_name, 'rb').read()
- conn.savefile(r_name, data)
+ with open(l_name, 'rb') as f:
+ conn.savefile(r_name, f.read())
class GPOCommand(Command):
@@ -1322,9 +1323,11 @@ class cmd_backup(GPOCommand):
self.outf.write('\nAttempting to generalize XML entities:\n')
entities = cmd_backup.generalize_xml_entities(self.outf, gpodir,
gpodir)
- import operator
- ents = "".join('<!ENTITY {} "{}\n">'.format(ent[1].strip('&;'),
ent[0]) \
- for ent in sorted(entities.items(),
key=operator.itemgetter(1)))
+
+ ent_list = [(v, k) for k, v in entities.items()]
+ ent_list.sort()
+ ents = "".join(f'<!ENTITY {ent.strip("&;")} "{val}">\n'
+ for ent, val in ent_list)
if ent_file:
with open(ent_file, 'w') as f:
@@ -1459,7 +1462,8 @@ class cmd_create(GPOCommand):
os.mkdir(os.path.join(gpodir, "Machine"))
os.mkdir(os.path.join(gpodir, "User"))
gpt_contents = "[General]\r\nVersion=0\r\n"
- open(os.path.join(gpodir, "GPT.INI"), "w").write(gpt_contents)
+ with open(os.path.join(gpodir, "GPT.INI"), "w") as f:
+ f.write(gpt_contents)
except Exception as e:
raise CommandError("Error Creating GPO files", e)
@@ -1650,8 +1654,8 @@ class cmd_restore(cmd_create):
entities_content = entities_file.read()
# Do a basic regex test of the entities file format
- if re.match(r'(\s*<!ENTITY\s*[a-zA-Z0-9_]+\s*.*?>)+\s*\Z',
- entities_content, flags=re.MULTILINE) is None:
+ if re.match(r'(\s*<!ENTITY\s+[a-zA-Z0-9_]+\s+.*?>)+\s*\Z',
+ entities_content, flags=re.MULTILINE|re.DOTALL) is
None:
raise CommandError("Entities file does not appear to "
"conform to format\n"
'e.g. <!ENTITY entity "value">')
@@ -3087,7 +3091,8 @@ samba-tool gpo manage files add
{31B2F340-016D-11D2-945F-00C04FB984F9} ./source.
out = BytesIO()
xml_data.write(out, encoding='UTF-8', xml_declaration=True)
out.seek(0)
- source_data = open(source, 'rb').read()
+ with open(source, 'rb') as f:
+ source_data = f.read()
sysvol_source = '\\'.join([vgp_dir, os.path.basename(source)])
try:
create_directory_hier(conn, vgp_dir)
@@ -3555,7 +3560,8 @@ samba-tool gpo manage scripts startup add
{31B2F340-016D-11D2-945F-00C04FB984F9}
else:
raise
- script_data = open(script, 'rb').read()
+ with open(script, 'rb') as f:
+ script_data = f.read()
listelement = ET.SubElement(data, 'listelement')
script_elm = ET.SubElement(listelement, 'script')
script_elm.text = os.path.basename(script)
@@ -3807,7 +3813,9 @@ samba-tool gpo manage motd set
{31B2F340-016D-11D2-945F-00C04FB984F9} "Message f
return
try:
- xml_data = ET.fromstring(conn.loadfile(vgp_xml))
+ xml_data = ET.ElementTree(ET.fromstring(conn.loadfile(vgp_xml)))
+ policysetting = xml_data.getroot().find('policysetting')
+ data = policysetting.find('data')
except NTSTATUSError as e:
if e.args[0] in [NT_STATUS_OBJECT_NAME_INVALID,
NT_STATUS_OBJECT_NAME_NOT_FOUND,
@@ -3833,7 +3841,9 @@ samba-tool gpo manage motd set
{31B2F340-016D-11D2-945F-00C04FB984F9} "Message f
else:
raise
- text = ET.SubElement(data, 'text')
+ text = data.find('text')
+ if text is None:
+ text = ET.SubElement(data, 'text')
text.text = value
out = BytesIO()
diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index 9177eef5afa..2e4696cd926 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -53,7 +53,9 @@ from samba.gp.gp_centrify_crontab_ext import
gp_centrify_crontab_ext, \
from samba.gp.gp_drive_maps_ext import gp_drive_maps_user_ext
from samba.common import get_bytes
from samba.dcerpc import preg
-from samba.ndr import ndr_pack
+from samba.ndr import ndr_pack, ndr_unpack
+from samba.dcerpc import misc
+
import codecs
from shutil import copyfile
import xml.etree.ElementTree as etree
@@ -7654,7 +7656,7 @@ class GPOTests(tests.TestCase):
_ldb.SCOPE_BASE, '(objectClass=*)', ['objectGUID'])
self.assertTrue(len(res2) == 1, 'objectGUID not found')
objectGUID = b'{%s}' % \
-
cae.octet_string_to_objectGUID(res2[0]['objectGUID'][0]).upper().encode()
+ str(ndr_unpack(misc.GUID,
res2[0]['objectGUID'][0])).upper().encode()
parser = GPPolParser()
parser.load_xml(etree.fromstring(advanced_enroll_reg_pol.strip() %
(objectGUID, objectGUID, objectGUID, objectGUID)))
diff --git a/python/samba/tests/samba_tool/gpo.py
b/python/samba/tests/samba_tool/gpo.py
index 851c70efecf..3f1111f8002 100644
--- a/python/samba/tests/samba_tool/gpo.py
+++ b/python/samba/tests/samba_tool/gpo.py
@@ -141,21 +141,27 @@ source_path =
os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../
provision_path = os.path.join(source_path, "source4/selftest/provisions/")
def has_difference(path1, path2, binary=True, xml=True, sortlines=False):
- """Use this function to determine if the GPO backup differs from another.
+ """Use this function to determine if the GPO backup differs from
+ another. It can compare pairs of files or pairs of directories.
xml=True checks whether any xml files are equal
binary=True checks whether any .SAMBABACKUP files are equal
+ sortlines=True ignore order of lines in comparison of single
+ files.
+
+ returns None if there is no difference between the paths,
+ otherwise *something*.
"""
if os.path.isfile(path1):
+ with open(path1) as f1, open(path2) as f2:
+ lines1 = f1.readlines()
+ lines2 = f2.readlines()
+
if sortlines:
- file1 = open(path1).readlines()
- file1.sort()
- file2 = open(path1).readlines()
- file2.sort()
- if file1 != file2:
- return path1
-
- elif open(path1).read() != open(path2).read():
+ lines1.sort()
+ lines2.sort()
+
+ if lines1 != lines2:
return path1
return None
@@ -184,8 +190,9 @@ def has_difference(path1, path2, binary=True, xml=True,
sortlines=False):
else:
if (l_name.endswith('.xml') and xml or
l_name.endswith('.SAMBABACKUP') and binary):
- if open(l_name, "rb").read() != open(r_name, "rb").read():
- return l_name
+ with open(l_name, "rb") as f1, open(r_name, "rb") as f2:
+ if f1.read() != f2.read():
+ return l_name
return None
@@ -711,7 +718,8 @@ class GpoCmdTestCase(SambaToolCmdTest):
self.assertTrue(os.path.exists(reg_pol),
'The Registry.pol does not exist')
- reg_data = ndr_unpack(preg.file, open(reg_pol, 'rb').read())
+ with open(reg_pol, 'rb') as f:
+ reg_data = ndr_unpack(preg.file, f.read())
ret = any([get_string(e.valuename) == policy and e.data == 1
for e in reg_data.entries])
self.assertTrue(ret, 'The sudoers entry was not added')
@@ -729,8 +737,8 @@ class GpoCmdTestCase(SambaToolCmdTest):
'Failed to unset apply group policies')
after_vers = gpt_ini_version(self.gpo_guid)
self.assertGreater(after_vers, before_vers, 'GPT.INI was not updated')
-
- reg_data = ndr_unpack(preg.file, open(reg_pol, 'rb').read())
+ with open(reg_pol, 'rb') as f:
+ reg_data = ndr_unpack(preg.file, f.read())
ret = not any([get_string(e.valuename) == policy and e.data == 1
for e in reg_data.entries])
self.assertTrue(ret, 'The sudoers entry was not removed')
@@ -804,7 +812,8 @@ class GpoCmdTestCase(SambaToolCmdTest):
os.environ["PASSWORD"]))
self.assertCmdSuccess(result, out, err,
'Failed to unset MaxTicketAge')
- inf_pol_contents = open(inf_pol, 'r').read()
+ with open(inf_pol, 'r') as f:
+ inf_pol_contents = f.read()
self.assertNotIn('MaxTicketAge = 10', inf_pol_contents,
'The test entry was still found!')
after_vers = gpt_ini_version(self.gpo_guid)
@@ -1572,6 +1581,42 @@ class GpoCmdTestCase(SambaToolCmdTest):
os.environ["PASSWORD"]))
self.assertNotIn(text, out, 'The test entry was still found!')
+ def test_vgp_motd_set_thrice(self):
+ url = f'ldap://{os.environ["SERVER"]}'
+ creds = f'-U{os.environ["USERNAME"]}%{os.environ["PASSWORD"]}'
+ old_version = gpt_ini_version(self.gpo_guid)
+
+ for i in range(1, 4):
+ msg = f"message {i}\n"
+ result, out, err = self.runcmd("gpo", "manage", "motd", "set",
+ "-H", url,
+ creds,
+ self.gpo_guid,
+ msg.format(i))
+
+ self.assertCmdSuccess(result, out, err, f'MOTD set {i} failed')
+ self.assertEqual(err, "", f"not expecting errors (round {i})")
+ new_version = gpt_ini_version(self.gpo_guid)
+ self.assertGreater(new_version, old_version,
+ f'GPT.INI was not updated in round {i}')
+ old_version = new_version
+
+ result, out, err = self.runcmd("gpo", "manage", "motd", "list",
+ "-H", url,
+ creds,
+ self.gpo_guid)
+
+ self.assertCmdSuccess(result, out, err, f'MOTD list {i} failed')
+ self.assertIn(msg, out)
+
+ # unset, by setting with no value
+ result, out, err = self.runcmd("gpo", "manage", "motd", "set",
+ "-H", url,
+ creds,
+ self.gpo_guid)
+ self.assertCmdSuccess(result, out, err, f'MOTD set {i} failed')
+ self.assertEqual(err, "", f"not expecting errors (round {i})")
+
def test_vgp_motd(self):
lp = LoadParm()
lp.load(os.environ['SERVERCONFFILE'])
--
Samba Shared Repository