Attachment 18244 Details for Bug 15574 – patch for Samba 4.20

[patch] patch for Samba 4.20

bug15574-v4-20.patch (text/plain), 39.34 KB, created by Jo Sutton on 2024-02-08 18:18:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jo Sutton

Created: 2024-02-08 18:18:03 UTC

Size: 39.34 KB

patch

obsolete

>From f39a746e64daa0876ca5948784f181d39edf37a8 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Jan 2024 09:26:51 +1300
>Subject: [PATCH 01/19] perftest:ndr_pack: rename SD tests with object ACEs
>
>We are looking at an optimisation for non-object ACEs, which
>are more common, but these tests are overwhelmed by object
>(OA) ACEs.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit d5371f6bcd2fe991d08fcf2006ce62e6a7449ae9)
>---
> source4/dsdb/tests/python/ndr_pack_performance.py | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/ndr_pack_performance.py b/source4/dsdb/tests/python/ndr_pack_performance.py
>index 5defacf19ec9..e7586028e90a 100644
>--- a/source4/dsdb/tests/python/ndr_pack_performance.py
>+++ b/source4/dsdb/tests/python/ndr_pack_performance.py
>@@ -171,27 +171,27 @@ class UserTests(samba.tests.TestCase):
> 
>         self.assertEqual(blob, blob2)
> 
>-    def test_pack_big_sd(self):
>+    def test_pack_big_sd_with_object_aces(self):
>         unpacked = self.get_desc(BIG_SD_SDDL)
>         self._test_pack(unpacked)
> 
>-    def test_unpack_big_sd(self):
>+    def test_unpack_big_sd_with_object_aces(self):
>         blob = self.get_blob(BIG_SD_SDDL)
>         self._test_unpack(blob)
> 
>-    def test_pack_unpack_big_sd(self):
>+    def test_pack_unpack_big_sd_with_object_aces(self):
>         unpacked = self.get_desc(BIG_SD_SDDL)
>         self._test_pack_unpack(unpacked)
> 
>-    def test_pack_little_sd(self):
>+    def test_pack_little_sd_with_object_aces(self):
>         unpacked = self.get_desc(LITTLE_SD_SDDL)
>         self._test_pack(unpacked)
> 
>-    def test_unpack_little_sd(self):
>+    def test_unpack_little_sd_with_object_aces(self):
>         blob = self.get_blob(LITTLE_SD_SDDL)
>         self._test_unpack(blob)
> 
>-    def test_pack_unpack_little_sd(self):
>+    def test_pack_unpack_little_sd_with_object_aces(self):
>         unpacked = self.get_desc(LITTLE_SD_SDDL)
>         self._test_pack_unpack(unpacked)
> 
>-- 
>2.39.1
>
>
>From 9c81497a47f87679446bda75a3286f9f36db8073 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 1 Jan 2024 21:48:15 +1300
>Subject: [PATCH 02/19] perftest: ndr_pack_performance gets more SD types
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit e802611743a9b899c18d6eeaa0a46323b676c296)
>---
> .../dsdb/tests/python/ndr_pack_performance.py | 41 +++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
>diff --git a/source4/dsdb/tests/python/ndr_pack_performance.py b/source4/dsdb/tests/python/ndr_pack_performance.py
>index e7586028e90a..0ff03a27f21b 100644
>--- a/source4/dsdb/tests/python/ndr_pack_performance.py
>+++ b/source4/dsdb/tests/python/ndr_pack_performance.py
>@@ -130,6 +130,23 @@ IOID;RP;5f202010-79a5-11d0-9020-00c04fc2d4cf;bf967aba-0de6-11d0-a285-00aa0030
> 0aa003049e2;RU)(OA;CIIOID;RP;b7c69e6d-2cc7-11d2-854e-00a0c983f608;bf967a86-0d
> e6-11d0-a285-00aa003049e2;ED)""".split())
> 
>+
>+CONDITIONAL_ACE_SDDL = ('O:SYG:SYD:(XA;OICI;CR;;;WD;'
>+                        '(@USER.ad://ext/AuthenticationSilo == "siloname"))')
>+
>+NON_OBJECT_SDDL = (
>+    "O:S-1-5-21-2212615479-2695158682-2101375468-512"
>+    "G:S-1-5-21-2212615479-2695158682-2101375468-513"
>+    "D:P(A;OICI;FA;;;S-1-5-21-2212615479-2695158682-2101375468-512)"
>+    "(A;OICI;FA;;;S-1-5-21-2212615479-2695158682-2101375468-519)"
>+    "(A;OICIIO;FA;;;CO)"
>+    "(A;OICI;FA;;;S-1-5-21-2212615479-2695158682-2101375468-512)"
>+    "(A;OICI;FA;;;SY)"
>+    "(A;OICI;0x1200a9;;;AU)"
>+    "(A;OICI;0x1200a9;;;ED)")
>+
>+
>+
> # set SCALE = 100 for normal test, or 1 for testing the test.
> SCALE = 100
> 
>@@ -195,6 +212,30 @@ class UserTests(samba.tests.TestCase):
>         unpacked = self.get_desc(LITTLE_SD_SDDL)
>         self._test_pack_unpack(unpacked)
> 
>+    def test_pack_conditional_ace_sd(self):
>+        unpacked = self.get_desc(CONDITIONAL_ACE_SDDL)
>+        self._test_pack(unpacked)
>+
>+    def test_unpack_conditional_ace_sd(self):
>+        blob = self.get_blob(CONDITIONAL_ACE_SDDL)
>+        self._test_unpack(blob)
>+
>+    def test_pack_unpack_conditional_ace_sd(self):
>+        unpacked = self.get_desc(CONDITIONAL_ACE_SDDL)
>+        self._test_pack_unpack(unpacked)
>+
>+    def test_pack_non_object_sd(self):
>+        unpacked = self.get_desc(NON_OBJECT_SDDL)
>+        self._test_pack(unpacked)
>+
>+    def test_unpack_non_object_sd(self):
>+        blob = self.get_blob(NON_OBJECT_SDDL)
>+        self._test_unpack(blob)
>+
>+    def test_pack_unpack_non_object_sd(self):
>+        unpacked = self.get_desc(NON_OBJECT_SDDL)
>+        self._test_pack_unpack(unpacked)
>+
>     def test_unpack_repl_sample(self):
>         blob = self.get_file_blob('testdata/replication-ndrpack-example.gz')
>         self._test_unpack(blob, cycles=20, cls=drsuapi.DsGetNCChangesCtr6)
>-- 
>2.39.1
>
>
>From e2e6225fb337d842060343895152ec7016fef608 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Jan 2024 09:43:01 +1300
>Subject: [PATCH 03/19] perftest:ndr_pack: slightly reduce python overhead
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit d25fe2447b553087f6285c80907ca5d0debcd827)
>---
> source4/dsdb/tests/python/ndr_pack_performance.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/ndr_pack_performance.py b/source4/dsdb/tests/python/ndr_pack_performance.py
>index 0ff03a27f21b..083abf6621f4 100644
>--- a/source4/dsdb/tests/python/ndr_pack_performance.py
>+++ b/source4/dsdb/tests/python/ndr_pack_performance.py
>@@ -172,16 +172,16 @@ class UserTests(samba.tests.TestCase):
>         pass
> 
>     def _test_pack(self, unpacked, cycles=10000):
>+        pack = unpacked.__ndr_pack__
>         for i in range(SCALE * cycles):
>-            ndr_pack(unpacked)
>+            pack()
> 
>     def _test_unpack(self, blob, cycles=10000, cls=security.descriptor):
>         for i in range(SCALE * cycles):
>-            ndr_unpack(cls, blob)
>+            cls().__ndr_unpack__(blob)
> 
>     def _test_pack_unpack(self, desc, cycles=5000, cls=security.descriptor):
>         blob2 = ndr_pack(desc)
>-
>         for i in range(SCALE * cycles):
>             blob = ndr_pack(desc)
>             desc = ndr_unpack(cls, blob)
>-- 
>2.39.1
>
>
>From a21361618ffcb98f93ef6e0cd4a60af2c2f93b8b Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Jan 2024 01:51:56 +1300
>Subject: [PATCH 04/19] perftest:ndr_pack_performance: remove irrelevant
> imports, options
>
>This includes removing the ANCIENT_SAMBA switch for pre-4.3, as
>nobody cares anymore and many tests would not run correctly anyway.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit ceb5389260c4469a8f03ee884325ca981c18a36a)
>---
> .../dsdb/tests/python/ndr_pack_performance.py | 55 +------------------
> 1 file changed, 3 insertions(+), 52 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/ndr_pack_performance.py b/source4/dsdb/tests/python/ndr_pack_performance.py
>index 083abf6621f4..c0c72cac99e5 100644
>--- a/source4/dsdb/tests/python/ndr_pack_performance.py
>+++ b/source4/dsdb/tests/python/ndr_pack_performance.py
>@@ -4,10 +4,7 @@ import optparse
> import sys
> sys.path.insert(0, 'bin/python')
> 
>-import os
> import samba
>-import samba.getopt as options
>-import random
> import gzip
> 
> # We try to use the test infrastructure of Samba 4.3+, but if it
>@@ -16,45 +13,13 @@ import gzip
> #
> # Don't copy this horror into ordinary tests -- it is special for
> # performance tests that want to apply to old versions.
>-try:
>-    from samba.tests.subunitrun import SubunitOptions, TestProgram
>-    ANCIENT_SAMBA = False
>-except ImportError:
>-    ANCIENT_SAMBA = True
>-    samba.ensure_external_module("testtools", "testtools")
>-    samba.ensure_external_module("subunit", "subunit/python")
>-    from subunit.run import SubunitTestRunner
>-    import unittest
>+
>+from samba.tests.subunitrun import TestProgram
> 
> from samba.ndr import ndr_pack, ndr_unpack
> from samba.dcerpc import security
> from samba.dcerpc import drsuapi
> 
>-parser = optparse.OptionParser("ndr_pack_performance.py [options] <host>")
>-sambaopts = options.SambaOptions(parser)
>-parser.add_option_group(sambaopts)
>-parser.add_option_group(options.VersionOptions(parser))
>-
>-if not ANCIENT_SAMBA:
>-    subunitopts = SubunitOptions(parser)
>-    parser.add_option_group(subunitopts)
>-
>-# use command line creds if available
>-credopts = options.CredentialsOptions(parser)
>-parser.add_option_group(credopts)
>-opts, args = parser.parse_args()
>-
>-if len(args) < 1:
>-    parser.print_usage()
>-    sys.exit(1)
>-
>-host = args[0]
>-
>-lp = sambaopts.get_loadparm()
>-creds = credopts.get_credentials(lp)
>-
>-random.seed(1)
>-
> 
> BIG_SD_SDDL = ''.join(
>     """O:S-1-5-21-3328325300-3937145445-4190589019-512G:S-1-5-2
>@@ -246,18 +211,4 @@ class UserTests(samba.tests.TestCase):
>         self._test_pack(desc, cycles=20)
> 
> 
>-if "://" not in host:
>-    if os.path.isfile(host):
>-        host = "tdb://%s" % host
>-    else:
>-        host = "ldap://%s" % host
>-
>-
>-if ANCIENT_SAMBA:
>-    runner = SubunitTestRunner()
>-    if not runner.run(unittest.TestLoader().loadTestsFromTestCase(
>-            UserTests)).wasSuccessful():
>-        sys.exit(1)
>-    sys.exit(0)
>-else:
>-    TestProgram(module=__name__, opts=subunitopts)
>+TestProgram(module=__name__)
>-- 
>2.39.1
>
>
>From 162866e22778aa699ffa667d8089cfe7fdd0eb30 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Jan 2024 01:52:39 +1300
>Subject: [PATCH 05/19] perftest:ndr_pack: use a valid dummy SID
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 2f68545087f25e5d4c7a7742d99527c7ebbd02ab)
>---
> source4/dsdb/tests/python/ndr_pack_performance.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/tests/python/ndr_pack_performance.py b/source4/dsdb/tests/python/ndr_pack_performance.py
>index c0c72cac99e5..ed1216e6392b 100644
>--- a/source4/dsdb/tests/python/ndr_pack_performance.py
>+++ b/source4/dsdb/tests/python/ndr_pack_performance.py
>@@ -126,7 +126,7 @@ class UserTests(samba.tests.TestCase):
>         return f.read()
> 
>     def get_desc(self, sddl):
>-        dummy_sid = security.dom_sid("S-2-0-0")
>+        dummy_sid = security.dom_sid("S-1-2-3")
>         return security.descriptor.from_sddl(sddl, dummy_sid)
> 
>     def get_blob(self, sddl):
>-- 
>2.39.1
>
>
>From 3cb45811ee921d1f22951e6d83c14fa4e77a681e Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Jan 2024 01:54:29 +1300
>Subject: [PATCH 06/19] perftest:ndr_pack: spin in do_nothing for a while
>
>The idea was to get a less jittery idea of the underlying noise, but
>ut is still almost instant. This I suppose is useful in indicating
>that this much of the test has very little overhead.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 93e6ea4cff2cb6bd084db27139addeea06945ea5)
>---
> source4/dsdb/tests/python/ndr_pack_performance.py | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/ndr_pack_performance.py b/source4/dsdb/tests/python/ndr_pack_performance.py
>index ed1216e6392b..45c1816df39b 100644
>--- a/source4/dsdb/tests/python/ndr_pack_performance.py
>+++ b/source4/dsdb/tests/python/ndr_pack_performance.py
>@@ -132,9 +132,10 @@ class UserTests(samba.tests.TestCase):
>     def get_blob(self, sddl):
>         return ndr_pack(self.get_desc(sddl))
> 
>-    def test_00_00_do_nothing(self):
>+    def test_00_00_do_nothing(self, cycles=10000):
>         # this gives us an idea of the overhead
>-        pass
>+        for i in range(SCALE * cycles):
>+            pass
> 
>     def _test_pack(self, unpacked, cycles=10000):
>         pack = unpacked.__ndr_pack__
>-- 
>2.39.1
>
>
>From 77ea1762e7956954e93c9e1ba8215bf3d9f17d96 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 5 Jan 2024 13:19:39 +1300
>Subject: [PATCH 07/19] perftest: ndr_pack runs in none environment
>
>This is worth changing, because having a server running in the
>background can only add noise to the results.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 5fa663766548eac2cc5932ae03d03b79ad1751b5)
>---
> selftest/perf_tests.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/selftest/perf_tests.py b/selftest/perf_tests.py
>index 2aed9deded93..cfbbf0e056e2 100644
>--- a/selftest/perf_tests.py
>+++ b/selftest/perf_tests.py
>@@ -26,8 +26,8 @@ plantestsuite_loadlist("samba4.ldap.ad_dc_performance.python(ad_dc_ntvfs)",
>                         '--workgroup=$DOMAIN',
>                         '$LOADLIST', '$LISTOPT'])
> 
>-plantestsuite_loadlist("samba4.ndr_pack_performance.python(ad_dc_ntvfs)",
>-                       "ad_dc_ntvfs",
>+plantestsuite_loadlist("samba4.ndr_pack_performance.python",
>+                       "none",
>                        [python, os.path.join(samba4srcdir,
>                                              "dsdb/tests/python/ndr_pack_performance.py"),
>                         '$SERVER', '-U"$USERNAME%$PASSWORD"',
>-- 
>2.39.1
>
>
>From 738f164569da9b506c5bf67d7213e02b3c7004fb Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 28 Dec 2023 23:07:56 +1300
>Subject: [PATCH 08/19] pidl: calculate subcontext_size only once per pull
>
>For security_ace_coda in security.idl, the sub-context size is
>involves a slightly non-trivial function call which returns a constant
>value.
>
>In all other cases, a constant expression is used, and this makes
>no difference.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 9811762775b28e16035afb2c319b55c4bf3699d3)
>---
> pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm
>index 859da0a914fe..d7386d5b2cce 100644
>--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm
>+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm
>@@ -603,7 +603,8 @@ sub ParseSubcontextPullStart($$$$$)
> 	$self->pidl("{");
> 	$self->indent;
> 	$self->pidl("struct ndr_pull *$subndr;");
>-	$self->pidl("NDR_CHECK(ndr_pull_subcontext_start($ndr, &$subndr, $l->{HEADER_SIZE}, $subcontext_size));");
>+	$self->pidl("ssize_t sub_size = $subcontext_size;");
>+	$self->pidl("NDR_CHECK(ndr_pull_subcontext_start($ndr, &$subndr, $l->{HEADER_SIZE}, sub_size));");
> 
> 	if (defined $l->{COMPRESSION}) {
> 		$subndr = $self->ParseCompressionPullStart($e, $l, $subndr, $env);
>@@ -622,7 +623,7 @@ sub ParseSubcontextPullEnd($$$$$)
> 		$self->ParseCompressionPullEnd($e, $l, $subndr, $env);
> 	}
> 
>-	$self->pidl("NDR_CHECK(ndr_pull_subcontext_end($ndr, $subndr, $l->{HEADER_SIZE}, $subcontext_size));");
>+	$self->pidl("NDR_CHECK(ndr_pull_subcontext_end($ndr, $subndr, $l->{HEADER_SIZE}, sub_size));");
> 	$self->deindent;
> 	$self->pidl("}");
> }
>-- 
>2.39.1
>
>
>From 2b551fd84f8af22df8628dce4f229851775cb43b Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 31 Dec 2023 13:03:32 +1300
>Subject: [PATCH 09/19] ndr: shift ndr_pull_security_ace to manual code
>
>This was manual until commit c73034cf7c4392f5d3505319948bc84634c20fa5
>(a few months ago).
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 1e6a876c2cc4b3b54895dde879492e756bb9b963)
>---
> librpc/idl/security.idl     |  2 +-
> librpc/ndr/ndr_sec_helper.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
>diff --git a/librpc/idl/security.idl b/librpc/idl/security.idl
>index d1552475b638..b0237d8b856a 100644
>--- a/librpc/idl/security.idl
>+++ b/librpc/idl/security.idl
>@@ -715,7 +715,7 @@ interface security
> 		[default][flag(NDR_REMAINING)] DATA_BLOB ignored;
> 	} security_ace_coda;
> 
>-	typedef [public,gensize,nosize] struct {
>+	typedef [public,gensize,nosize,nopull] struct {
> 		security_ace_type type;  /* SEC_ACE_TYPE_* */
> 		security_ace_flags flags; /* SEC_ACE_FLAG_* */
> 		[value(ndr_size_security_ace(r,ndr->flags))] uint16 size;
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index d74d31b28232..a634dfd75fc7 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -75,6 +75,36 @@ size_t ndr_size_security_ace(const struct security_ace *ace, libndr_flags flags)
> 	return ret;
> }
> 
>+_PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags_type ndr_flags, struct security_ace *r)
>+{
>+	NDR_PULL_CHECK_FLAGS(ndr, ndr_flags);
>+	if (ndr_flags & NDR_SCALARS) {
>+		NDR_CHECK(ndr_pull_align(ndr, 5));
>+		NDR_CHECK(ndr_pull_security_ace_type(ndr, NDR_SCALARS, &r->type));
>+		NDR_CHECK(ndr_pull_security_ace_flags(ndr, NDR_SCALARS, &r->flags));
>+		NDR_CHECK(ndr_pull_uint16(ndr, NDR_SCALARS, &r->size));
>+		NDR_CHECK(ndr_pull_uint32(ndr, NDR_SCALARS, &r->access_mask));
>+		NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>+		NDR_CHECK(ndr_pull_security_ace_object_ctr(ndr, NDR_SCALARS, &r->object));
>+		NDR_CHECK(ndr_pull_dom_sid(ndr, NDR_SCALARS, &r->trustee));
>+		{
>+			struct ndr_pull *_ndr_coda;
>+			ssize_t sub_size = ndr_subcontext_size_of_ace_coda(r, r->size, ndr->flags);
>+			NDR_CHECK(ndr_pull_subcontext_start(ndr, &_ndr_coda, 0, sub_size));
>+			NDR_CHECK(ndr_pull_set_switch_value(_ndr_coda, &r->coda, r->type));
>+			NDR_CHECK(ndr_pull_security_ace_coda(_ndr_coda, NDR_SCALARS|NDR_BUFFERS, &r->coda));
>+			NDR_CHECK(ndr_pull_subcontext_end(ndr, _ndr_coda, 0, sub_size));
>+		}
>+		NDR_CHECK(ndr_pull_trailer_align(ndr, 5));
>+	}
>+	if (ndr_flags & NDR_BUFFERS) {
>+		NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>+		NDR_CHECK(ndr_pull_security_ace_object_ctr(ndr, NDR_BUFFERS, &r->object));
>+	}
>+	return NDR_ERR_SUCCESS;
>+}
>+
>+
> /*
>  * An ACE coda can't be bigger than the space allowed for by
>  * ace->size, so we need to check this from the context of the ACE.
>-- 
>2.39.1
>
>
>From 6272e2e097e05d120194894937225bdbe3e31252 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 31 Dec 2023 13:06:40 +1300
>Subject: [PATCH 10/19] ndr: short-circuit ace coda if no bytes left
>
>The overwhelmingly common case is that there are no bytes left, and
>regardless of the ACE type we want to store an empty blob.
>
>We know the blob will be empty if there are no bytes, so we don't need
>to allocate a sub-ndr and tokens list and so forth.
>
>This can save almost half the time of a security descriptor pull.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit ac0c8ee01ea624e9c486251da2132710c2a43ddc)
>---
> librpc/ndr/ndr_sec_helper.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index a634dfd75fc7..6ae177f292d5 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -79,6 +79,7 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags
> {
> 	NDR_PULL_CHECK_FLAGS(ndr, ndr_flags);
> 	if (ndr_flags & NDR_SCALARS) {
>+		ssize_t sub_size;
> 		NDR_CHECK(ndr_pull_align(ndr, 5));
> 		NDR_CHECK(ndr_pull_security_ace_type(ndr, NDR_SCALARS, &r->type));
> 		NDR_CHECK(ndr_pull_security_ace_flags(ndr, NDR_SCALARS, &r->flags));
>@@ -87,9 +88,12 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags
> 		NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
> 		NDR_CHECK(ndr_pull_security_ace_object_ctr(ndr, NDR_SCALARS, &r->object));
> 		NDR_CHECK(ndr_pull_dom_sid(ndr, NDR_SCALARS, &r->trustee));
>-		{
>+		sub_size = ndr_subcontext_size_of_ace_coda(r, r->size, ndr->flags);
>+		if (sub_size == 0) {
>+			r->coda.ignored.data = NULL;
>+			r->coda.ignored.length = 0;
>+		} else {
> 			struct ndr_pull *_ndr_coda;
>-			ssize_t sub_size = ndr_subcontext_size_of_ace_coda(r, r->size, ndr->flags);
> 			NDR_CHECK(ndr_pull_subcontext_start(ndr, &_ndr_coda, 0, sub_size));
> 			NDR_CHECK(ndr_pull_set_switch_value(_ndr_coda, &r->coda, r->type));
> 			NDR_CHECK(ndr_pull_security_ace_coda(_ndr_coda, NDR_SCALARS|NDR_BUFFERS, &r->coda));
>-- 
>2.39.1
>
>
>From 612732c1d38461275ad2c1f07c438588cd5140b3 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 31 Dec 2023 17:30:47 +1300
>Subject: [PATCH 11/19] ndr: make security_ace push manual
>
>This will allow some optimisations; in this commit we just copy the
>code.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit dc08e7924c2e359afeb4b86f306868cad00189a0)
>---
> librpc/idl/security.idl     |  2 +-
> librpc/ndr/ndr_sec_helper.c | 28 ++++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
>diff --git a/librpc/idl/security.idl b/librpc/idl/security.idl
>index b0237d8b856a..8783b6781576 100644
>--- a/librpc/idl/security.idl
>+++ b/librpc/idl/security.idl
>@@ -715,7 +715,7 @@ interface security
> 		[default][flag(NDR_REMAINING)] DATA_BLOB ignored;
> 	} security_ace_coda;
> 
>-	typedef [public,gensize,nosize,nopull] struct {
>+	typedef [public,gensize,nosize,nopush,nopull] struct {
> 		security_ace_type type;  /* SEC_ACE_TYPE_* */
> 		security_ace_flags flags; /* SEC_ACE_FLAG_* */
> 		[value(ndr_size_security_ace(r,ndr->flags))] uint16 size;
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index 6ae177f292d5..b3de2f65f23a 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -108,6 +108,34 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags
> 	return NDR_ERR_SUCCESS;
> }
> 
>+_PUBLIC_ enum ndr_err_code ndr_push_security_ace(struct ndr_push *ndr, ndr_flags_type ndr_flags, const struct security_ace *r)
>+{
>+	NDR_PUSH_CHECK_FLAGS(ndr, ndr_flags);
>+	if (ndr_flags & NDR_SCALARS) {
>+		NDR_CHECK(ndr_push_align(ndr, 5));
>+		NDR_CHECK(ndr_push_security_ace_type(ndr, NDR_SCALARS, r->type));
>+		NDR_CHECK(ndr_push_security_ace_flags(ndr, NDR_SCALARS, r->flags));
>+		NDR_CHECK(ndr_push_uint16(ndr, NDR_SCALARS, ndr_size_security_ace(r, ndr->flags)));
>+		NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, r->access_mask));
>+		NDR_CHECK(ndr_push_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>+		NDR_CHECK(ndr_push_security_ace_object_ctr(ndr, NDR_SCALARS, &r->object));
>+		NDR_CHECK(ndr_push_dom_sid(ndr, NDR_SCALARS, &r->trustee));
>+		{
>+			struct ndr_push *_ndr_coda;
>+			NDR_CHECK(ndr_push_subcontext_start(ndr, &_ndr_coda, 0, ndr_subcontext_size_of_ace_coda(r, ndr_size_security_ace(r, ndr->flags), ndr->flags)));
>+			NDR_CHECK(ndr_push_set_switch_value(_ndr_coda, &r->coda, r->type));
>+			NDR_CHECK(ndr_push_security_ace_coda(_ndr_coda, NDR_SCALARS|NDR_BUFFERS, &r->coda));
>+			NDR_CHECK(ndr_push_subcontext_end(ndr, _ndr_coda, 0, ndr_subcontext_size_of_ace_coda(r, ndr_size_security_ace(r, ndr->flags), ndr->flags)));
>+		}
>+		NDR_CHECK(ndr_push_trailer_align(ndr, 5));
>+	}
>+	if (ndr_flags & NDR_BUFFERS) {
>+		NDR_CHECK(ndr_push_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>+		NDR_CHECK(ndr_push_security_ace_object_ctr(ndr, NDR_BUFFERS, &r->object));
>+	}
>+	return NDR_ERR_SUCCESS;
>+}
>+
> 
> /*
>  * An ACE coda can't be bigger than the space allowed for by
>-- 
>2.39.1
>
>
>From 47db8a4676031c2c98370d83c4231c15ca3e67f3 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 31 Dec 2023 17:39:23 +1300
>Subject: [PATCH 12/19] ndr: ACE push avoids no-op coda pushes
>
>We don't expect an ordinary ACE to have a non-empty coda, and we don't
>really want to push it if it does, but for this patch we still will.
>
>This will not change the data on the wire.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit ee1b8ae04b10306c059174a5b4b637b080fe23fd)
>---
> librpc/ndr/ndr_sec_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index b3de2f65f23a..ae4006b26632 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -120,7 +120,7 @@ _PUBLIC_ enum ndr_err_code ndr_push_security_ace(struct ndr_push *ndr, ndr_flags
> 		NDR_CHECK(ndr_push_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
> 		NDR_CHECK(ndr_push_security_ace_object_ctr(ndr, NDR_SCALARS, &r->object));
> 		NDR_CHECK(ndr_push_dom_sid(ndr, NDR_SCALARS, &r->trustee));
>-		{
>+		if (sec_ace_has_extra_blob(r->type) || r->coda.ignored.length != 0) {
> 			struct ndr_push *_ndr_coda;
> 			NDR_CHECK(ndr_push_subcontext_start(ndr, &_ndr_coda, 0, ndr_subcontext_size_of_ace_coda(r, ndr_size_security_ace(r, ndr->flags), ndr->flags)));
> 			NDR_CHECK(ndr_push_set_switch_value(_ndr_coda, &r->coda, r->type));
>-- 
>2.39.1
>
>
>From 82edb4599590c196d13cf0c0e45c7525a1a00e99 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 29 Dec 2023 15:15:48 +1300
>Subject: [PATCH 13/19] ndr: skip talloc when pulling empty DATA_BLOB
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit c2673b02a7a51761e8b6631eb0c0e7062cbbed7b)
>---
> librpc/ndr/ndr_basic.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/librpc/ndr/ndr_basic.c b/librpc/ndr/ndr_basic.c
>index fc8620f28c77..5fd15730a74c 100644
>--- a/librpc/ndr/ndr_basic.c
>+++ b/librpc/ndr/ndr_basic.c
>@@ -1453,6 +1453,12 @@ _PUBLIC_ enum ndr_err_code ndr_pull_DATA_BLOB(struct ndr_pull *ndr, ndr_flags_ty
> 	} else {
> 		NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &length));
> 	}
>+	if (length == 0) {
>+		/* skip the talloc for an empty blob */
>+		blob->data = NULL;
>+		blob->length = 0;
>+		return NDR_ERR_SUCCESS;
>+	}
> 	NDR_PULL_NEED_BYTES(ndr, length);
> 	*blob = data_blob_talloc(ndr->current_mem_ctx, ndr->data+ndr->offset, length);
> 	ndr->offset += length;
>-- 
>2.39.1
>
>
>From 4380a973df046eb1d4cf1bf8251365c3c0e15fe2 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 29 Dec 2023 15:27:08 +1300
>Subject: [PATCH 14/19] ndr: mark invalid pull ndr_flags as unlikely
>
>This might have little effect, but sometimes we see primatives like
>ndr_pull_uint32() taking a few percent of the CPU time, and this is in
>all those functions.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 4face258dee93dcd01dce71fcb7448b285ff4860)
>---
> librpc/ndr/libndr.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h
>index 70dd01e49a67..03d1aead01a6 100644
>--- a/librpc/ndr/libndr.h
>+++ b/librpc/ndr/libndr.h
>@@ -367,7 +367,7 @@ enum ndr_compression_alg {
> };
> 
> #define NDR_PULL_CHECK_FLAGS(ndr, ndr_flags) do { \
>-	if ((ndr_flags) & ~(NDR_SCALARS|NDR_BUFFERS)) { \
>+	if (unlikely((ndr_flags) & ~(NDR_SCALARS|NDR_BUFFERS))) { \
> 		return ndr_pull_error(ndr, NDR_ERR_FLAGS, "Invalid pull struct ndr_flags 0x%"PRI_NDR_FLAGS_TYPE, ndr_flags); \
> 	} \
> } while (0)
>-- 
>2.39.1
>
>
>From 15879329b300cd30af8e360216c5651fbf1576c8 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 31 Dec 2023 17:45:36 +1300
>Subject: [PATCH 15/19] ndr: do not push ACE->coda.ignored blob
>
>From 1e80221b2340de5ef5e2a17f10511bbc2c041163 (2008) until
>c73034cf7c4392f5d3505319948bc84634c20fa5 (conditional ACEs, etc, 2023)
>we had a manual ndr_pull_security_ace() that would discard trailing
>bytes, which are those bytes that we now call the coda. The ACE types
>that we handled then are those that end up with a coda.ignored data
>blob.
>
>With this we effectively restore the long-standing behaviour in the
>event that we push and pull an ACE -- though now we discard the
>ignored bytes on push rather than pull.
>
>This change is not because the trailing bytes caused any problems (as
>far as is known), but because it is much faster to not do the push.
>
>It may be that such ACEs no longer occur.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 2a60ec98409b161cfeb4b51414ba61feb26c01b9)
>---
> librpc/ndr/ndr_sec_helper.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index ae4006b26632..98663a12ad9e 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -64,7 +64,11 @@ size_t ndr_size_security_ace(const struct security_ace *ace, libndr_flags flags)
> 	} else if (ace->type == SEC_ACE_TYPE_SYSTEM_RESOURCE_ATTRIBUTE) {
> 		ret += ndr_size_security_ace_coda(&ace->coda, ace->type, flags);
> 	} else {
>-		ret += ace->coda.ignored.length;
>+		/*
>+		 * Normal ACEs have a coda.ignored blob that is always or
>+		 * almost always empty. We aren't going to push it (it is
>+		 * ignored), so we don't add that length to the size.
>+		 */
> 	}
> 	/* round up to a multiple of 4  (MS-DTYP 2.4.4.1) */
> 	ret = (ret + 3ULL) & ~3ULL;
>@@ -120,7 +124,7 @@ _PUBLIC_ enum ndr_err_code ndr_push_security_ace(struct ndr_push *ndr, ndr_flags
> 		NDR_CHECK(ndr_push_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
> 		NDR_CHECK(ndr_push_security_ace_object_ctr(ndr, NDR_SCALARS, &r->object));
> 		NDR_CHECK(ndr_push_dom_sid(ndr, NDR_SCALARS, &r->trustee));
>-		if (sec_ace_has_extra_blob(r->type) || r->coda.ignored.length != 0) {
>+		if (sec_ace_has_extra_blob(r->type)) {
> 			struct ndr_push *_ndr_coda;
> 			NDR_CHECK(ndr_push_subcontext_start(ndr, &_ndr_coda, 0, ndr_subcontext_size_of_ace_coda(r, ndr_size_security_ace(r, ndr->flags), ndr->flags)));
> 			NDR_CHECK(ndr_push_set_switch_value(_ndr_coda, &r->coda, r->type));
>-- 
>2.39.1
>
>
>From a241a9b6b84341b6c7514b8892e85e3b67a1023d Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 1 Jan 2024 10:21:33 +1300
>Subject: [PATCH 16/19] ndr: avoid object ACE pull overhead for non-object ACE
>
>When an ACE is not an object ACE, which is common, setting the switch
>value and attempting the object ACE GUID pull is just going to do
>nothing, and we know that ahead of time. By noticing that we can save
>a bit of time on a common operation.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit fce4d51eb492a6fc807c6849cd4bd65ca7714509)
>---
> librpc/ndr/ndr_sec_helper.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index 98663a12ad9e..4e721e165051 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -79,6 +79,27 @@ size_t ndr_size_security_ace(const struct security_ace *ace, libndr_flags flags)
> 	return ret;
> }
> 
>+
>+static inline enum ndr_err_code ndr_maybe_pull_security_ace_object_ctr(struct ndr_pull *ndr,
>+								       ndr_flags_type ndr_flags,
>+								       struct security_ace *r)
>+{
>+	/*
>+	 * If this is not an object ACE (as is usually common),
>+	 * ndr_pull_security_ace_object_ctr() will do nothing.
>+	 *
>+	 * By avoiding calling the function in that case, we avoid some
>+	 * tallocing and ndr token busywork.
>+	 */
>+	bool is_object = sec_ace_object(r->type);
>+	if (is_object) {
>+		NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->object, is_object));
>+		NDR_CHECK(ndr_pull_security_ace_object_ctr(ndr, ndr_flags, &r->object));
>+	}
>+	return NDR_ERR_SUCCESS;
>+}
>+
>+
> _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags_type ndr_flags, struct security_ace *r)
> {
> 	NDR_PULL_CHECK_FLAGS(ndr, ndr_flags);
>@@ -89,8 +110,7 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags
> 		NDR_CHECK(ndr_pull_security_ace_flags(ndr, NDR_SCALARS, &r->flags));
> 		NDR_CHECK(ndr_pull_uint16(ndr, NDR_SCALARS, &r->size));
> 		NDR_CHECK(ndr_pull_uint32(ndr, NDR_SCALARS, &r->access_mask));
>-		NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>-		NDR_CHECK(ndr_pull_security_ace_object_ctr(ndr, NDR_SCALARS, &r->object));
>+		NDR_CHECK(ndr_maybe_pull_security_ace_object_ctr(ndr, NDR_SCALARS, r));
> 		NDR_CHECK(ndr_pull_dom_sid(ndr, NDR_SCALARS, &r->trustee));
> 		sub_size = ndr_subcontext_size_of_ace_coda(r, r->size, ndr->flags);
> 		if (sub_size == 0) {
>@@ -106,8 +126,7 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags
> 		NDR_CHECK(ndr_pull_trailer_align(ndr, 5));
> 	}
> 	if (ndr_flags & NDR_BUFFERS) {
>-		NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>-		NDR_CHECK(ndr_pull_security_ace_object_ctr(ndr, NDR_BUFFERS, &r->object));
>+		NDR_CHECK(ndr_maybe_pull_security_ace_object_ctr(ndr, NDR_BUFFERS, r));
> 	}
> 	return NDR_ERR_SUCCESS;
> }
>-- 
>2.39.1
>
>
>From 7e9a7d57d745d14231c2523a169bcc14597f7854 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 1 Jan 2024 10:21:55 +1300
>Subject: [PATCH 17/19] ndr: avoid object ACE push overhead for non-object ACE
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit ecb5da3e49283ca3a03dea81d22db4a081e192e4)
>---
> librpc/ndr/ndr_sec_helper.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index 4e721e165051..e07ee233599e 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -131,6 +131,24 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags
> 	return NDR_ERR_SUCCESS;
> }
> 
>+
>+static inline enum ndr_err_code ndr_maybe_push_security_ace_object_ctr(struct ndr_push *ndr,
>+								       ndr_flags_type ndr_flags,
>+								       const struct security_ace *r)
>+{
>+	/*
>+	 * ndr_push_security_ace_object_ctr() does nothing (except tallocing
>+	 * and ndr_token fiddling) unless the ACE is an object ACE, which is
>+	 * usually very unlikely.
>+	 */
>+	bool is_object = sec_ace_object(r->type);
>+	if (is_object) {
>+		NDR_CHECK(ndr_push_set_switch_value(ndr, &r->object, is_object));
>+		NDR_CHECK(ndr_push_security_ace_object_ctr(ndr, ndr_flags, &r->object));
>+	}
>+	return NDR_ERR_SUCCESS;
>+}
>+
> _PUBLIC_ enum ndr_err_code ndr_push_security_ace(struct ndr_push *ndr, ndr_flags_type ndr_flags, const struct security_ace *r)
> {
> 	NDR_PUSH_CHECK_FLAGS(ndr, ndr_flags);
>@@ -140,8 +158,7 @@ _PUBLIC_ enum ndr_err_code ndr_push_security_ace(struct ndr_push *ndr, ndr_flags
> 		NDR_CHECK(ndr_push_security_ace_flags(ndr, NDR_SCALARS, r->flags));
> 		NDR_CHECK(ndr_push_uint16(ndr, NDR_SCALARS, ndr_size_security_ace(r, ndr->flags)));
> 		NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, r->access_mask));
>-		NDR_CHECK(ndr_push_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>-		NDR_CHECK(ndr_push_security_ace_object_ctr(ndr, NDR_SCALARS, &r->object));
>+		NDR_CHECK(ndr_maybe_push_security_ace_object_ctr(ndr, NDR_SCALARS, r));
> 		NDR_CHECK(ndr_push_dom_sid(ndr, NDR_SCALARS, &r->trustee));
> 		if (sec_ace_has_extra_blob(r->type)) {
> 			struct ndr_push *_ndr_coda;
>@@ -153,8 +170,7 @@ _PUBLIC_ enum ndr_err_code ndr_push_security_ace(struct ndr_push *ndr, ndr_flags
> 		NDR_CHECK(ndr_push_trailer_align(ndr, 5));
> 	}
> 	if (ndr_flags & NDR_BUFFERS) {
>-		NDR_CHECK(ndr_push_set_switch_value(ndr, &r->object, sec_ace_object(r->type)));
>-		NDR_CHECK(ndr_push_security_ace_object_ctr(ndr, NDR_BUFFERS, &r->object));
>+		NDR_CHECK(ndr_maybe_push_security_ace_object_ctr(ndr, NDR_BUFFERS, r));
> 	}
> 	return NDR_ERR_SUCCESS;
> }
>-- 
>2.39.1
>
>
>From 84e28516e44ce56115752cbf590950606da83254 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 8 Jan 2024 14:50:30 +1300
>Subject: [PATCH 18/19] ndr: ndr_push_security_ace: calculate coda size once
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit a72c198921f64f2502f543c7158762c64cb3074e)
>---
> librpc/ndr/ndr_sec_helper.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index e07ee233599e..508bcd219dda 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -162,10 +162,14 @@ _PUBLIC_ enum ndr_err_code ndr_push_security_ace(struct ndr_push *ndr, ndr_flags
> 		NDR_CHECK(ndr_push_dom_sid(ndr, NDR_SCALARS, &r->trustee));
> 		if (sec_ace_has_extra_blob(r->type)) {
> 			struct ndr_push *_ndr_coda;
>-			NDR_CHECK(ndr_push_subcontext_start(ndr, &_ndr_coda, 0, ndr_subcontext_size_of_ace_coda(r, ndr_size_security_ace(r, ndr->flags), ndr->flags)));
>+			size_t coda_size = ndr_subcontext_size_of_ace_coda(
>+				r,
>+				ndr_size_security_ace(r, ndr->flags),
>+				ndr->flags);
>+			NDR_CHECK(ndr_push_subcontext_start(ndr, &_ndr_coda, 0, coda_size));
> 			NDR_CHECK(ndr_push_set_switch_value(_ndr_coda, &r->coda, r->type));
> 			NDR_CHECK(ndr_push_security_ace_coda(_ndr_coda, NDR_SCALARS|NDR_BUFFERS, &r->coda));
>-			NDR_CHECK(ndr_push_subcontext_end(ndr, _ndr_coda, 0, ndr_subcontext_size_of_ace_coda(r, ndr_size_security_ace(r, ndr->flags), ndr->flags)));
>+			NDR_CHECK(ndr_push_subcontext_end(ndr, _ndr_coda, 0, coda_size));
> 		}
> 		NDR_CHECK(ndr_push_trailer_align(ndr, 5));
> 	}
>-- 
>2.39.1
>
>
>From 15571366c889cc2a7f24f938f224f3c1b79b0cc7 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 8 Jan 2024 15:05:35 +1300
>Subject: [PATCH 19/19] ndr: ignore trailing bytes in ndr_pull_security_ace()
>
>This returns the behaviour with ordinary ACEs to where it was with 4.19.
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15574
>(cherry picked from commit 0c1f421c107be3156b3f1db75aced24a1bca3d2f)
>---
> librpc/ndr/ndr_sec_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c
>index 508bcd219dda..f870a17aafc6 100644
>--- a/librpc/ndr/ndr_sec_helper.c
>+++ b/librpc/ndr/ndr_sec_helper.c
>@@ -113,7 +113,7 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags
> 		NDR_CHECK(ndr_maybe_pull_security_ace_object_ctr(ndr, NDR_SCALARS, r));
> 		NDR_CHECK(ndr_pull_dom_sid(ndr, NDR_SCALARS, &r->trustee));
> 		sub_size = ndr_subcontext_size_of_ace_coda(r, r->size, ndr->flags);
>-		if (sub_size == 0) {
>+		if (!sec_ace_has_extra_blob(r->type) || sub_size == 0) {
> 			r->coda.ignored.data = NULL;
> 			r->coda.ignored.length = 0;
> 		} else {
>-- 
>2.39.1
>

Flags:

dbagnall: review+

Actions: View

Attachments on bug 15574: 18244