1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-22 13:34:15 +03:00
Commit Graph

3960 Commits

Author SHA1 Message Date
Douglas Bagnall
7b9462faf0 pytest: security_descriptors: tests without revision number hack
ACL revision 4 (SECURITY_ACL_REVISION_ADS) is effectively a superset
of revision 2 (SECURITY_ACL_REVISION_NT4), so any revision 2
ACL can be called revision 4 without any problem. But not vice versa:
a revision 4 ACL can contain ACE types that a revision 2 ACL can't. The
extra ACE types relate to objects.

Samba currently simplifies things by calling all its ACLs revision 4,
even if (as is commonly the case) the ACLs contain only revision 2 ACEs.
On the other hand, Windows will use revision 2 whenever it can. In other
tests we skip past this by forcing Windows ACLs to v4 before comparison.
This test is to remind us of the incompatibility.

It would not be hard to fix.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-09-26 23:45:35 +00:00
Douglas Bagnall
63be840120 pytest: security_descriptors test for repetitive ACLs
If there are multiple identical ACEs in an SDDL ACL, Windows will decode
them all and put extra trailing zeroes at the end of the ACL.

In contrast, Samba will decode the ACEs and not put extra zeroes at the
end.

The problem comes when Samba tries to read a binary ACL from Windows that
has the extra zeroes, because Samba's ACL size calculation is based on
the size of its constituent ACEs, not the ACL size field.

There is no good reason for an ACL to have repeated ACEs, but they could
be added accidentally.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-09-26 23:45:35 +00:00
Douglas Bagnall
e4865a3ba1 libcli/security: test SDDL compilation in cmocka
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-09-26 23:45:35 +00:00
Volker Lendecke
3481bbfede smbd: Fix BZ15481
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15481

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Wed Sep 20 22:42:48 UTC 2023 on atb-devel-224
2023-09-20 22:42:48 +00:00
Volker Lendecke
56df75d447 tests: Add reproducer for BZ15481
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15481

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2023-09-20 21:48:36 +00:00
Jeremy Allison
11280f1705 s3: smbd: Ensure we remove any pending aio values for named pipes on forced shutdown.
Matches file and directory closes.

Remove knownfail.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Wed Sep 20 02:43:18 UTC 2023 on atb-devel-224
2023-09-20 02:43:18 +00:00
Jeremy Allison
66398dd03c s3: torture: Add a new SMB2 test: SMB2-PIPE-READ-ASYNC-DISCONNECT
Shows the server crashes if we open a named pipe, do an async read
and then disconnect.

Adds knownfail:

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2023-09-20 01:49:35 +00:00
Stefan Metzmacher
5b7f9840f7 selftest: add some basic testing for the io_uring vfs module
We're now able to build it on all linux systems and
the ci runners have at least a 5.4 kernel. That's
all the current vfs_io_uring requires.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Sun Sep 17 18:04:18 UTC 2023 on atb-devel-224
2023-09-17 18:04:18 +00:00
Stefan Metzmacher
91b30a7261 nsswitch/wb_common.c: don't operate on a stale wb_global_ctx.key
If nss_winbind is loaded into a process that uses fork multiple times
without any further calls into nss_winbind, wb_atfork_child handler
was using a wb_global_ctx.key that was no longer registered in the
pthread library, so we operated on a slot that was potentially
reused by other libraries or the main application. Which is likely
to cause memory corruption.

So we better don't call pthread_key_delete() in wb_atfork_child().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15464

