1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-25 23:21:54 +03:00
Commit Graph

128965 Commits

Author SHA1 Message Date
Ralph Boehme
11947a8e59 smbd: use metadata_fsp() with SMB_VFS_FSET_NT_ACL()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126
MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 4ab29e2a34)
2022-09-06 07:54:13 +00:00
Ralph Boehme
9823e91999 smbd: use metadata_fsp() with SMB_VFS_FGET_NT_ACL()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126
MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(backported from commit c949e4b2a4)
[slow@samba.org: context mismatch due to smbd_check_access_rights_fname() call in master]
2022-09-06 07:54:13 +00:00
Ralph Boehme
3e6566222c CI: add a test trying to delete a stream on a pathref ("stat open") handle
When using vfs_streams_xattr, for a pathref handle of a stream the system fd
will be a fake fd created by pipe() in vfs_fake_fd().

For the following callchain we wrongly pass a stream fsp to
SMB_VFS_FGET_NT_ACL():

SMB_VFS_CREATE_FILE(..., "file:stream", ...)
=> open_file():
   if (open_fd):
   -> taking the else branch:
   -> smbd_check_access_rights_fsp(stream_fsp)
      -> SMB_VFS_FGET_NT_ACL(stream_fsp)

This is obviously wrong and can lead to strange permission errors when using
vfs_acl_xattr:

in vfs_acl_xattr we will try to read the stored ACL by calling
fgetxattr(fake-fd) which of course faild with EBADF. Now unfortunately the
vfs_acl_xattr code ignores the specific error and handles this as if there was
no ACL stored and subsequently runs the code to synthesize a default ACL
according to the setting of "acl:default acl style".

As the correct access check for streams has already been carried out by calling
check_base_file_access() from create_file_unixpath(), the above problem is not
a security issue: it can only lead to "decreased" permissions resulting in
unexpected ACCESS_DENIED errors.

The fix is obviously going to be calling
smbd_check_access_rights_fsp(stream_fsp->base_fsp).

This test verifies that deleting a file works when the stored NT ACL grants
DELETE_FILE while the basic POSIX permissions (used in the acl_xattr fallback
code) do not.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126
MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 23bc760ec5)
2022-09-06 07:54:13 +00:00
Ralph Boehme
00ce839865 vfs_xattr_tdb: add "xattr_tdb:ignore_user_xattr" option
Allows passing on "user." xattr to the backend. This can be useful for testing
specific aspects of operation on streams when "streams_xattr" is configured as
stream filesystem backend.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126
MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 92e0045d7c)
2022-09-06 07:54:13 +00:00
Ralph Boehme
6d66f43229 vfs_xattr_tdb: add a module config
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126
MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 451ad315a9)
2022-09-06 07:54:13 +00:00
Ralph Boehme
b83ff1252e vfs_xattr_tdb: move close_xattr_db()
This just makes the diff of the next commit smaller and easier to digest.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126
MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit b26dc252aa)
2022-09-06 07:54:13 +00:00
Ralph Boehme
56ab836157 smdb: use fsp_is_alternate_stream() in open_file()
No change in behaviour.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15126
MR: https://gitlab.com/samba-team/samba/-/merge_requests/2643

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 0d3995cec1)
2022-09-06 07:54:13 +00:00
Volker Lendecke
20a425fcde smbd: Introduce metadata_fsp()
Centralize the pattern

if (fsp->base_fsp != NULL) {
	fsp = fsp->base_fsp;
}

with a descriptive name.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
(backported from commit ac58b0b942)
[slow@samba.org: only backport the function, skip all updated callers]
2022-09-06 07:54:13 +00:00
Volker Lendecke
3160ff28e8 smbd: Introduce fsp_is_alternate_stream()
To me this is more descriptive than "fsp->base_fsp != NULL". If this
turns out to be a performance problem, I would go and make this a
static inline in smbd/proto.h.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
(backported from commit 21b380ca13)
[slow@samba.org: only backport the function, skip all changed callers]
2022-09-06 07:54:13 +00:00
Andreas Schneider
df7d6f0c48 lib:replace: Only include <sys/mount.h> on non-Linux systems
Details at:
https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E

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

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit 766151bf5b)

