pytest:sid_strings: test the strings with local parsing

The reason the existing tests send the SID over the wire as SDDL for
defaultSecurityDescriptor is it is one of the few ways to force the
server to reckon with a SID-string as a SID. At least, that's the case
with Windows. In Samba we make no effort to decode the SDDL until it
comes to the time of creating an object, at which point we don't notice
the difference between bad SDDL and missing SDDL.

So here we add a set of dynamic tests that push the strings through our
SDDL parsing code. This doesn't tell us very much more, but it is very
quick and sort of confirms that the other tests are on the right track.

To run against Windows without also running the internal Samba tests,
add `SAMBA_SID_STRINGS_SKIP_LOCAL=1` to your environment variables.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

2 files changed, 40 insertions, 0 deletions

diff --git a/python/samba/tests/sid_strings.py b/python/samba/tests/sid_strings.py
index 94a94a13e44..d1cc4da69f8 100644
--- a/python/samba/tests/sid_strings.py
+++ b/python/samba/tests/sid_strings.py
@@ -47,6 +47,10 @@ late_ERR_CONSTRAINT_VIOLATION = b"a hack to allow Windows to sometimes fail late
 class SidStringBase(TestCase):
     @classmethod
     def setUpDynamicTestCases(cls):
+        if not hasattr(cls, 'skip_local'):
+            cls.skip_local = env_get_var_value('SAMBA_SID_STRINGS_SKIP_LOCAL',
+                                               allow_missing=True)
+
         if env_get_var_value('CHECK_ALL_COMBINATIONS',
                              allow_missing=True):
             for x in string.ascii_uppercase:
@@ -60,6 +64,9 @@ class SidStringBase(TestCase):
 
             cls.generate_dynamic_test('test_sid_string', name,
                                       code, expected_sid)
+            if not cls.skip_local:
+                cls.generate_dynamic_test('test_sid_string_internal', name,
+                                          code, expected_sid)
 
     @classmethod
     def setUpClass(cls):
@@ -200,6 +207,27 @@ cn: {object_name}
 
         return expected_sid.format(domain_sid=self.domain_sid)
 
+    def _test_sid_string_internal_with_args(self, code, expected_sid):
+        """We just want to test the SIDs, which Samba can't really do because
+        it doesn't parse them until creating an object using the
+        schema class, at which time it doesn't distinguish between a
+        missing value and a nonsense value.
+
+        So let's also run the test using libcli/security/sddl.c and
+        see what we *would* have done.
+        """
+        sddl = f"O:{code}"
+        domsid = security.dom_sid(self.domain_sid)
+
+        try:
+            sd = security.descriptor.from_sddl(sddl, domsid)
+        except ValueError:
+            # we don't have detail as to what went wrong
+            self.assertNotIsInstance(expected_sid, str)
+        else:
+            expected_sid = self.format_expected_sid(expected_sid)
+            self.assertEqual(expected_sid, str(sd.owner_sid))
+
 
 @DynamicTestCase
 class SidStringTests(SidStringBase):
diff --git a/selftest/knownfail.d/sid-strings b/selftest/knownfail.d/sid-strings
index 8116d25354a..aae68c55ec7 100644
--- a/selftest/knownfail.d/sid-strings
+++ b/selftest/knownfail.d/sid-strings
@@ -1,6 +1,9 @@
 ^samba.tests.sid_strings.+.SidStringTests.test_sid_string_Aa.ad_dc
 ^samba.tests.sid_strings.+.SidStringTests.test_sid_string_aA.ad_dc
 ^samba.tests.sid_strings.+.SidStringTests.test_sid_string_aa.ad_dc
+^samba.tests.sid_strings.+.SidStringTests.test_sid_string_internal_Aa.ad_dc
+^samba.tests.sid_strings.+.SidStringTests.test_sid_string_internal_aA.ad_dc
+^samba.tests.sid_strings.+.SidStringTests.test_sid_string_internal_aa.ad_dc
 ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_.S-1-1-1-1-1-1-1.ad_dc
 ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0.ad_dc
 ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0x05-32-579.ad_dc
@@ -16,4 +19,13 @@
 ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-5-32-.579.ad_dc
 ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-5-32.-579.ad_dc
 ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-99999999999999999999999999999999999999-32-11111111111.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x05-32-579.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x5-0x20-0x243.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x50000000-32-579.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x500000000-32-579.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0xABcDef123-0xABCDef-579.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-22.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-5-0x20-579.ad_dc
+^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_s-1-5-32-579.ad_dc
 ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_s-1-5-32-579.ad_dc