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

Reply via email to