Autobuild-User(v4-16-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-16-test): Tue Aug 23 08:53:41 UTC 2022 on sn-devel-184
2022-08-23 08:53:41 +00:00
Jeremy Allison
ce464a83c7 s3: smbd: Plumb close_type parameter through close_file_in_loop(), file_close_conn()
Allows close_file_in_loop() to differentiate between SHUTDOWN_CLOSE
(previously it only used this close type) and ERROR_CLOSE - called
on error from smbXsrv_tcon_disconnect() in the error path. In that
case we want to close the fd, but not run any delete-on-close actions.

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

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

Autobuild-User(master): Noel Power <npower@samba.org>
Autobuild-Date(master): Thu Aug 18 14:10:18 UTC 2022 on sn-devel-184

(cherry picked from commit cf5f7b1489)
2022-08-23 07:45:16 +00:00
Jeremy Allison
a5cf33d404 s3: smbd: Add "enum file_close_type close_type" parameter to file_close_conn().
Not yet used.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
(cherry picked from commit 7005a6354d)
[npower@samba.org Adjusted for 4.15 filename change
         smb2-service.c -> service.c]
2022-08-23 07:45:16 +00:00
Jeremy Allison
706c64c6f0 s3: smbd: Add "enum file_close_type close_type" parameter to close_cnum().
Not yet used, but needed so we can differentiate between
SHUTDOWN_CLOSE and ERROR_CLOSE in smbXsrv_tcon_disconnect()
if we fail to chdir. In that case we want to close the fd,
but not run any delete-on-close actions.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
(cherry picked from commit 9203d17106)
[npower@samba.org Adjusted for 4.15 filename change
         smb2-service.c -> service.c]
2022-08-23 07:45:16 +00:00
Jeremy Allison
d1bc0d0b51 s3/smbd: Use after free when iterating smbd_server_connection->connections
Change conn_free() to just use a destructor. We now
catch any other places where we may have forgetten to
call conn_free() - it's implicit on talloc_free(conn).

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

Based on code from Noel Power <noel.power@suse.com>.

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): Wed Aug 17 09:54:06 UTC 2022 on sn-devel-184

(cherry picked from commit f92bacbe21)
2022-08-23 07:45:16 +00:00
Jeremy Allison
56e1a9fc62 s3/smbd: Use after free when iterating smbd_server_connection->connections
In SMB2 smbd_smb2_tree_connect() we create a new conn struct
inside make_connection_smb2() then move the ownership to tcon using:

        tcon->compat = talloc_move(tcon, &compat_conn);

so the lifetime of tcon->compat is tied directly to tcon.

Inside smbXsrv_tcon_disconnect() we have:

 908                 ok = chdir_current_service(tcon->compat);
 909                 if (!ok) {
 910                         status = NT_STATUS_INTERNAL_ERROR;
 911                         DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): "
 912                                   "chdir_current_service() failed: %s\n",
 913                                   tcon->global->tcon_global_id,
 914                                   tcon->global->share_name,
 915                                   nt_errstr(status)));
 916                         tcon->compat = NULL;
 917                         return status;
 918                 }
 919
 920                 close_cnum(tcon->compat, vuid);
 921                 tcon->compat = NULL;

If chdir_current_service(tcon->compat) fails, we return status without ever having
called close_cnum(tcon->compat, vuid), leaving the conn pointer left in the linked
list sconn->connections.

The caller frees tcon and (by ownership) tcon->compat, still leaving the
freed tcon->compat pointer on the sconn->connections linked list.

When deadtime_fn() fires and walks the sconn->connections list it
indirects this freed pointer. We must call close_cnum() on error also.

Valgrind trace from Noel Power <noel.power@suse.com> is:

==6432== Invalid read of size 8
==6432==    at 0x52CED3A: conn_lastused_update (conn_idle.c:38)
==6432==    by 0x52CEDB1: conn_idle_all (conn_idle.c:54)
==6432==    by 0x5329971: deadtime_fn (smb2_process.c:1566)
==6432==    by 0x5DA2339: smbd_idle_event_handler (util_event.c:45)
==6432==    by 0x685F2F8: tevent_common_invoke_timer_handler (tevent_timed.c:376)

==6432==  Address 0x19074b88 is 232 bytes inside a block of size 328 free'd
==6432==    at 0x4C3451B: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6432==    by 0x5B38521: _tc_free_internal (talloc.c:1222)
==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
==6432==    by 0x5B385C5: _talloc_free_internal (talloc.c:1248)
==6432==    by 0x5B3988D: _talloc_free (talloc.c:1792)
==6432==    by 0x5349B22: smbd_smb2_flush_send_queue (smb2_server.c:4828)

==6432==  Block was alloc'd at
==6432==    at 0x4C332EF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6432==    by 0x5B378D9: __talloc_with_prefix (talloc.c:783)
==6432==    by 0x5B37A73: __talloc (talloc.c:825)
==6432==    by 0x5B37E0C: _talloc_named_const (talloc.c:982)
==6432==    by 0x5B3A8ED: _talloc_zero (talloc.c:2421)
==6432==    by 0x539873A: conn_new (conn.c:70)
==6432==    by 0x532D692: make_connection_smb2 (smb2_service.c:909)
==6432==    by 0x5352B5E: smbd_smb2_tree_connect (smb2_tcon.c:344)

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
(cherry picked from commit 0bdfb5a5e6)
2022-08-23 07:45:16 +00:00
Stefan Metzmacher
9cb4043727 s3:smbd: only clear LEASE_READ if there's no read lease is left
If contend_level2_oplocks_begin_default() skips break it's
own lease, we should not clear SHARE_MODE_LEASE_READ
in share_mode_data->flags.

Otherwise that lease won't see any lease break notifications
for writes from other clients (file handles not using the same lease
key).

So we need to count the number existing read leases (including
the one with the same lease key) in order to know it's
safe to clear SMB2_LEASE_READ/SHARE_MODE_LEASE_READ.

Otherwise the next run (likely from another client)
will get the wrong result from file_has_read_lease().

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

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

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Thu Aug 18 19:41:33 UTC 2022 on sn-devel-184

(cherry picked from commit 96e2a82760)
2022-08-23 07:45:16 +00:00
Stefan Metzmacher
b910d9f6e0 s4:torture/smb2: add smb2.lease.v[1,2]_bug_15148
This demonstrates the bug that happens with a
write to a file handle holding an R lease,
while there are other openers without any lease.

When one of the other openers writes to the file,
the R lease of the only lease holder isn't broken to NONE.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit 9e5ff607eb)
2022-08-23 07:45:16 +00:00
Stefan Metzmacher
19f285e080 s3:smbd: share_mode_flags_set() takes SMB2_LEASE_* values
We currently only ever pass SMB2_LEASE_READ and both
have the same value of 0x1, so for now it's only cosmetic,
but that will change soon.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit 7592aad4d7)
2022-08-23 07:45:16 +00:00
Joseph Sutton
f6afc5b35e libcli/smb: Set error status if 'iov' pointer is NULL
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15152

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Mon Aug 22 09:03:29 UTC 2022 on sn-devel-184

(cherry picked from commit 75e03ea021)
2022-08-23 07:45:16 +00:00
Joseph Sutton
f33ad1c172 libcli/smb: Ensure we call tevent_req_nterror() on failure
Commit 3594c3ae20 added a NULL check for
'inhdr', but it meant we didn't always call tevent_req_nterror() when we
should.

Now we handle connection errors. We now also set an error status if the
NULL check fails.

