The branch, master has been updated via 4486d686f5c gp: Add site-dn fallback when rpc call fails via c80affe0f19 Add a WHATSNEW entry indicating libgpo py deprecation via ee04bafc25c gpo: Group Policy tests require a s3 loadparam via ac4726106c6 gpupdate: Deprecate libgpo.get_gpo_list via a8bad5d5b85 gpupdate: Implement get_gpo_list in python via 848bce061af libcli/security/tests: test strings for windows and samba SDDL tests via d36bab52d0f s3/utils: when encoding ace string use "FA", "FR", "FW", "FX" string rights via 0a153c1d58d s3/utils: value for ace_flags value "FA" is incorrect via 9fc6062bd3b pytest:sddl: show the correct handling of the "FA" SDDL flag via 334afc7157e pytest:sddl Samba had the wrong value for FA, now fix the tests via c0d477738ea libcli:security:sddl: accept only 8-4-4-4-12 GUIDs via 4c1d9e92e11 pytest:large_ldap: use a valid ACE via 2e90ba7ec6f pytest:sddl: test we only accept normal GUIDs via 46793d384e9 libcli:security:sddl_decode_access allows spaces between flags via ec2d2f8ea83 pytest:sddl: tests around spaces in access flags and SIDs via 0528da54b8c pytest:sddl debugging: should_fail test says how it failed via e7445aa677f libcli:security: sddl_decode_ace: don't allow junk after SID via c67f2292cba libcli/security: sddl_decode_access rejects trailing rubbish via faf1b80a900 libcli:security: sddl_map_flags rejects trailing nonsense via 96fe7ebe3f3 s3:torture: sid2unixid2: DEBUG blames the right function via 396d2805465 s3:torture:LOCAL-IDMAP-TDB-COMMON: avoid talloc stacktrace via 1d9712283bf pytest:sddl: add tests for long DACLs, differing flag interpretations via de6d4700630 pytest:sddl: let hex numbers differ in case (0xa == 0xA) via 030ce22f525 pytest:sddl: helpers to exchange SDDL strings with Windows testprogram via d9e1fa34563 libcli/security: SDDL parse tests to run on Windows via 97353c00917 pytest:sddl: SDDL strings where Windows behaviour differs via fb588d768be pytest:sddl: Add negative tests of unparseable strings via a2009b56b51 pytest:sddl: allow tests to make negative assertions via ba6f4013401 pytest:sddl: split each string into it's own test via eac400b4dbe pytest:sddl: tweak some test strings via 4652d2766a7 pytest/sddl: split tests into canonical and non-canonical via 1107952c2b9 pytest/sddl: remove unused imports via ec85c1fdff5 pytest/sddl: rework to allow multiple lists, no early stop via 4a24c520569 pytest/sddl: assert sddl string equality via f87f63997ff pytest/sddl: remove duplicate test case via 298821a8edb pytest/sddl: give test more of a name via 35bf8ff4f46 pytests/sddl: clarify boundaries between sddl cases via 67500da1486 pytest:posixacl: expect canonical ACE flag format via c08959d1358 pytest:samba-tool ntacl: expect canonical ACE flag format via a655e7e4962 py:provision: use canonical representation of ACE flags via e521b0a26a9 pytest:ntacls: adapt for canonical flag format via 82b3281fffb s3:test_larg_acl: adapt for the canonical ACE flags format via 75a089dc467 test:bb/samba-tool ntacl: let return acl flag lack hex padding via 16d2687cc7f libcli/security: do not pad sddl flags with zeros via 251da186bf4 libcli/security: ace type is not enum not flags via 56da318ceea libcli/security: disallow sddl access masks greater than 32 bits via 11add4d631f libcli/security: allow decimal/octal numbers in SDDL access mask via 5abd687fceb lib/sec/sddl: allow empty non-trailing ACL with flags via 7c97df17863 pytest:sddl: test empty DACL with flags via b621c59f64c libcli/sec/sddl decode: allow hex numbers in SIDs via 22fe657c8a2 libcli/sec/sddl decode: don't ignore random junk. via 4f5737cbf29 libcli/security/dom_sid: use (unsigned char) in isdigit() via 1149d391592 libcli/security/dom_sid: hex but not octal is OK for sub-auth via 67ff4ca200e libcli/security: avoid overflow in subauths via b3cff5636bc libcli/security: stricter identauth parsing via 6f37f8324c3 libcli/security: avoid overflow in revision number via 2398faef230 libcli/security/dom_sid: remove a couple of lost comments via fe8ce9e34e3 pytest:sid_strings: Do bad SIDs fail differently in simple-bind? via a4bbd944ee5 pytest:sid_strings: do bad SIDS work in search filters? via 866069172bf pytest:sid_strings: test SID DNs with ldb parsing via 953ad43f15e pytest:sid_strings: test SIDs as search base via f66b0f86883 pytest:sid_strings: Windows and Samba divergent tests via 2d75daa9c4d pytest:sid_strings: test the strings with local parsing via fa04c387403 pytest:sid_strings: separate out expected_sid formatting via cb356a8d909 pytest:sid_strings: add explicit S-1-* sid tests via 4380b4694f5 pytest:sid_strings: allow other errors to be specified via 5805dcf3ebf pytest:sid_strings: add a superclass, allowing for derivatives via 5c4f4dc9ead pytest:sid_strings: use hashed instead of random unique numbers via 708d9896aa3 pytest:sid_strings: same timestamp for all tests in the run via 489cdc42c43 librpc/py_security: exception message blames the bad SID via aa378b4bd51 pytest:upgradeprovision: don't use misleading SDDL in tests via 9abdd675650 librpc/ndr/pysecurity: use better exceptions via 9ab0d65fc0e lib/fuzzing: add fuzzer for sddl_parse from dc96e9cfd5d libcli:smb: Fix code spelling
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 4486d686f5c9404acc6fff7bc67432f14cac5800 Author: David Mulder <dmul...@samba.org> Date: Wed Apr 19 14:11:05 2023 -0600 gp: Add site-dn fallback when rpc call fails In testing I noticed that the rpc call for the site name is failing when joined via SSSD. This commit adds a fallback to check using the old style method found in ads_site_dn_for_machine() (which works, but doesn't obey the Group Policy spec) if the rpc call fails. Signed-off-by: David Mulder <dmul...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Fri Apr 28 03:14:25 UTC 2023 on atb-devel-224 commit c80affe0f192db9f851b5ed0617586783a02a82d Author: David Mulder <dmul...@samba.org> Date: Wed Mar 15 13:46:58 2023 -0600 Add a WHATSNEW entry indicating libgpo py deprecation BUG: https://bugzilla.samba.org/show_bug.cgi?id=15225 Signed-off-by: David Mulder <dmul...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ee04bafc25c7b09e53fe2036c5188531b58526a8 Author: David Mulder <dmul...@samba.org> Date: Tue Mar 14 15:35:01 2023 -0600 gpo: Group Policy tests require a s3 loadparam BUG: https://bugzilla.samba.org/show_bug.cgi?id=15225 Signed-off-by: David Mulder <dmul...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ac4726106c6d99794f03591fc0b526d91b947fad Author: David Mulder <dmul...@samba.org> Date: Tue Mar 14 12:37:54 2023 -0600 gpupdate: Deprecate libgpo.get_gpo_list This is no longer used by gpupdate. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15225 Signed-off-by: David Mulder <dmul...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a8bad5d5b859a2a76ce18919fbe2bf42f8ef7562 Author: David Mulder <dmul...@samba.org> Date: Tue Mar 14 11:21:02 2023 -0600 gpupdate: Implement get_gpo_list in python The ADS code in libgpo is buggy. Rewrite get_gpo_list in python using SamDB. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15225 Signed-off-by: David Mulder <dmul...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 848bce061afa514a2cc340f1b8895f83129ebd1a Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sun Apr 16 18:13:55 2023 +1200 libcli/security/tests: test strings for windows and samba SDDL tests These are produced by editing `python/samba/test/sddl.py to enable `test_write_test_strings`, the running `make test TESTS='sddl\\b'`. The windows executable from the C file added in a recent commit can run these tests using the `-i` flag. The Samba sddl.py tests can be induced to use them too, but that is only useful for showing they are still in sync. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit d36bab52d0fd68a8d28238dbba7e7ea35b936e6c Author: Noel Power <noel.po...@suse.com> Date: Thu Aug 25 14:29:09 2022 +0100 s3/utils: when encoding ace string use "FA", "FR", "FW", "FX" string rights prior to this patch rights matching "FA", "FR", "FW", "FX" were outputted as the hex string representing the bit value. While outputting the hex string is perfectly fine, it makes it harder to compare icacls output (which always uses the special string values) Additionally adjust various tests to deal with use of shortcut access masks as sddl format now uses FA, FR, FW & FX strings (like icalcs does) instead of hex representation of the bit mask. adjust samba4.blackbox.samba-tool_ntacl samba3.blackbox.large_acl samba.tests.samba_tool.ntacl samba.tests.ntacls samba.tests.posixacl so various string comparisons of the sddl format now pass Signed-off-by: Noel Power <noel.po...@suse.com> Reviewed-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> [abart...@samba.org Adapted to new stricter SDDL behaviour around leading zeros in hex numbers, eg 0x001] commit 0a153c1d58d8ae22432e990779afa0bb8fc9f9c9 Author: Noel Power <noel.po...@suse.com> Date: Thu Aug 25 13:52:56 2022 +0100 s3/utils: value for ace_flags value "FA" is incorrect value for FA should be 0x001f01ff (instead of 0x00001ff) Signed-off-by: Noel Power <noel.po...@suse.com> Reviewed-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> commit 9fc6062bd3b8c44ccdef0ac6572c28b0798fb557 Author: Andrew Bartlett <abart...@samba.org> Date: Wed Apr 26 17:00:17 2023 +1200 pytest:sddl: show the correct handling of the "FA" SDDL flag The "FA" flag should map to 0x1f01ff, and 0x1f01ff should be converted back into "FA". This will be fixed over the next couple of commits. Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> commit 334afc7157ecf783db9186e93f5489136e273b8a Author: Andrew Bartlett <abart...@samba.org> Date: Wed Apr 26 16:27:38 2023 +1200 pytest:sddl Samba had the wrong value for FA, now fix the tests The tests that were in SddlWindowsFlagsAreDifferent have the behaviour we want, and as we aim for Samba flags no longer being different, we shift them to SddlNonCanonical. The tests in SddlSambaDoesItsOwnThing are removed because they showed Samba's old behaviour around FA. This will create knownfails, which will be fixed by the commit fixing the value of "FA". Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> commit c0d477738eaf736d13b4c17f638776d320b3f1e6 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 26 10:24:25 2023 +1200 libcli:security:sddl: accept only 8-4-4-4-12 GUIDs Before we would take strings in a variety of lengths and formats, which is not what Windows does or [MS-DTYP] says. This was found by looking at evolved fuzz seeds. Note the 16 and 32 byte sequences in GUID position below: $ hd $(ls -t seeds/fuzz_sddl_parse/* | head -1)| head 00000000 44 3a 41 52 50 50 50 50 50 28 4f 4c 3b 3b 46 57 |D:ARPPPPP(OL;;FW| 00000010 3b 30 7e ff ff ff ff ff ff ff 2d 31 38 f5 ff ff |;0~.......-18...| 00000020 fb 3b 3b 52 43 29 28 4f 44 3b 3b 46 57 3b 3b 3b |.;;RC)(OD;;FW;;;| 00000030 52 43 29 28 4f 44 3b 3b 46 57 3b 30 30 ff ff ff |RC)(OD;;FW;00...| 00000040 fb 30 e9 9b 3c cf e6 f5 ff ff fb 3b 3b 52 43 29 |.0..<......;;RC)| 00000050 28 4f 44 3b 3b 46 57 43 52 3b 3b 3b 52 43 29 28 |(OD;;FWCR;;;RC)(| 00000060 4f 44 3b 3b 46 58 47 52 3b 3b 33 43 43 35 38 37 |OD;;FXGR;;3CC587| 00000070 32 35 44 44 44 44 44 44 44 44 44 44 44 44 44 44 |25DDDDDDDDDDDDDD| 00000080 44 44 44 44 44 44 44 44 44 44 3b 52 43 29 28 4f |DDDDDDDDDD;RC)(O| 00000090 44 3b 3b 46 58 3b 3b 3b 52 43 29 28 4f 44 3b 3b |D;;FX;;;RC)(OD;;| Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 4c1d9e92e11030b1757d9f77c9ab20a5d7f0d3ea Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 26 12:40:22 2023 +1200 pytest:large_ldap: use a valid ACE Real ACEs don't have {} around their GUIDs. This will soon be banned. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 2e90ba7ec6f022264d6764774cfa22993a6d9ca9 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 26 10:33:12 2023 +1200 pytest:sddl: test we only accept normal GUIDs By normal GUID, I mean ones like f30e3bbf-9ff0-11d1-b603-0000f80367c1, with four hyphens and no curly braces. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 46793d384e9810b4d814ffbea65714c064e16439 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sun Apr 23 12:36:35 2023 +1200 libcli:security:sddl_decode_access allows spaces between flags because Windows does. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ec2d2f8ea83f433b32071ebf40a8358c084b060b Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sun Apr 23 08:52:42 2023 +1200 pytest:sddl: tests around spaces in access flags and SIDs It turns out that in accesss flags Windows will allow leading spaces and spaces separating flags but not trailing spaces. We choose to follow this in part because we found it happening in the wild in our tests for upgradeprovision until a few commits ago. Windows will also allow spaces in some parts of SIDs. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 0528da54b8cc53954f10f23049fa90a5f3bdd50c Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Apr 22 00:48:30 2023 +1200 pytest:sddl debugging: should_fail test says how it failed Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit e7445aa677fb56540781ba144fda967d9af95552 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Apr 22 00:47:16 2023 +1200 libcli:security: sddl_decode_ace: don't allow junk after SID sddl_decode_sid() will stop at the first non-SID character. Windows doesn't allow white space here, and nor do we. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit c67f2292cba7a2ee047b196e565cf97cd6900973 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Apr 21 15:47:32 2023 +1200 libcli/security: sddl_decode_access rejects trailing rubbish Before we just ignored things like negative numbers, because they'd end up being seen as not-numbers, so treated as flags, then as not-flags. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit faf1b80a9003b883c77451beaec599777b400eb8 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Apr 21 15:47:10 2023 +1200 libcli:security: sddl_map_flags rejects trailing nonsense Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 96fe7ebe3f3f17b479daa07be30b52f51797e194 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 19 17:08:02 2023 +1200 s3:torture: sid2unixid2: DEBUG blames the right function Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 396d2805465e9eb9a930e043026c36d44203b461 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 19 16:37:53 2023 +1200 s3:torture:LOCAL-IDMAP-TDB-COMMON: avoid talloc stacktrace The short version is: Running LOCAL-IDMAP-TDB-COMMON test_getnewid1: PASSED! test_setmap1: PASSED! test_unixid2sid1: PASSED! test_sid2unixid1: could not create uid map! TEST LOCAL-IDMAP-TDB-COMMON FAILED! LOCAL-IDMAP-TDB-COMMON took 0.029819 secs Freed frame ../../source3/torture/torture.c:15748, expected ../../source3/torture/test_idmap_tdb_common.c:986. =============================================================== INTERNAL ERROR: Frame not freed in order. in pid 3692106 (4.19.0pre1-DEVELOPERBUILD) If you are running a recent Samba version, and if you think this problem is not yet fixed in the latest versions, please consider reporting this bug, see https://wiki.samba.org/index.php/Bug_Reporting =============================================================== PANIC (pid 3692106): Frame not freed in order. in 4.19.0pre1-DEVELOPERBUILD BACKTRACE: 11 stack frames: #0 bin/shared/private/libgenrand-samba4.so(log_stack_trace+0x32) [0x7f2f39b430ba] #1 bin/shared/private/libgenrand-samba4.so(smb_panic_log+0x1dd) [0x7f2f39b43037] #2 bin/shared/private/libgenrand-samba4.so(smb_panic+0x1c) [0x7f2f39b43056] #3 bin/shared/libsamba-util.so.0(+0x75309) [0x7f2f3a659309] #4 bin/shared/private/libtalloc-samba4.so(+0x5cc6) [0x7f2f3a758cc6] #5 bin/shared/private/libtalloc-samba4.so(+0x6173) [0x7f2f3a759173] #6 bin/shared/private/libtalloc-samba4.so(_talloc_free+0x10c) [0x7f2f3a75a54b] #7 /data/samba/samba-review/bin/smbtorture3(main+0xa97) [0x55cb3dc8cedc] #8 /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7f2f396d4d90] #9 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7f2f396d4e40] #10 /data/samba/samba-review/bin/smbtorture3(_start+0x25) [0x55cb3dc59895] smb_panic(): calling panic action [/data/samba/samba-review/selftest/gdb_backtrace 3692106] Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 1d9712283bf15afb576130370f067b7a383ecc6c Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Mon Apr 17 14:46:52 2023 +1200 pytest:sddl: add tests for long DACLs, differing flag interpretations Windows converts hex numbers into flags differently, and has different ideas of what constitutes "FA", and possibly others. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit de6d470063066321e30465189f4c9ba18ca37200 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sun Apr 16 18:43:40 2023 +1200 pytest:sddl: let hex numbers differ in case (0xa == 0xA) Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 030ce22f52561e23a0f79327b47a48e4655ac9c4 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Apr 15 20:29:53 2023 +1200 pytest:sddl: helpers to exchange SDDL strings with Windows testprogram Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit d9e1fa34563b9dbde1f6840412b19b380be9ffb0 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Mar 22 15:49:26 2023 +1300 libcli/security: SDDL parse tests to run on Windows The C version tests the public SDDL API on Windows which seems to follow Active Directory closely, though case in hex numbers is reversed vis-a-vis defaultSecurityDescriptor. The python version is less refined and tests powershell functions. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 97353c00917c7313ba91c4d424e4358ecf9eead4 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Apr 15 20:32:30 2023 +1200 pytest:sddl: SDDL strings where Windows behaviour differs These ones we might want to match. They are understandable behaviours, like matching lowercase flags and coping with whitespace in some places. These tests are set up to document the differences without overwhelming the knownfails. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit fb588d768be91b7d1618a9e69d48ae6a2a23dbec Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Apr 15 20:24:24 2023 +1200 pytest:sddl: Add negative tests of unparseable strings Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a2009b56b51cc42ec3a9386db1022e46cad7636f Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Apr 15 20:42:12 2023 +1200 pytest:sddl: allow tests to make negative assertions If the subclass has `should_succeed = False`, all the cases in that class will be tested to ensure they can't be successfully parsed. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ba6f401340103fca5acd78058e1978f830ac6454 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 15:59:32 2023 +1200 pytest:sddl: split each string into it's own test This of course allows for fine-grained knownfails. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit eac400b4dbe061260f6d44828670dc1aa2838358 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sat Apr 22 18:11:49 2023 +1200 pytest:sddl: tweak some test strings Adding, diversifying, and disambiguating. The leading portion of the test stirngs will soon be used in the test name, and strings that don't differ in the first hundred characters will cause naming clashes. There is no good reason for them all to test the same flags in the same order. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 4652d2766a72dd1e1e30c429aae400fd9c07ecec Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 22:18:21 2023 +1200 pytest/sddl: split tests into canonical and non-canonical The examples in the canonical list are already in the form that Windows and Samba will use for that SD. We check the round trip. The examples in the non-canonical list will change in a round trip, so we also give the string we think they should end up as. These have been checked on Windows. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 1107952c2b9012651589ad1676fa155f0d8b3819 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 23:18:04 2023 +1200 pytest/sddl: remove unused imports Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ec85c1fdff5665b1f539a03c9e8d0bab6baf9e63 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Apr 14 01:00:18 2023 +1200 pytest/sddl: rework to allow multiple lists, no early stop The test will fail right now because it makes round trip assertions. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 4a24c520569ff3e5d32a44fc599f58fb6b9f5326 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Mar 22 16:31:10 2023 +1300 pytest/sddl: assert sddl string equality It's not that I think our SD equality check will miss anything, but we are here to test things like that. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit f87f63997ff482c4d284c20b3cc2e8c5028619c8 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Mar 21 13:10:52 2023 +1300 pytest/sddl: remove duplicate test case The other copy is on line 102. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 298821a8edbe399ea9e8515b574b057789734dea Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Mar 21 13:05:55 2023 +1300 pytest/sddl: give test more of a name I think it worked, but the convention is that tests have a test_ prefix, and it woudn't be surpoising if something somewhere decides to depend on that. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 35bf8ff4f46f1eb14108cca7e063216c9c13086e Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Mar 21 13:02:13 2023 +1300 pytests/sddl: clarify boundaries between sddl cases It is now easier to see where one SD ends and another starts. Best looked at with -b or --word-diff. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 67500da14868450cb9db01a916f7f6eda9eb3455 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Apr 18 11:50:23 2023 +1200 pytest:posixacl: expect canonical ACE flag format Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit c08959d1358b763fcfa56ff1aaf45fbae37ca8e7 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Apr 18 11:44:04 2023 +1200 pytest:samba-tool ntacl: expect canonical ACE flag format Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a655e7e496211db5b3126bd349c6cb54efc49938 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Apr 18 11:42:57 2023 +1200 py:provision: use canonical representation of ACE flags This is because in ceetain places we compare strings rather than security descriptors. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit e521b0a26a9db03e76b3d8ed0e767fb59f7eba85 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Apr 18 11:16:03 2023 +1200 pytest:ntacls: adapt for canonical flag format Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 82b3281fffbf3ff592fb56fd10e370e1d79ca6b7 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Apr 18 11:52:29 2023 +1200 s3:test_larg_acl: adapt for the canonical ACE flags format Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 75a089dc4679b473847fec49f339319d63e2e006 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Mon Apr 17 14:48:41 2023 +1200 test:bb/samba-tool ntacl: let return acl flag lack hex padding Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 16d2687cc7f189495295c621c3d2d3af9946f66a Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Mar 24 14:21:14 2023 +1300 libcli/security: do not pad sddl flags with zeros We don't see this happening on Windows. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 251da186bf4cf184ec0561ae404cfd5f08b0ae65 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Mar 24 16:18:44 2023 +1300 libcli/security: ace type is not enum not flags Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 56da318ceea55763134587ab615cbfbbf955df11 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 12 10:46:30 2023 +1200 libcli/security: disallow sddl access masks greater than 32 bits Our previous behaviour (at least with glibc) was to clip off the extra bits, so that 0x123456789 would become 0x23456789. That's kind of the obvious thing, but is not what Windows does, which is to saturate the value, rounding to 0xffffffff. The effect of this is to turn on all the flags, which quite possibly not what you meant. Now we just return an error. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 11add4d631f559996dd5fda0a6cce7eb675cc6ae Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Mar 23 21:28:09 2023 +0000 libcli/security: allow decimal/octal numbers in SDDL access mask This follows Windows and [MS-DTYP]. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 5abd687fceb09451986359253361bca0d649372b Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Mar 16 21:17:56 2023 +1300 lib/sec/sddl: allow empty non-trailing ACL with flags The string "S:D:P" is parsed by us and Windows into a valid struct, which has an empty DACL with the PROTECTED flag, and an empty SACL. This is reconstructed in canonical order as "D:PS:", which Windows will correctly parse, but Samba has assumed the "S" is a bad DACL flag. Now we don't make that assumption. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7c97df1786329eeaacb3bf7f4741ecec9b7d1304 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Mar 17 12:19:00 2023 +1300 pytest:sddl: test empty DACL with flags Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit b621c59f64cb85877e4fd116f31e5556f146d88e Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Mar 16 15:46:08 2023 +1300 libcli/sec/sddl decode: allow hex numbers in SIDs These occur canonically when the indentifier authority is > 2^32, but also are accepted by Windows for any number. There is a tricky case with an "O:" or "G:" SID that is immediately followed by a "D:" dacl, because the "D" looks like a hex digit. When we detect this we need to subtract one from the length. We also need to do look out for trailing garbage. This was not an issue before because any string caught by the strspn(..., "-0123456789") would be either rejected or fully comsumed by dom_sid_parse_talloc(), but with hex digits, a string like "S-1-1-2x0xabcxxx-X" would be successfully parsed as "S-1-1-2", and the "x0xabcxxx-X" would be skipped over. That's why we switch to using dom_sid_parse_endp(), so we can compare the consumed length to the expected length. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 22fe657c8a2626816bdb458afe8dc2f094245822 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Mar 16 15:44:11 2023 +1300 libcli/sec/sddl decode: don't ignore random junk. previously a string could have anything in it, so long as every second character was ':'. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 4f5737cbf2931903216322d68084206280a210ad Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Apr 21 15:32:01 2023 +1200 libcli/security/dom_sid: use (unsigned char) in isdigit() The man page notes: The standards require that the argument c for these functions is either EOF or a value that is representable in the type unsigned char. If the argument c is of type char, it must be cast to unsigned char, as in the following example: Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 1149d391592a919c768df0e0959d7987c62fc7de Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sun Apr 16 12:21:16 2023 +1200 libcli/security/dom_sid: hex but not octal is OK for sub-auth Following Windows, the numbers that would be octal (e.g. "0123") are converted to decimal by skipping over the zeros. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 67ff4ca200e69a112afa3a25362da707e00732e6 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 12 11:39:25 2023 +1200 libcli/security: avoid overflow in subauths Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit b3cff5636bcf9fee23207dce5a34569912f4b1cb Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 12 11:38:24 2023 +1200 libcli/security: stricter identauth parsing We don't want octal numbers or overflows. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 6f37f8324c39336c111c1d519d25387f4cf1b434 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Mar 16 15:42:52 2023 +1300 libcli/security: avoid overflow in revision number Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 2398faef2300ec6942c396484957058597be9b02 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Mar 16 15:39:05 2023 +1300 libcli/security/dom_sid: remove a couple of lost comments The second one came with code obsoleting the "BIG NOTE" about 10 years ago, but that code later wandered off somewhere else. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit fe8ce9e34e35a61acf9114b2c3e52d2a63d2944c Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 12:17:28 2023 +1200 pytest:sid_strings: Do bad SIDs fail differently in simple-bind? No. That's good and expected because a failure here should fall back to the next thing in the simple bind pecking order (canonical names). Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a4bbd944ee50e54b2b07c713f624193dd857ec0f Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 12:13:26 2023 +1200 pytest:sid_strings: do bad SIDS work in search filters? Yes. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 866069172bfdfb0235163e9d2624cae12c3a11a0 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 12:11:48 2023 +1200 pytest:sid_strings: test SID DNs with ldb parsing By using an ldb.Dn as an intermediary, we get to see which SIDs Samba thinks are OK but Windows thinks are bad. It is things like "S-0-5-32-579". Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 953ad43f15eb157d0a05edb31b03a20e13c40da4 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 11:21:38 2023 +1200 pytest:sid_strings: test SIDs as search base As a way of testing the interpretation of a SID string in a remote server, we search on the base DN "<SID=x>" where x is a non-existent or malformed SID. On Windows some or all malformed SIDs are detected before the search begins, resulting in a complaint about DN syntax rather than one about missing objects. From this we can get a picture of what Windows considers to be a proper SID in this context. Samba does not make a distinction here, always returning NO_SUCH_OBJECT. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit f66b0f868836a9fd1a223e1f374fea61f89d4cd5 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 12 13:31:40 2023 +1200 pytest:sid_strings: Windows and Samba divergent tests The Samba side is aspirational -- what we actually do is generally worse. However the Windows behaviour in these cases seems more surprising still, and seems to be neither documented nor used. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 2d75daa9c4de54a5e179a254d7944f6bcd407370 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 11:47:19 2023 +1200 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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit fa04c3874038d08cbba197454607e0323f31bb08 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Apr 13 11:30:26 2023 +1200 pytest:sid_strings: separate out expected_sid formatting This is going to be useful for another test, soon. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit cb356a8d9090095e4dd524d0c9ea0d5824c135e4 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 5 15:39:24 2023 +1200 pytest:sid_strings: add explicit S-1-* sid tests We are mostly testing edge cases around the handling of numeric limits. These tests are based on ground truth established by running them against Windows. Many fail against Samba, because the defaulSecurityDescriptor attribute is not validated at the time it is set while on Windows it is. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 4380b4694f593d877ebcf16a9ffd58361f7d2a60 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 5 16:05:59 2023 +1200 pytest:sid_strings: allow other errors to be specified Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 5805dcf3ebf09a86c87cc113ccd45d874efb6dd6 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 5 17:20:46 2023 +1200 pytest:sid_strings: add a superclass, allowing for derivatives This will allow e.g. a suite of tests that assert Windows behaviour that we might not choose to follow. Because @DynamicTestCase will mangle the class as it finds it, we can't use SidStringTests itself as a superclass for others. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 5c4f4dc9ead3636bd60fb0f7dc15331cbe49c2ec Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 5 15:20:57 2023 +1200 pytest:sid_strings: use hashed instead of random unique numbers This removes the slim chance of flapping failures, and makes tracking the created class back to the SID string theoretically possible. To maintain uniqueness of the governs-id, we in chuck some of the timestamp. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 708d9896aa3e2c4f13fb98f78aa74c3d4d5ad45e Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 5 15:16:21 2023 +1200 pytest:sid_strings: same timestamp for all tests in the run We don't care about the exact time of the test, just that we disambiguate between different runs (each run leaves an immutable scar on the target server). Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 489cdc42c43424949f99ff591bb0ec7454b308db Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 12 21:34:47 2023 +1200 librpc/py_security: exception message blames the bad SID It can be useful to know what you're looking for. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit aa378b4bd510f99bb72b81a52f29c3c6e61617b1 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Sun Apr 23 08:41:23 2023 +1200 pytest:upgradeprovision: don't use misleading SDDL in tests The ACE string "(A;CI;RP LCLORC;;;AU)", with a space after "RP", is currently not parsed well by Samba. At the moment we parse only the "RP" and ignore the " LCLORC". What Windows would do is parse it as if it said "RPLCLORC", without the space, thus using all the flags. It seems very likely we thought this was happening with Samba. Soon Samba will have Windows' behaviour here and it will be tested in python/samba/tests/sddl.py. That means this test can relax and focus on whatever it was trying to do with upgradeprovision. We thank it for finding this discrepency. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9abdd6756500af1b0373bd325e5c0805755f2a4d Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Apr 12 17:34:35 2023 +1200 librpc/ndr/pysecurity: use better exceptions The wrong string is the wrong value but the right type. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9ab0d65fc0e0b52a7c24c2ca0d2b951a83e40acd Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Dec 18 17:58:56 2020 +1300 lib/fuzzing: add fuzzer for sddl_parse Apart from catching crashes in the actual parsing, we abort if the SD we end up with will not round trip back through SDDL to an identical SD. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: WHATSNEW.txt | 8 + ...redentials_parse_string.c => fuzz_sddl_parse.c} | 58 +- lib/fuzzing/wscript_build | 5 + libcli/security/dom_sid.c | 54 +- libcli/security/sddl.c | 209 ++++- libcli/security/tests/windows-sddl-test.py | 258 ++++++ libcli/security/tests/windows/canonical.txt | 19 + libcli/security/tests/windows/non_canonical.txt | 49 ++ libcli/security/tests/windows/should_fail.txt | 42 + libcli/security/tests/windows/windows-sddl-tests.c | 341 ++++++++ libcli/security/tests/windows/windows_is_fussy.txt | 1 + .../tests/windows/windows_is_less_fussy.txt | 23 + libcli/security/tests/windows/windows_is_weird.txt | 10 + libgpo/pygpo.c | 191 +++- .../rpc/dcerpc.py => python/samba/gp/__init__.py | 5 +- python/samba/gp/gpclass.py | 318 ++++++- python/samba/netcmd/domain/trust.py | 4 +- python/samba/provision/__init__.py | 4 +- python/samba/samdb.py | 2 +- python/samba/tests/gpo.py | 151 ++-- python/samba/tests/gpo_member.py | 2 +- python/samba/tests/ntacls.py | 2 +- python/samba/tests/posixacl.py | 16 +- python/samba/tests/samba_tool/ntacl.py | 6 +- python/samba/tests/sddl.py | 966 +++++++++++++++++---- python/samba/tests/security.py | 2 +- python/samba/tests/sid_strings.py | 403 ++++++++- python/samba/tests/upgradeprovision.py | 16 +- .../__init__.py => selftest/knownfail.d/sddl | 0 selftest/knownfail.d/sid-strings | 86 +- source3/script/tests/test_large_acl.sh | 2 +- source3/torture/test_idmap_tdb_common.c | 18 +- source4/dsdb/tests/python/large_ldap.py | 2 +- source4/librpc/ndr/py_security.c | 5 +- testprogs/blackbox/test_samba-tool_ntacl.sh | 17 +- 35 files changed, 2883 insertions(+), 412 deletions(-) copy lib/fuzzing/{fuzz_cli_credentials_parse_string.c => fuzz_sddl_parse.c} (56%) create mode 100644 libcli/security/tests/windows-sddl-test.py create mode 100644 libcli/security/tests/windows/canonical.txt create mode 100644 libcli/security/tests/windows/non_canonical.txt create mode 100644 libcli/security/tests/windows/should_fail.txt create mode 100644 libcli/security/tests/windows/windows-sddl-tests.c create mode 100644 libcli/security/tests/windows/windows_is_fussy.txt create mode 100644 libcli/security/tests/windows/windows_is_less_fussy.txt create mode 100644 libcli/security/tests/windows/windows_is_weird.txt copy source4/librpc/rpc/dcerpc.py => python/samba/gp/__init__.py (87%) copy buildtools/wafsamba/__init__.py => selftest/knownfail.d/sddl (100%) Changeset truncated at 500 lines: diff --git a/WHATSNEW.txt b/WHATSNEW.txt index d6b23b06f60..2b472aa0cdc 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -28,6 +28,14 @@ if needed, this is documented in the manpage. Please check the smbget manpage or --help output. +gpupdate changes +---------------- + +The libgpo.get_gpo_list function has been deprecated in favor of +an implementation written in python. The new function can be imported via +`import samba.gp`. The python implementation connects to Active Directory +using the SamDB module, instead of ADS (which is what libgpo uses). + REMOVED FEATURES ================ diff --git a/lib/fuzzing/fuzz_cli_credentials_parse_string.c b/lib/fuzzing/fuzz_sddl_parse.c similarity index 56% copy from lib/fuzzing/fuzz_cli_credentials_parse_string.c copy to lib/fuzzing/fuzz_sddl_parse.c index b71ef357309..b6c48fb7ca5 100644 --- a/lib/fuzzing/fuzz_cli_credentials_parse_string.c +++ b/lib/fuzzing/fuzz_sddl_parse.c @@ -1,6 +1,6 @@ /* - Fuzz cli_credentials_parse_string - Copyright (C) Catalyst IT 2020 + Fuzz sddl decoding and encoding + Copyright (C) Catalyst IT 2023 This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -15,49 +15,51 @@ You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ + #include "includes.h" -#include "auth/credentials/credentials.h" +#include "libcli/security/security.h" #include "fuzzing/fuzzing.h" -#define MAX_LENGTH (1024 * 10) -char buf[MAX_LENGTH + 1]; +#define MAX_LENGTH (100 * 1024 - 1) +static char sddl_string[MAX_LENGTH + 1] = {0}; +static struct dom_sid dom_sid = {0}; -const enum credentials_obtained obtained = CRED_UNINITIALISED; +int LLVMFuzzerInitialize(int *argc, char ***argv) +{ + string_to_sid(&dom_sid, + "S-1-5-21-2470180966-3899876309-2637894779"); + return 0; +} int LLVMFuzzerTestOneInput(uint8_t *input, size_t len) { TALLOC_CTX *mem_ctx = NULL; - struct cli_credentials *credentials = NULL; - bool anon; - const char *username; - const char *domain; + struct security_descriptor *sd1 = NULL; + struct security_descriptor *sd2 = NULL; + char *result = NULL; + bool ok; if (len > MAX_LENGTH) { return 0; } - memcpy(buf, input, len); - buf[len] = '\0'; + memcpy(sddl_string, input, len); + sddl_string[len] = '\0'; mem_ctx = talloc_new(NULL); - credentials = cli_credentials_init(mem_ctx); - - cli_credentials_parse_string(credentials, buf, obtained); - - anon = cli_credentials_is_anonymous(credentials); - - cli_credentials_get_ntlm_username_domain(credentials, - mem_ctx, - &username, - &domain); + sd1 = sddl_decode(mem_ctx, sddl_string, &dom_sid); + if (sd1 == NULL) { + goto end; + } + result = sddl_encode(mem_ctx, sd1, &dom_sid); + sd2 = sddl_decode(mem_ctx, result, &dom_sid); + ok = security_descriptor_equal(sd1, sd2); + if (!ok) { + abort(); + } +end: talloc_free(mem_ctx); return 0; } - - -int LLVMFuzzerInitialize(int *argc, char ***argv) -{ - return 0; -} diff --git a/lib/fuzzing/wscript_build b/lib/fuzzing/wscript_build index ee3cfc14317..187c23c7cb8 100644 --- a/lib/fuzzing/wscript_build +++ b/lib/fuzzing/wscript_build @@ -32,6 +32,11 @@ bld.SAMBA_BINARY('fuzz_reg_parse', deps='fuzzing samba3-util smbconf REGFIO afl-fuzz-main', fuzzer=True) +bld.SAMBA_BINARY('fuzz_sddl_parse', + source='fuzz_sddl_parse.c', + deps='fuzzing samba-security afl-fuzz-main', + fuzzer=True) + bld.SAMBA_BINARY('fuzz_nmblib_parse_packet', source='fuzz_nmblib_parse_packet.c', deps='fuzzing libsmb afl-fuzz-main', diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c index 891d3c5e17c..52d4454e4e5 100644 --- a/libcli/security/dom_sid.c +++ b/libcli/security/dom_sid.c @@ -131,8 +131,8 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout, const char **endp) { const char *p; - char *q; - /* BIG NOTE: this function only does SIDS where the identauth is not >= 2^32 */ + char *q = NULL; + char *end = NULL; uint64_t conv; int error = 0; @@ -145,28 +145,42 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout, /* Get the revision number. */ p = sidstr + 2; - if (!isdigit(*p)) { + if (!isdigit((unsigned char)*p)) { goto format_error; } conv = smb_strtoul(p, &q, 10, &error, SMB_STR_STANDARD); - if (error != 0 || (*q != '-') || conv > UINT8_MAX) { + if (error != 0 || (*q != '-') || conv > UINT8_MAX || q - p > 4) { goto format_error; } sidout->sid_rev_num = (uint8_t) conv; q++; - if (!isdigit(*q)) { + if (!isdigit((unsigned char)*q)) { goto format_error; } + while (q[0] == '0' && isdigit((unsigned char)q[1])) { + /* + * strtoull will think this is octal, which is not how SIDs + * work! So let's walk along until there are no leading zeros + * (or a single zero). + */ + q++; + } /* get identauth */ - conv = smb_strtoull(q, &q, 0, &error, SMB_STR_STANDARD); + conv = smb_strtoull(q, &end, 0, &error, SMB_STR_STANDARD); if (conv & AUTHORITY_MASK || error != 0) { goto format_error; } + if (conv >= (1ULL << 48) || end - q > 15) { + /* + * This identauth looks like a big number, but resolves to a + * small number after rounding. + */ + goto format_error; + } - /* When identauth >= UINT32_MAX, it's in hex with a leading 0x */ /* NOTE - the conv value is in big-endian format. */ sidout->id_auth[0] = (conv & 0xff0000000000ULL) >> 40; sidout->id_auth[1] = (conv & 0x00ff00000000ULL) >> 32; @@ -176,6 +190,7 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout, sidout->id_auth[5] = (conv & 0x0000000000ffULL); sidout->num_auths = 0; + q = end; if (*q != '-') { /* Just id_auth, no subauths */ goto done; @@ -184,14 +199,27 @@ bool dom_sid_parse_endp(const char *sidstr,struct dom_sid *sidout, q++; while (true) { - char *end; - - if (!isdigit(*q)) { + if (!isdigit((unsigned char)*q)) { goto format_error; } - - conv = smb_strtoull(q, &end, 10, &error, SMB_STR_STANDARD); - if (conv > UINT32_MAX || error != 0) { + while (q[0] == '0' && isdigit((unsigned char)q[1])) { + /* + * strtoull will think this is octal, which is not how + * SIDs work! So let's walk along until there are no + * leading zeros (or a single zero). + */ + q++; + } + conv = smb_strtoull(q, &end, 0, &error, SMB_STR_STANDARD); + if (conv > UINT32_MAX || error != 0 || end - q > 12) { + /* + * This sub-auth is greater than 4294967295, + * and hence invalid. Windows will treat it as + * 4294967295, while we prefer to refuse (old + * versions of Samba will wrap, arriving at + * another number altogether). + */ + DBG_NOTICE("bad sub-auth in %s\n", sidstr); goto format_error; } diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c index 508ac3e5666..e14b2748384 100644 --- a/libcli/security/sddl.c +++ b/libcli/security/sddl.c @@ -23,7 +23,10 @@ #include "lib/util/debug.h" #include "libcli/security/security.h" #include "librpc/gen_ndr/ndr_misc.h" +#include "lib/util/smb_strtox.h" #include "system/locale.h" +#include "lib/util/util_str_hex.h" + struct sddl_transition_state { const struct dom_sid *machine_sid; @@ -60,22 +63,22 @@ static bool sddl_map_flag( map a series of letter codes into a uint32_t */ static bool sddl_map_flags(const struct flag_map *map, const char *str, - uint32_t *pflags, size_t *plen) + uint32_t *pflags, size_t *plen, + bool unknown_flag_is_part_of_next_thing) { const char *str0 = str; if (plen != NULL) { *plen = 0; } *pflags = 0; - while (str[0] && isupper(str[0])) { + while (str[0] != '\0' && isupper((unsigned char)str[0])) { size_t len; uint32_t flags; bool found; found = sddl_map_flag(map, str, &len, &flags); if (!found) { - DEBUG(1, ("Unknown flag - %s in %s\n", str, str0)); - return false; + break; } *pflags |= flags; @@ -84,9 +87,22 @@ static bool sddl_map_flags(const struct flag_map *map, const char *str, } str += len; } - return true; + /* + * For ACL flags, unknown_flag_is_part_of_next_thing is set, + * and we expect some more stuff that isn't flags. + * + * For ACE flags, unknown_flag_is_part_of_next_thing is unset, + * and the flags have been tokenised into their own little + * string. We don't expect anything here, even whitespace. + */ + if (*str == '\0' || unknown_flag_is_part_of_next_thing) { + return true; + } + DBG_WARNING("Unknown flag - '%s' in '%s'\n", str, str0); + return false; } + /* a mapping between the 2 letter SID codes and sid strings */ @@ -191,16 +207,47 @@ static struct dom_sid *sddl_decode_sid(TALLOC_CTX *mem_ctx, const char **sddlp, /* see if its in the numeric format */ if (strncmp(sddl, "S-", 2) == 0) { - struct dom_sid *sid; - char *sid_str; - size_t len = strspn(sddl+2, "-0123456789"); - sid_str = talloc_strndup(mem_ctx, sddl, len+2); - if (!sid_str) { + struct dom_sid *sid = NULL; + char *sid_str = NULL; + const char *end = NULL; + bool ok; + size_t len = strspn(sddl + 2, "-0123456789ABCDEFabcdefxX") + 2; + if (len < 5) { /* S-1-x */ return NULL; } - (*sddlp) += len+2; - sid = dom_sid_parse_talloc(mem_ctx, sid_str); - talloc_free(sid_str); + if (sddl[len - 1] == 'D' && sddl[len] == ':') { + /* + * we have run into the "D:" dacl marker, mistaking it + * for a hex digit. There is no other way for this + * pair to occur at the end of a SID in SDDL. + */ + len--; + } + + sid_str = talloc_strndup(mem_ctx, sddl, len); + if (sid_str == NULL) { + return NULL; + } + sid = talloc(mem_ctx, struct dom_sid); + if (sid == NULL) { + TALLOC_FREE(sid_str); + return NULL; + }; + ok = dom_sid_parse_endp(sid_str, sid, &end); + if (!ok) { + DBG_WARNING("could not parse SID '%s'\n", sid_str); + TALLOC_FREE(sid_str); + TALLOC_FREE(sid); + return NULL; + } + if (end - sid_str != len) { + DBG_WARNING("trailing junk after SID '%s'\n", sid_str); + TALLOC_FREE(sid_str); + TALLOC_FREE(sid); + return NULL; + } + TALLOC_FREE(sid_str); + (*sddlp) += len; return sid; } @@ -279,30 +326,96 @@ static const struct flag_map ace_access_mask[] = { }; static const struct flag_map decode_ace_access_mask[] = { - { "FA", FILE_ALL_ACCESS }, + { "FA", FILE_GENERIC_ALL }, { "FR", FILE_GENERIC_READ }, { "FW", FILE_GENERIC_WRITE }, { "FX", FILE_GENERIC_EXECUTE }, { NULL, 0 }, }; + +static char *sddl_match_file_rights(TALLOC_CTX *mem_ctx, + uint32_t flags) +{ + int i; + + /* try to find an exact match */ + for (i=0;decode_ace_access_mask[i].name;i++) { + if (decode_ace_access_mask[i].flag == flags) { + return talloc_strdup(mem_ctx, + decode_ace_access_mask[i].name); + } + } + return NULL; +} + static bool sddl_decode_access(const char *str, uint32_t *pmask) { const char *str0 = str; + char *end = NULL; uint32_t mask = 0; - int cmp; - - cmp = strncmp(str, "0x", 2); - if (cmp == 0) { - *pmask = strtol(str, NULL, 16); + unsigned long long numeric_mask; + int err; + /* + * The access mask can be a number or a series of flags. + * + * Canonically the number is expressed in hexadecimal (with 0x), but + * per MS-DTYP and Windows behaviour, octal and decimal numbers are + * also accepted. + * + * Windows has two behaviours we choose not to replicate: + * + * 1. numbers exceeding 0xffffffff are truncated at that point, + * turning on all access flags. + * + * 2. negative numbers are accepted, so e.g. -2 becomes 0xfffffffe. + */ + numeric_mask = smb_strtoull(str, &end, 0, &err, SMB_STR_STANDARD); + if (err == 0) { + if (numeric_mask > UINT32_MAX) { + DBG_WARNING("Bad numeric flag value - %llu in %s\n", + numeric_mask, str0); + return false; + } + if (end - str > sizeof("037777777777")) { + /* here's the tricky thing: if a number is big + * enough to overflow the uint64, it might end + * up small enough to fit in the uint32, and + * we'd miss that it overflowed. So we count + * the digits -- any more than 12 (for + * "037777777777") is too long for 32 bits, + * and the shortest 64-bit wrapping string is + * 19 (for "0x1" + 16 zeros). + */ + DBG_WARNING("Bad numeric flag value in '%s'\n", str0); + return false; + } + if (*end != '\0') { + DBG_WARNING("Bad characters in '%s'\n", str0); + return false; + } + *pmask = numeric_mask; return true; } + /* It's not a positive number, so we'll look for flags */ - while ((str[0] != '\0') && isupper(str[0])) { + while ((str[0] != '\0') && + (isupper((unsigned char)str[0]) || str[0] == ' ')) { uint32_t flags = 0; size_t len = 0; bool found; - + while (str[0] == ' ') { + /* + * Following Windows we accept spaces between flags + * but not after flags. Not tabs, though, never tabs. + */ + str++; + if (str[0] == '\0') { + DBG_WARNING("trailing whitespace in flags " + "- '%s'\n", str0); + return false; + } + } found = sddl_map_flag( ace_access_mask, str, &len, &flags); found |= sddl_map_flag( @@ -314,11 +427,24 @@ static bool sddl_decode_access(const char *str, uint32_t *pmask) mask |= flags; str += len; } - + if (*str != '\0') { + DBG_WARNING("Bad characters in '%s'\n", str0); + return false; + } *pmask = mask; return true; } + +static bool sddl_decode_guid(const char *str, struct GUID *guid) +{ + if (strlen(str) != 36) { + return false; + } + return parse_guid_string(str, guid); +} + + /* decode an ACE return true on success, false on failure @@ -333,6 +459,7 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx, struct security_ace *ace, char uint32_t v; struct dom_sid *sid; bool ok; + size_t len; ZERO_STRUCTP(ace); @@ -347,13 +474,20 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx, struct security_ace *ace, char } /* parse ace type */ - if (!sddl_map_flags(ace_types, tok[0], &v, NULL)) { -- Samba Shared Repository