Reported-by: Krzysztof Piotr Oledzki <ole@ans.pl>
Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2023-09-14 17:56:30 +00:00
Stefan Metzmacher
62af25d44e nsswitch: add test for pthread_key_delete missuse (bug 15464)
This is based on https://bugzilla.samba.org/attachment.cgi?id=18081
written by Krzysztof Piotr Oledzki <ole@ans.pl>

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15464

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2023-09-14 17:56:30 +00:00
Joseph Sutton
26fd734d56 selftest: Fix code spelling
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-09-11 02:42:41 +00:00
Volker Lendecke
ebb6eb9c2f libsmb: Fix parsing symlink reparse points
Untested code is broken code. For symlinks we need to hand over the
full reparse buffer into symlink_reparse_buffer_parse(), as this is
also used for the smb2 error response handling. For that, the
"reserved" field in [MS-FSCC] 2.1.2.4 Symbolic Link Reparse Data
Buffer is used for the "unparsed" field.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri Sep  8 17:24:19 UTC 2023 on atb-devel-224
2023-09-08 17:24:19 +00:00
Volker Lendecke
c9a000be41 tests: Add test_symlink_reparse_data_buffer_parse
The blob was taken from a smbclient allinfo command for a Windows
symlink. Show that reparse_data_buffer_parse() is broken.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2023-09-08 16:27:39 +00:00
Volker Lendecke
03ce770e8d tests: Create symlinks using posix extensions
This way we can run the tests and more easily put them into knownfail
individually. Before this, everything went into the error category,
which was not so easy to catch in something like knownfail.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2023-09-08 16:27:39 +00:00
Joseph Sutton
0898329b8d selftest: Don’t use invalid escape sequences
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-30 02:15:29 +00:00
Joseph Sutton
0fe4a12b3f selftest: Remove star imports
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-30 02:15:29 +00:00
Joseph Sutton
6db02afab8 selftest: Remove unused imports
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-30 02:15:29 +00:00
Joseph Sutton
e390e674ec tests/krb5: Remove test of pre-1.20 MIT Kerberos behaviour
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-30 02:15:29 +00:00
Gabriel Nagy
7dc181757c gp: Send list of keys instead of dict to remove
`cache_get_all_attribute_values` returns a dict whereas we need to pass
a list of keys to `remove`. These will be interpolated in the gpdb search.

Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Mon Aug 28 03:01:22 UTC 2023 on atb-devel-224
2023-08-28 03:01:22 +00:00
Gabriel Nagy
ee814f7707 gp: Test disabled enrollment unapplies policy
For this we need to stage a Registry.pol file with certificate
autoenrollment enabled, but with checkboxes unticked.

Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
2023-08-28 02:04:36 +00:00
Gabriel Nagy
2a6ae997f2 gp: Template changes should invalidate cache
If certificate templates are added or removed, the autoenroll extension
should react to this and reapply the policy. Previously this wasn't
taken into account.

Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
2023-08-28 02:04:36 +00:00
Gabriel Nagy
2d6943a864 gp: Test adding new cert templates enforces changes
Ensure that cepces-submit reporting additional templates and re-applying
will enforce the updated policy.

Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
2023-08-28 02:04:36 +00:00
Gabriel Nagy
157335ee93 gp: Convert CA certificates to base64
I don't know whether this applies universally, but in our case the
contents of `es['cACertificate'][0]` are binary, so cleanly converting
to a string fails with the following:

'utf-8' codec can't decode byte 0x82 in position 1: invalid start byte

We found a fix to be encoding the certificate to base64 when
constructing the CA list.

Section 4.4.5.2 of MS-CAESO also suggests that the content of
`cACertificate` is binary (OCTET string).

Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
2023-08-28 02:04:36 +00:00
Gabriel Nagy
1ef722cf66 gp: Test with binary content for certificate data
This fails all GPO-related tests that call `gpupdate --rsop`.

Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
2023-08-28 02:04:36 +00:00
Douglas Bagnall
18f44f3ba4 selftest:ndrdump: adjust xattr_NTACL test for ACE coda
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Thu Aug 24 03:47:08 UTC 2023 on atb-devel-224
2023-08-24 03:47:08 +00:00
Douglas Bagnall
882ad16138 pytest: compare Samba vs Windows SDDL security descriptors
Can Samba understand Windows security descriptors? Does it parse SDDL
the same way?

Here we test on over 7000 SDDL/descriptor pairs and find the answer
is pleasing. In later commits we will add more tests using different
classes of ACE.