I noticed this when an ECONNRESET error from a server refusing SMB1
wasn't handled, and the client subsequently hung in epoll_wait().

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 40d4912d84)
2022-08-23 07:45:16 +00:00
Michael Tokarev
b75b5f60ba s3/util/py_net.c: fix samba-tool domain join&leave segfault
We process python args using PyArg_ParseTupleAndKeywords(), and use "p"
type modifier there.  According to documentation, this type modifier,
while works for a boolean type, expects an argument of type int. But in
py_net_join_member() and  py_net_leave() we use argument of type uint8_t
(no_dns_update, keep_account, r->in.debug). So when PyArg_ParseTupleAndKeywords()
tries to assign a value to &no_dns_update, it updates subsequent, unrelated bytes
too, - which ones depends on the stack and structure layout used by the compiler.

Fix this by using int type for all relevant variables, and by introducing proxy
variable "debug" (of the same type) for r->in.debug.

While at it, also ensure all variables have sensible default values.

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

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Wed May 25 06:19:32 UTC 2022 on sn-devel-184

(backported from commit 976326fa2b,
 cherry-pick -x with BUG: line added)

Autobuild-User(v4-16-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-16-test): Mon Aug  8 10:32:22 UTC 2022 on sn-devel-184
2022-08-08 10:32:22 +00:00
Andreas Schneider
529e86163a s3:rpcclient: Goto done in cmd_samr_setuserinfo_int()
We need to free the frame or we will run into:
    smb_panic (why=0x7fa8c511aa88 "Frame not freed in order.")

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

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
(cherry picked from commit 2b32d93222)

Autobuild-User(v4-16-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-16-test): Sun Jul 31 19:14:59 UTC 2022 on sn-devel-184
2022-07-31 19:14:59 +00:00
Ralph Boehme
e10ce26dcb mdssvc: return all-zero policy handle if spotlight is disabled
A Mac SMB server returns an all zero handle and an empty path if Spotlight is
disabled on a share. We must return the exact same error return in order to
trigger client-side searching.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15086
pcap: https://www.samba.org/~slow/pcaps/mac-bigsur-smbserver-spotlight-disabled.pcapng.gz

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Noel Power <npower@samba.org>

Autobuild-User(master): Noel Power <npower@samba.org>
Autobuild-Date(master): Tue Jul 12 15:42:52 UTC 2022 on sn-devel-184

(cherry picked from commit 23e6e50c0f)
2022-07-31 18:09:11 +00:00
Ralph Boehme
5598ddaaf4 CI: fix check for correct mdsvc resonse when connecting to a share with Spotlight disabled
A Mac SMB server returns an all zero handle and an empty path if Spotlight is
disabled on a share. We must return the exact same error return in order to
trigger client-side searching.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15086
pcap: https://www.samba.org/~slow/pcaps/mac-bigsur-smbserver-spotlight-disabled.pcapng.gz

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
(backported from commit 8e997bd6e9)
[slow@samba.org: conflicts in tests.py caused by unrelated tests]
2022-07-31 18:09:11 +00:00
Ralph Boehme
5fd138ad80 mdssvc: convert mds_init_ctx() to return NTSTATUS
No change in behavour. In preperation for returning a special error to signal
the caller that spotlight is disabled for a share.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15086

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
(cherry picked from commit 72468166b2)
2022-07-31 18:09:11 +00:00
Jule Anger
ae0977177b VERSION: Bump version up to Samba 4.16.5...
and re-enable GIT_SNAPSHOT.

Signed-off-by: Jule Anger <janger@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
2022-07-27 12:49:29 +02:00
Jule Anger
9618af1b66 VERSION: Disable GIT_SNAPSHOT for the 4.16.4 release.
Signed-off-by: Jule Anger <janger@samba.org>
2022-07-24 11:46:16 +02:00
Jule Anger
b0ad1276b5 WHATSNEW: Add release notes for Samba 4.16.4.
Signed-off-by: Jule Anger <janger@samba.org>
2022-07-24 11:32:44 +02:00
Jeremy Allison
74946420dd CVE-2022-32742: s3: smbd: Harden the smbreq_bufrem() macro.
Fixes the raw.write.bad-write test.

