[SCM] Samba Shared Repository - branch master updated
Andrew Bartlett
abartlet at samba.org
Fri Apr 28 03:15:01 UTC 2023
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 <dmulder at 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 <dmulder at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
Autobuild-Date(master): Fri Apr 28 03:14:25 UTC 2023 on atb-devel-224
commit c80affe0f192db9f851b5ed0617586783a02a82d
Author: David Mulder <dmulder at 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 <dmulder at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ee04bafc25c7b09e53fe2036c5188531b58526a8
Author: David Mulder <dmulder at 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 <dmulder at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ac4726106c6d99794f03591fc0b526d91b947fad
Author: David Mulder <dmulder at 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 <dmulder at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit a8bad5d5b859a2a76ce18919fbe2bf42f8ef7562
Author: David Mulder <dmulder at 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 <dmulder at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 848bce061afa514a2cc340f1b8895f83129ebd1a
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit d36bab52d0fd68a8d28238dbba7e7ea35b936e6c
Author: Noel Power <noel.power at 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.power at suse.com>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
[abartlet at samba.org Adapted to new stricter SDDL behaviour around leading zeros in hex
numbers, eg 0x001]
commit 0a153c1d58d8ae22432e990779afa0bb8fc9f9c9
Author: Noel Power <noel.power at 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.power at suse.com>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit 9fc6062bd3b8c44ccdef0ac6572c28b0798fb557
Author: Andrew Bartlett <abartlet at 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 <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit 334afc7157ecf783db9186e93f5489136e273b8a
Author: Andrew Bartlett <abartlet at 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 <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit c0d477738eaf736d13b4c17f638776d320b3f1e6
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4c1d9e92e11030b1757d9f77c9ab20a5d7f0d3ea
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 2e90ba7ec6f022264d6764774cfa22993a6d9ca9
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 46793d384e9810b4d814ffbea65714c064e16439
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ec2d2f8ea83f433b32071ebf40a8358c084b060b
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 0528da54b8cc53954f10f23049fa90a5f3bdd50c
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit e7445aa677fb56540781ba144fda967d9af95552
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit c67f2292cba7a2ee047b196e565cf97cd6900973
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit faf1b80a9003b883c77451beaec599777b400eb8
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 96fe7ebe3f3f17b479daa07be30b52f51797e194
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 396d2805465e9eb9a930e043026c36d44203b461
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 1d9712283bf15afb576130370f067b7a383ecc6c
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit de6d470063066321e30465189f4c9ba18ca37200
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 030ce22f52561e23a0f79327b47a48e4655ac9c4
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit d9e1fa34563b9dbde1f6840412b19b380be9ffb0
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 97353c00917c7313ba91c4d424e4358ecf9eead4
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit fb588d768be91b7d1618a9e69d48ae6a2a23dbec
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit a2009b56b51cc42ec3a9386db1022e46cad7636f
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ba6f401340103fca5acd78058e1978f830ac6454
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit eac400b4dbe061260f6d44828670dc1aa2838358
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4652d2766a72dd1e1e30c429aae400fd9c07ecec
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 1107952c2b9012651589ad1676fa155f0d8b3819
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Apr 13 23:18:04 2023 +1200
pytest/sddl: remove unused imports
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ec85c1fdff5665b1f539a03c9e8d0bab6baf9e63
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4a24c520569ff3e5d32a44fc599f58fb6b9f5326
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit f87f63997ff482c4d284c20b3cc2e8c5028619c8
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 298821a8edbe399ea9e8515b574b057789734dea
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 35bf8ff4f46f1eb14108cca7e063216c9c13086e
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 67500da14868450cb9db01a916f7f6eda9eb3455
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit c08959d1358b763fcfa56ff1aaf45fbae37ca8e7
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit a655e7e496211db5b3126bd349c6cb54efc49938
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit e521b0a26a9db03e76b3d8ed0e767fb59f7eba85
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 82b3281fffbf3ff592fb56fd10e370e1d79ca6b7
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 75a089dc4679b473847fec49f339319d63e2e006
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 16d2687cc7f189495295c621c3d2d3af9946f66a
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 251da186bf4cf184ec0561ae404cfd5f08b0ae65
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 56da318ceea55763134587ab615cbfbbf955df11
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 11add4d631f559996dd5fda0a6cce7eb675cc6ae
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 5abd687fceb09451986359253361bca0d649372b
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 7c97df1786329eeaacb3bf7f4741ecec9b7d1304
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit b621c59f64cb85877e4fd116f31e5556f146d88e
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 22fe657c8a2626816bdb458afe8dc2f094245822
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4f5737cbf2931903216322d68084206280a210ad
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 1149d391592a919c768df0e0959d7987c62fc7de
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 67ff4ca200e69a112afa3a25362da707e00732e6
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Apr 12 11:39:25 2023 +1200
libcli/security: avoid overflow in subauths
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit b3cff5636bcf9fee23207dce5a34569912f4b1cb
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 6f37f8324c39336c111c1d519d25387f4cf1b434
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 2398faef2300ec6942c396484957058597be9b02
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit fe8ce9e34e35a61acf9114b2c3e52d2a63d2944c
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit a4bbd944ee50e54b2b07c713f624193dd857ec0f
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 866069172bfdfb0235163e9d2624cae12c3a11a0
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 953ad43f15eb157d0a05edb31b03a20e13c40da4
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit f66b0f868836a9fd1a223e1f374fea61f89d4cd5
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 2d75daa9c4de54a5e179a254d7944f6bcd407370
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit fa04c3874038d08cbba197454607e0323f31bb08
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit cb356a8d9090095e4dd524d0c9ea0d5824c135e4
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 4380b4694f593d877ebcf16a9ffd58361f7d2a60
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 5805dcf3ebf09a86c87cc113ccd45d874efb6dd6
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 5c4f4dc9ead3636bd60fb0f7dc15331cbe49c2ec
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 708d9896aa3e2c4f13fb98f78aa74c3d4d5ad45e
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 489cdc42c43424949f99ff591bb0ec7454b308db
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit aa378b4bd510f99bb72b81a52f29c3c6e61617b1
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 9abdd6756500af1b0373bd325e5c0805755f2a4d
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 9ab0d65fc0e0b52a7c24c2ca0d2b951a83e40acd
Author: Douglas Bagnall <douglas.bagnall at 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.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at 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
More information about the samba-cvs
mailing list