The test cases are derived from fuzz seeds, exported to Windows via
the script in the last commit, with the Windows descriptor bytes found
using libcli/security/tests/windows/windows-sddl-test.py.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-24 02:53:31 +00:00
Douglas Bagnall
c73034cf7c librpc/security.idl: adjust size calculations for upcoming ace types
Soon we will get Conditional ACEs and Resource Attribute ACES, each of
which have trailing bytes at the end of the ACE. Here's a diagram:

              ____      The ACE size field may indicate a size bigger
  .type      /    |     than the known parts, even when you take
  .flags    /     |     rounding to a multiple of four into account.
  .size  --'      |     This extra data is meaningful in some ACEs.
  .access_mask    |
  .trustee (sid) _|  <- known data ends here.
                  :
   "coda"      ___:  <- the trailing part, Zero size unless the size
                        field points beyond the end of the known data.
			Probably empty for ordinary ACE types.

Until now we have thrown away these extra bytes, because they have no
meaning in the ACE types we recognise. But with conditional and
resource attribute ACEs we need to catch and process these bytes, so
we add an extra field for that.

Thus we can drop the manually written ndr_pull_security_ace() that
discarded the trailing bytes, because we just allow it to be pulled
into an unused blob. In the very common case, the blob will be empty.

Microsoft does not use a common name across different ACE types to
describe this end-data -- "coda" is a Samba term.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-24 02:53:31 +00:00
Joseph Sutton
aba7eb094b selftest: Report time at which testsuite starts
With no call to report_time() preceding it,
PlainFormatter.start_testsuite() would always claim that no time had
elapsed prior to the first testsuite starting to run. This gave a
misleading impression of the time spent running the first testsuite. Now
the time will be consistent with that reported for subsequent
testsuites, and will properly include any time that test environments
took to start up.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Tue Aug 22 00:36:52 UTC 2023 on atb-devel-224
2023-08-22 00:36:52 +00:00
Joseph Sutton
fc41a35a9b selftest: Fix subunit reporting the time incorrectly
Although report_time() would output the time in UTC, it neglected to
specify the timezone offset. Thus subunithelper.parse_results() would
interpret the time string it was given as being in local time.
TestProtocolClient.time() then converted that *back* into UTC, giving an
incorrect result (unless UTC is your local timezone).

Fix this by having report_time() indicate that the time zone is UTC.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15162

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-21 23:37:29 +00:00
Joseph Sutton
cbc0ff187a selftest: Remove leftover from debugging
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-21 23:37:29 +00:00
Joseph Sutton
7ce6e3c9ca selftest: Remove unused variable
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-21 23:37:29 +00:00
Joseph Sutton
c2126e08f4 selftest: Remove semicolon
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-21 23:37:29 +00:00
Joseph Sutton
aa1815519c claims.idl: Use ‘int64’ instead of ‘dlong’ for INT64 claims
This field is supposed to be aligned to eight bytes, but the ‘dlong’
type is aligned to only four bytes. This discrepancy resulted in claims
being encoded and decoded incorrectly.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15452

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-15 18:46:33 +00:00
Joseph Sutton
631e26e1d6 tests/krb5: Add a test decoding INT64 PAC claims issued by Windows
Our NDR code currently handles INT64 claims incorrectly.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15452

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-15 18:46:33 +00:00
Jeremy Allison
4145bfb1b5 s3: smbd: Ensure init_smb1_request() zeros out what the incoming pointer points to.
Remove the now unneeded req->xxx = NULL assignments (and the
deliberately bogus req->session = (void *)0xDEADBEEF one
used to demonstrate the bug).

Remove knownfail.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <npower@samba.org>

Autobuild-User(master): Noel Power <npower@samba.org>
Autobuild-Date(master): Tue Aug 15 12:06:36 UTC 2023 on atb-devel-224
2023-08-15 12:06:36 +00:00
Jeremy Allison
c32df3bb31 s3: torture: Add SMB1-NEGOTIATE-TCON that shows the SMB1 server crashes on the uninitialized req->session.
Found by Robert Morris <rtm@lcs.mit.edu>.

Adds knownfail.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <nopower@samba.org>
2023-08-15 11:12:32 +00:00
Jeremy Allison
d79d0508a4 s3: smbd: Add missing 'return;'s in exit paths in reply_exit_done().
Remove knownfail.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15430

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <npower@samba.org>