NB. We need the two (==0) changes in source3/smbd/reply.c
as the gcc optimizer now knows that the return from
smbreq_bufrem() can never be less than zero.

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

Remove knownfail.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: David Disseldorp <ddiss@samba.org>
2022-07-24 09:29:26 +02:00
Jeremy Allison
ed3f82f4d7 CVE-2022-32742: s4: torture: Add raw.write.bad-write test.
Reproduces the test code in:

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

Add knownfail.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: David Disseldorp <ddiss@samba.org>
2022-07-24 09:29:26 +02:00
Joseph Sutton
e650b41ff9 CVE-2022-2031 testprogs: Add test for short-lived ticket across an incoming trust
We ensure that the KDC does not reject a TGS-REQ with our short-lived
TGT over an incoming trust.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2022-07-24 09:23:56 +02:00
Joseph Sutton
a46dd2846f CVE-2022-2031 s4:kpasswd: Do not accept TGTs as kpasswd tickets
If TGTs can be used as kpasswd tickets, the two-minute lifetime of a
authentic kpasswd ticket may be bypassed. Furthermore, kpasswd tickets
are not supposed to be cached, but using this flaw, a stolen credentials
cache containing a TGT may be used to change that account's password,
and thus is made more valuable to an attacker.

Since all TGTs should be issued with a REQUESTER_SID PAC buffer, and
service tickets without it, we assert the absence of this buffer to
ensure we're not accepting a TGT.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>

[jsutton@samba.org Fixed knownfail conflicts]
2022-07-24 09:23:56 +02:00
Joseph Sutton
8c0f421852 CVE-2022-2031 s4:auth: Use PAC to determine whether ticket is a TGT
We use the presence or absence of a REQUESTER_SID PAC buffer to
determine whether the ticket is a TGT. We will later use this to reject
TGTs where a service ticket is expected.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
9895018b64 CVE-2022-2031 auth: Add ticket type field to auth_user_info_dc and auth_session_info
This field may be used to convey whether we were provided with a TGT or
a non-TGT. We ensure both structures are zeroed out to avoid incorrect
results being produced by an uninitialised field.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
ff66f68a11 CVE-2022-2031 tests/krb5: Add test that we cannot provide a TGT to kpasswd
The kpasswd service should require a kpasswd service ticket, and
disallow TGTs.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>

[jsutton@samba.org Fixed knownfail conflicts]
2022-07-24 09:23:56 +02:00
Joseph Sutton
7ee246ef9c CVE-2022-32744 s4:kpasswd: Ensure we pass the kpasswd server principal into krb5_rd_req_ctx()
To ensure that, when decrypting the kpasswd ticket, we look up the
correct principal and don't trust the sname from the ticket, we should
pass the principal name of the kpasswd service into krb5_rd_req_ctx().
However, gensec_krb5_update_internal() will pass in NULL unless the
principal in our credentials is CRED_SPECIFIED.

At present, our principal will be considered obtained as CRED_SMB_CONF
(from the cli_credentials_set_conf() a few lines up), so we explicitly
set the realm again, but this time as CRED_SPECIFIED. Now the value of
server_in_keytab that we provide to smb_krb5_rd_req_decoded() will not
be NULL.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
c9e1949fa8 CVE-2022-32744 s4:kdc: Modify HDB plugin to only look up kpasswd principal
This plugin is now only used by the kpasswd service. Thus, ensuring we
only look up the kadmin/changepw principal means we can't be fooled into
accepting tickets for other service principals. We make sure not to
specify a specific kvno, to ensure that we do not accept RODC-issued
tickets.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>

