Attachment 18620 Details for Bug 15839 – patch for 4.21 and 4.22 for bugs 15839, 15829, and 15774

[patch] patch for 4.21 and 4.22 for bugs 15839, 15829, and 15774

gpo-bugs-4-22.patch (text/plain), 16.09 KB, created by Douglas Bagnall on 2025-03-25 07:26:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Douglas Bagnall

Created: 2025-03-25 07:26:39 UTC

Size: 16.09 KB

patch

obsolete

>From 7d0cb3b4eea548d03db7058ba1750d9cd0f863bc Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>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 <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
>(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 <andreas.hasenack@canonical.com>
>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 <andreas.hasenack@canonical.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
>(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 <douglas.bagnall@catalyst.net.nz>
>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 <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
>(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 <douglas.bagnall@catalyst.net.nz>
>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 <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
>(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('<!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:
>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 <douglas.bagnall@catalyst.net.nz>
>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 '<!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 <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
>(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*<!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">')
>-- 
>2.34.1
>
>
>From 89f3dc3458ef6a24f6706dd8ba035ff349fca390 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>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 <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
>
>Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
>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('<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/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
>

Flags:

jsutton: review+

Actions: View

Attachments on bug 15839: 18618 | 18619 | 18620