Autobuild-User(master): Noel Power <npower@samba.org>
Autobuild-Date(master): Mon Aug 14 19:52:49 UTC 2023 on atb-devel-224
2023-08-14 19:52:49 +00:00
Jeremy Allison
63895e03c4 s3: torture: Add a test doing an SMB1 negotiate+exit.
Robert Morris <rtm@lcs.mit.edu> noticed a missing
return in reply_exit_done().

Adds knownfail.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15430

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
2023-08-14 18:56:38 +00:00
Jeremy Allison
9220c45cc1 s3: smbd: Ensure srvstr_pull_req_talloc() always NULLs out *dest.
Robert Morris <rtm@lcs.mit.edu> noticed that in the case
where srvstr_pull_req_talloc() is being called with
buffer remaining == 0, we don't NULL out the destination
pointed which is *always* done in the codepaths inside
pull_string_talloc(). This prevents a crash in the caller.

Remove knownfail.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15420

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
2023-08-14 14:57:32 +00:00
Jeremy Allison
963fd8aa9b s3: torture: Add SMB1-TRUNCATED-SESSSETUP test.
Shows that we indirect through an uninitialized pointer and the client crashes
it's own smbd.

Add knownfail.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15420

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
2023-08-14 14:57:32 +00:00
Joseph Sutton
0cf658cd10 s4:kdc: Don’t issue forwardable or proxiable tickets to Protected Users
If an authentication policy enforces a maximum TGT lifetime for a
Protected User, that limit should stand in place of the four-hour limit
usually applied to Protected Users; we should nevertheless continue to
ensure that forwardable or proxiable tickets are not issued to such
users.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-14 04:57:34 +00:00
Joseph Sutton
7026b08e23 tests/krb5: Test that neither forwardable nor proxiable tickets are issued to Protected Users
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-14 04:57:34 +00:00
Joseph Sutton
f552933760 tests/krb5: Rename compatability_tests class
We should not have two unrelated classes both named SimpleKerberosTests.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-14 04:57:34 +00:00
Andrew Bartlett
b896da351c krb5: Increase the minimum MIT Krb5 version to 1.21
This is the version we test with in CI after the image update
in the next commit.  This addresses the issues that were
fixed in CVE-2022-37967 (KrbtgtFullPacSignature) and ensures
that Samba builds against the MIT version that allows us to
avoid that attack.

The hooks to allow these expectations to be disabled in the tests
are kept for now, to allow this to be reverted or to test
older servers.

With MIT 1.21 as the new test standard for the MIT KDC build
we update the knownfail_mit_kdc - this was required regadless
after the CI image update.

Any update to the CI image, even an unrelated one, brings in
a new MIT Krb5, version 1.21-3 in this case.  This has new
behaviour that needs to be noted in the knownfail files or
else the tests, which haven't changed, will fail and
pipelines won't pass.

(The image generated by the earlier bootstrap commit brought
in krb5-1.21-2 which was buggy with CVE-2023-39975)

Further tweaks to tests or the server should reduce the number
of knownfail entries, but this keeps the pipelines passing for now.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15231

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2023-08-14 03:46:35 +00:00
Andrew Bartlett
4ae3e9b208 selftest: Allow MIT Krb5 1.21 to still start to fl2000dc
This is the simplest way to keep this test environment alive.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2023-08-14 03:46:35 +00:00
Andrew Bartlett
79ca6ef28a s4-rpc_server/drsupai: Avoid looping with Azure AD Connect by not incrementing temp_highest_usn for the NC root
We send the NC root first, as a special case for every chunk
that we send until the natural point where it belongs.

We do not bump the tmp_highest_usn in the highwatermark that
the client and server use (it is meant to be an opauqe cookie)
until the 'natural' point where the object appears, similar
to the cache for GET_ANC.

The issue is that without this, because the NC root was sorted
first in whatever chunk it appeared in but could have a 'high'
highwatermark, Azure AD Connect will send back the same
new_highwatermark->tmp_highest_usn, and due to a bug,
a zero reserved_usn, which makes Samba discard it.