[jsutton@samba.org Fixed knownfail conflicts]
2022-07-24 09:23:56 +02:00
Joseph Sutton
fa198ce28f s4:kdc: Remove kadmin mode from HDB plugin
It appears we no longer require it.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
d03021791b CVE-2022-32744 s4:kdc: Rename keytab_name -> kpasswd_keytab_name
This makes explicitly clear the purpose of this keytab.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
0cb4100d16 CVE-2022-2031 s4:kdc: Don't use strncmp to compare principal components
We would only compare the first 'n' characters, where 'n' is the length
of the principal component string, so 'k@REALM' would erroneously be
considered equal to 'krbtgt@REALM'.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
1f54e16cf1 CVE-2022-2031 tests/krb5: Test truncated forms of server principals
We should not be able to use krb@REALM instead of krbtgt@REALM.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
8d8ffbfc7b CVE-2022-32744 s4:kdc: Don't allow HDB keytab iteration
A fallback in krb5_rd_req_ctx() means that Samba's kpasswd service will
try many inappropriate keys to decrypt the ticket supplied to it. For
example, it will accept a ticket encrypted with the Administrator's key,
when it should rather accept only tickets encrypted with the krbtgt's
key (and not an RODC krbtgt). To fix this, declare the HDB keytab using
the HDBGET ops, which do not support iteration.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
90e53b8eae CVE-2022-2031 s4:kdc: Reject tickets during the last two minutes of their life
For Heimdal, this now matches the behaviour of Windows. The object of
this requirement is to ensure we don't allow kpasswd tickets, not having
a lifetime of more than two minutes, to be passed off as TGTs.

An existing requirement for TGTs to contain a REQUESTER_SID PAC buffer
suffices to prevent kpasswd ticket misuse, so this is just an additional
precaution on top.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
b77fb6e636 CVE-2022-2031 third_party/heimdal: Add function to get current KDC time
This allows the plugin to check the endtime of a ticket against the
KDC's current time, to see if the ticket will expire in the next two
minutes.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2022-07-24 09:23:56 +02:00
Joseph Sutton
f70ada5eb4 CVE-2022-2031 s4:kdc: Limit kpasswd ticket lifetime to two minutes or less
This matches the behaviour of Windows.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>

[jsutton@samba.org Adapted entry to entry_ex->entry; included
 samba_kdc.h header file]
2022-07-24 09:23:56 +02:00
Joseph Sutton
fb7391ca60 CVE-2022-2031 s4:kdc: Fix canonicalisation of kadmin/changepw principal
Since this principal goes through the samba_kdc_fetch_server() path,
setting the canonicalisation flag would cause the principal to be
replaced with the sAMAccountName; this meant requests to
kadmin/changepw@REALM would result in a ticket to krbtgt@REALM. Now we
properly handle canonicalisation for the kadmin/changepw principal.

View with 'git show -b'.

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

Pair-Programmed-With: Andreas Schneider <asn@samba.org>
Signed-off-by: Andreas Schneider <asn@samba.org>
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>

[jsutton@samba.org Adapted entry to entry_ex->entry; removed MIT KDC
 1.20-specific knownfails]
2022-07-24 09:23:56 +02:00
Joseph Sutton
2b63f021e5 CVE-2022-2031 s4:kdc: Refactor samba_kdc_get_entry_principal()
This eliminates some duplicate branches.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Pair-Programmed-With: Andreas Schneider <asn@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2022-07-24 09:23:56 +02:00
Joseph Sutton
9022a69aeb CVE-2022-2031 s4:kdc: Split out a samba_kdc_get_entry_principal() function
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>

[jsutton@samba.org Adapted entry to entry_ex->entry]
2022-07-24 09:23:56 +02:00
Andreas Schneider
ada799129e CVE-2022-2031 s4:kdc: Implement is_kadmin_changepw() helper function
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>

[jsutton@samba.org Adapted entry to entry_ex->entry]
2022-07-24 09:23:56 +02:00
Andreas Schneider
4aafa72991 CVE-2022-2031 testprogs: Add kadmin/changepw canonicalization test with MIT kpasswd
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2022-07-24 09:23:56 +02:00