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
Jones Syue [Mon, 7 Aug 2023 09:08:29 +0000 (17:08 +0800)]
mdssvc: Do an early talloc_free() in _mdssvc_open()
Environment setup:
When macOS Finder connect to a samba server with 'spotlight = yes',
macOS would issue mdssvc open (mdssvc.opnum == 0) to samba and it goes
through api _mdssvc_open().
After applied 578e434a94147dc2d7dbfc006d2ab84807859c1d,
(this is reported by jaywei@qnap.com)
this line 'talloc_free(path);' is deleted if _mdssvc_open() normal exit,
so memory is lazy de-allocate: delayed to
smbd_tevent_trace_callback() @ smb2_process.c. [1]
Supposed to explicitly free 'path' in _mdssvc_open() @ srv_mdssvc_nt.c[2]
just like abnormal exit, do not wait for main loop to free 'path' which is
no longer used, this is more consistent while reading source code.
[1] gdb tracing 'path' address 0x56204ccc67e0 to know how it is freed.
Breakpoint 2, _tc_free_children_internal (tc=0x56204ccc6780, ptr=0x56204ccc67e0, location=0x7ff430d96410 "../../lib/talloc/talloc.c:1714") at ../../lib/talloc/talloc.c:1656
1656 while (tc->child) {
(gdb) bt
0 _tc_free_children_internal (tc=0x56204ccc6780, ptr=0x56204ccc67e0, location=0x7ff430d96410 "../../lib/talloc/talloc.c:1714") at ../../lib/talloc/talloc.c:1656
1 0x00007ff430d92b14 in _tc_free_internal (tc=0x56204ccc6780, location=0x7ff430d96410 "../../lib/talloc/talloc.c:1714") at ../../lib/talloc/talloc.c:1183
2 0x00007ff430d93b71 in _tc_free_children_internal (tc=0x56204ccc6720, ptr=0x56204ccc6780, location=0x7ff430d96410 "../../lib/talloc/talloc.c:1714") at ../../lib/talloc/talloc.c:1668
3 0x00007ff430d93d66 in talloc_free_children (ptr=0x56204ccc6780) at ../../lib/talloc/talloc.c:1714
4 0x00007ff432235aca in talloc_pop (frame=0x56204ccc6780) at ../../lib/util/talloc_stack.c:125
5 0x00007ff430d92959 in _tc_free_internal (tc=0x56204ccc6720, location=0x7ff431f358d0 "../../source3/smbd/process.c:3726") at ../../lib/talloc/talloc.c:1157
6 0x00007ff430d92cd5 in _talloc_free_internal (ptr=0x56204ccc6780, location=0x7ff431f358d0 "../../source3/smbd/process.c:3726") at ../../lib/talloc/talloc.c:1247
7 0x00007ff430d93f96 in _talloc_free (ptr=0x56204ccc6780, location=0x7ff431f358d0 "../../source3/smbd/process.c:3726") at ../../lib/talloc/talloc.c:1791
8 0x00007ff431d81292 in smbd_tevent_trace_callback (point=TEVENT_TRACE_AFTER_LOOP_ONCE, private_data=0x7ffe46591e30) at ../../source3/smbd/process.c:3726
<...cut...>
[2] gdb tracing 'path' address 0x55a6d66deed0 to know how it is freed.
Breakpoint 2, _tc_free_children_internal (tc=0x55a6d66deed0, ptr=0x55a6d66def30, location=0x7fc4cca84040 "../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:189") at ../../lib/talloc/talloc.c:1656
1656 while (tc->child) {
(gdb) bt
0 _tc_free_children_internal (tc=0x55a6d66deed0, ptr=0x55a6d66def30, location=0x7fc4cca84040 "../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:189") at ../../lib/talloc/talloc.c:1656
1 0x00007fc4cb892b14 in _tc_free_internal (tc=0x55a6d66deed0, location=0x7fc4cca84040 "../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:189") at ../../lib/talloc/talloc.c:1183
2 0x00007fc4cb892cd5 in _talloc_free_internal (ptr=0x55a6d66def30, location=0x7fc4cca84040 "../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:189") at ../../lib/talloc/talloc.c:1247
3 0x00007fc4cb893f96 in _talloc_free (ptr=0x55a6d66def30, location=0x7fc4cca84040 "../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:189") at ../../lib/talloc/talloc.c:1791
4 0x00007fc4cc9396e4 in _mdssvc_open (p=0x55a6d66d5600, r=0x55a6d66edc60) at ../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:189
<...cut...>
Signed-off-by: Jones Syue <jonessyue@qnap.com> Reviewed-by: Noel Power <npower@samba.org> Reviewed-by: Volker Lendecke <vl@samba.org>
Autobuild-User(master): Noel Power <npower@samba.org>
Autobuild-Date(master): Mon Aug 14 18:11:37 UTC 2023 on atb-devel-224
Jeremy Allison [Fri, 11 Aug 2023 17:42:41 +0000 (10:42 -0700)]
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.
Joseph Sutton [Thu, 10 Aug 2023 22:54:52 +0000 (10:54 +1200)]
s4:kdc: Add get_claims_set_for_principal()
Add a new function, get_claims_set_for_principal(), that returns the
claims as a CLAIMS_SET structure rather than as a blob. To accommodate
this, move the call to encode_claims_set() out of get_all_claims() and
into get_claims_blob_for_principal().
Being able to get the unencoded claims will save us from having to
decode claims that we just needlessly encoded.
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): Mon Aug 14 05:51:45 UTC 2023 on atb-devel-224
Joseph Sutton [Thu, 10 Aug 2023 22:15:56 +0000 (10:15 +1200)]
s4:kdc: Don’t enforce a server authentication policy for the krbtgt
As the server authentication policy will be non-NULL only for entries
looked up as servers, the krbtgt shouldn’t have an authentication policy
anyway. But we might as well be explicit.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Joseph Sutton [Thu, 10 Aug 2023 22:02:28 +0000 (10:02 +1200)]
s4:kdc: Return (possibly) more appropriate error codes
This change ultimately won’t make much difference to responses, as
unrecognized codes are mapped to ERR_GENERIC in any case. But it might
provide some help for debugging.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Joseph Sutton [Thu, 10 Aug 2023 04:52:13 +0000 (16:52 +1200)]
s4:kdc: Ensure the value of h->len is accurate
If we exited this function early due to an error, h->len would contain
the number of elements that *ought* to be in h->val, but not all of
those elements must have been initialized. Subsequently trying to free
this partially-uninitialized structure with free_Keys() could have bad
results.
Avoid this by ensuring that h->len accurately reports the actual number
of initialized elements.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Joseph Sutton [Thu, 10 Aug 2023 04:35:52 +0000 (16:35 +1200)]
s4:kdc: Consistently zero HDB structures
To these conversion functions we sometimes pass malloc-allocated HDB
structures, which we free afterwards if conversion fails. If parts of
these structures are still uninitialized when we try to free them, all
sorts of fun things can result.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Joseph Sutton [Thu, 10 Aug 2023 04:07:17 +0000 (16:07 +1200)]
s4:kdc: Fix leaks
Use a temporary context to allocate these variables. Each variable that
needs to be transferred to the caller is stolen onto an appropriate
talloc context just prior to the function’s returning.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Joseph Sutton [Thu, 10 Aug 2023 02:37:34 +0000 (14:37 +1200)]
s4:kdc: Initialize entry->modified_by
If smb_krb5_make_principal() fails without setting the principal,
sdb_entry_free() will try to free whatever memory the uninitialized
member points to.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Joseph Sutton [Tue, 8 Aug 2023 22:47:08 +0000 (10:47 +1200)]
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>
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Mon Aug 14 04:40:03 UTC 2023 on atb-devel-224
Andrew Bartlett [Tue, 18 Jul 2023 02:34:47 +0000 (14:34 +1200)]
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.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Volker Lendecke <vl@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Sun Aug 13 22:54:55 UTC 2023 on atb-devel-224
Andrew Bartlett [Wed, 26 Jul 2023 02:27:16 +0000 (14:27 +1200)]
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.
Andrew Bartlett [Tue, 27 Jun 2023 02:39:18 +0000 (14:39 +1200)]
s4-rpc_server/drsuapi: Avoid modification to ncRoot input variable in GetNCChanges
This tries to avoid it appearing that ncRoot is a value that can
be trusted and used internally by not updating it and instead leaving
it just as an input/echo-back value.
Andrew Bartlett [Mon, 26 Jun 2023 04:53:10 +0000 (16:53 +1200)]
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.
Andrew Bartlett [Mon, 24 Jul 2023 00:05:18 +0000 (12:05 +1200)]
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.
projects / thirdparty / samba.git / log