The reserved_usn is now much less likely to ever be set because
the tmp_higest_usn is now always advancing.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2023-08-13 21:59:29 +00:00
Andrew Bartlett
99579e7063 s4-rpc_server/drsuapi: Only keep and invalidate replication cycle state for normal replication
This changes the GetNCChanges server to use a per-call state for
extended operations like RID_ALLOC or REPL_OBJ and only maintain
and (more importantly) invalidate the state during normal replication.

This allows REPL_OBJ to be called during a normal replication cycle
that continues using after that call, continuing with the same
highwatermark cookie.

Azure AD will do a sequence of (roughly)

* Normal replication (objects 1..100)
* REPL_OBJ (of 1 object)
* Normal replication (objects 101..200)

However, if there are more than 100 (in this example) objects in the
domain, and the second replication is required, the objects 1..100
are sent, as the replication state was invalidated by the REPL_OBJ call.

RN: Improve GetNChanges to address some (but not all "Azure AD Connect")
syncronisation tool looping during the initial user sync phase.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2023-08-13 21:59:29 +00:00
Andrew Bartlett
8741495521 s4-torture/drs: Add test showing that if present in the set the NC root leads and tmp_highest_usn moves
The NC root, on any replication when it appears, is the first object to be
replicated, including for all subsequent chunks in the replication.

However the tmp_highest_usn is not updated by that USN, it must
only be updated for the non-NC changes (to match Windows exactly),
or at least only updated with the non-NC changes until it would
naturally appear.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2023-08-13 21:59:29 +00:00
Andrew Bartlett
b323169d6f s4-torture/drs: Add test demonstrating that a GetNCChanges REPL_OBJ will not reset the replication cookie
This demonstrates the behaviour used by the "Azure AD Connect" cloud sync tool.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15401

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2023-08-13 21:59:29 +00:00
Stefan Metzmacher
f348b84fbc s3:smbd: fix multichannel connection passing race
If a client opens multiple connection with the same
client guid in parallel, our connection passing is likely
to hit a race.

Assume we have 3 processes:

smbdA: This process already handles all connections for
       a given client guid
smbdB: This just received a new connection with an
       SMB2 neprot for the same client guid
smbdC: This also received a new connection with an
       SMB2 neprot for the same client guid

Now both smbdB and smbdC send a MSG_SMBXSRV_CONNECTION_PASS
message to smbdA. These messages contain the socket fd
for each connection.

While waiting for a MSG_SMBXSRV_CONNECTION_PASSED message
from smbdA, both smbdB and smbdC watch the smbXcli_client.tdb
record for changes (that also verifies smbdA stays alive).

Once one of them say smbdB received the MSG_SMBXSRV_CONNECTION_PASSED
message, the dbwrap_watch logic will wakeup smbdC in order to
let it recheck the smbXcli_client.tdb record in order to
handle the case where smbdA died or deleted its record.

Now smbdC rechecks the smbXcli_client.tdb record, but it
was not woken because of a problem with smbdA. It meant
that smbdC sends a MSG_SMBXSRV_CONNECTION_PASS message
including the socket fd again.

As a result smbdA got the socket fd from smbdC twice (or even more),
and creates two (or more) smbXsrv_connection structures for the
same low level tcp connection. And it also sends more than one
SMB2 negprot response. Depending on the tevent logic, it will
use different smbXsrv_connection structures to process incoming
requests. And this will almost immediately result in errors.

The typicall error is:
 smb2_validate_sequence_number: smb2_validate_sequence_number: bad message_id 2 (sequence id 2) (granted = 1, low = 1, range = 1)

But other errors would also be possible.

The detail that leads to the long delays on the client side is
that our smbd_server_connection_terminate_ex() code will close
only the fd of a single smbXsrv_connection, but the refcount
on the socket fd in the kernel is still not 0, so the tcp
connection is still alive...

Now we remember the server_id of the process that we send
the MSG_SMBXSRV_CONNECTION_PASS message to. And just keep
watching the smbXcli_client.tdb record if the server_id
don't change. As we just need more patience to wait for
the MSG_SMBXSRV_CONNECTION_PASSED message.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Tue Aug  8 13:59:58 UTC 2023 on atb-devel-224
2023-08-08 13:59:58 +00:00