From 7d0cb3b4eea548d03db7058ba1750d9cd0f863bc Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 14 Mar 2025 18:22:53 +1300 Subject: [PATCH 1/6] pytest: check we can set GPO more than once BUG: https://bugzilla.samba.org/show_bug.cgi?id=15774 Signed-off-by: Douglas Bagnall Reviewed-by: Jennifer Sutton (cherry picked from commit 969cb41e06247949c3992cab25e824795204e31e) --- python/samba/tests/samba_tool/gpo.py | 36 ++++++++++++++++++++++++++++ selftest/knownfail.d/samba-tool-gpo | 1 + 2 files changed, 37 insertions(+) create mode 100644 selftest/knownfail.d/samba-tool-gpo diff --git a/python/samba/tests/samba_tool/gpo.py b/python/samba/tests/samba_tool/gpo.py index 851c70efecf..714f8e54d8d 100644 --- a/python/samba/tests/samba_tool/gpo.py +++ b/python/samba/tests/samba_tool/gpo.py @@ -1572,6 +1572,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']) diff --git a/selftest/knownfail.d/samba-tool-gpo b/selftest/knownfail.d/samba-tool-gpo new file mode 100644 index 00000000000..e8ab4690fa8 --- /dev/null +++ b/selftest/knownfail.d/samba-tool-gpo @@ -0,0 +1 @@ +^samba.tests.samba_tool.gpo.+GpoCmdTestCase.test_vgp_motd_set_thrice -- 2.34.1 From 8ae9cb9525f9d62d27837085a850567183f3f797 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Tue, 18 Feb 2025 12:43:46 -0300 Subject: [PATCH 2/6] 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 Reviewed-by: Douglas Bagnall Reviewed-by: Jennifer Sutton (cherry picked from commit e87e20c04d90292e3a5caac8ea3105b16f948ed3) --- python/samba/netcmd/gpo.py | 8 ++++++-- selftest/knownfail.d/samba-tool-gpo | 1 - 2 files changed, 6 insertions(+), 3 deletions(-) delete mode 100644 selftest/knownfail.d/samba-tool-gpo diff --git a/python/samba/netcmd/gpo.py b/python/samba/netcmd/gpo.py index 96fce917f0f..d4f6bad67af 100644 --- a/python/samba/netcmd/gpo.py +++ b/python/samba/netcmd/gpo.py @@ -3808,7 +3808,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, @@ -3834,7 +3836,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/selftest/knownfail.d/samba-tool-gpo b/selftest/knownfail.d/samba-tool-gpo deleted file mode 100644 index e8ab4690fa8..00000000000 --- a/selftest/knownfail.d/samba-tool-gpo +++ /dev/null @@ -1 +0,0 @@ -^samba.tests.samba_tool.gpo.+GpoCmdTestCase.test_vgp_motd_set_thrice -- 2.34.1 From 3741112b102acc2bf13b85f8be498f3f131ad14e Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 14 Mar 2025 19:52:57 +1300 Subject: [PATCH 3/6] 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 Reviewed-by: Jennifer Sutton (cherry picked from commit 6b619b568f6661d3a5f0701cdfaf1e1e4943ff6f) --- python/samba/tests/samba_tool/gpo.py | 24 +++++++++++++++--------- selftest/knownfail.d/samba-tool-gpo | 2 ++ 2 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 selftest/knownfail.d/samba-tool-gpo diff --git a/python/samba/tests/samba_tool/gpo.py b/python/samba/tests/samba_tool/gpo.py index 714f8e54d8d..6df9c9fb5b6 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 diff --git a/selftest/knownfail.d/samba-tool-gpo b/selftest/knownfail.d/samba-tool-gpo new file mode 100644 index 00000000000..097495bba08 --- /dev/null +++ b/selftest/knownfail.d/samba-tool-gpo @@ -0,0 +1,2 @@ +^samba.tests.samba_tool.gpo.+GpoCmdTestCase.test_backup_restore_generalize + -- 2.34.1 From dcf4d60cf0b493d85ab7048368c6083596ddef33 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 14 Mar 2025 21:55:29 +1300 Subject: [PATCH 4/6] 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 Reviewed-by: Jennifer Sutton (cherry picked from commit 65751f2562f98bd7fd0734dc00784e6395d76322) --- python/samba/netcmd/gpo.py | 8 +++++--- selftest/knownfail.d/samba-tool-gpo | 2 -- 2 files changed, 5 insertions(+), 5 deletions(-) delete mode 100644 selftest/knownfail.d/samba-tool-gpo diff --git a/python/samba/netcmd/gpo.py b/python/samba/netcmd/gpo.py index d4f6bad67af..12fc2c6c409 100644 --- a/python/samba/netcmd/gpo.py +++ b/python/samba/netcmd/gpo.py @@ -1322,9 +1322,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(''.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'\n' + for ent, val in ent_list) if ent_file: with open(ent_file, 'w') as f: diff --git a/selftest/knownfail.d/samba-tool-gpo b/selftest/knownfail.d/samba-tool-gpo deleted file mode 100644 index 097495bba08..00000000000 --- a/selftest/knownfail.d/samba-tool-gpo +++ /dev/null @@ -1,2 +0,0 @@ -^samba.tests.samba_tool.gpo.+GpoCmdTestCase.test_backup_restore_generalize - -- 2.34.1 From 6c48a32b169d178bc33d52264e3dd453a3929c59 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 14 Mar 2025 17:45:18 +1300 Subject: [PATCH 5/6] 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 ' 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 Reviewed-by: Jennifer Sutton (cherry picked from commit 6107656ebc8d092b2c1907940b2486ab0265aad9) --- python/samba/netcmd/gpo.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/samba/netcmd/gpo.py b/python/samba/netcmd/gpo.py index 12fc2c6c409..b5d18b59e2f 100644 --- a/python/samba/netcmd/gpo.py +++ b/python/samba/netcmd/gpo.py @@ -1653,8 +1653,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*)+\s*\Z', - entities_content, flags=re.MULTILINE) is None: + if re.match(r'(\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. ') -- 2.34.1 From 89f3dc3458ef6a24f6706dd8ba035ff349fca390 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Mon, 24 Mar 2025 22:26:12 +0000 Subject: [PATCH 6/6] 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 Reviewed-by: Jennifer Sutton Autobuild-User(master): Douglas Bagnall Autobuild-Date(master): Tue Mar 25 05:21:49 UTC 2025 on atb-devel-224 (cherry picked from commit 47ff42232048c008a7b361a948e5ac79311b5458) --- python/samba/gp/gp_cert_auto_enroll_ext.py | 13 ++++--------- python/samba/tests/gpo.py | 6 ++++-- 2 files changed, 8 insertions(+), 11 deletions(-) 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('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/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))) -- 2.34.1