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

4356 Commits

Author SHA1 Message Date
Ralph Boehme
6140c3177a smbd: fix share access check for overwrite dispostions
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15732

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

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Mon Oct 14 12:23:04 UTC 2024 on atb-devel-224
2024-10-14 12:23:04 +00:00
Ralph Boehme
849afe05ad smbtorture: add subtests for overwrite dispositions vs sharemodes
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15732

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2024-10-14 11:09:35 +00:00
Stefan Metzmacher
9263ce5752 dcesrv_core: better fault codes dcesrv_auth_prepare_auth3()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14356

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): Thu Oct 10 15:17:46 UTC 2024 on atb-devel-224
2024-10-10 15:17:46 +00:00
Stefan Metzmacher
8b8e4ff1b1 dcesrv_core: fix the auth3 for large ntlmssp messages
I know finding any real logic in reading the patch,
doesn't really show what's going on. I tried hard
to simplify it, but this is the only way I found
that fixed the test_auth_pad_ntlm_2889_auth3 test
without breaking other tests...

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
86808d66f3 gensec:spnego: ignore trailing bytes in SPNEGO_SERVER_START state
This matches Windows (at least Server 2012_R2).

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
55dd8bdb05 gensec:ntlmssp: only allow messages up to 2888 bytes
This matches Windows (at least Server 2012_R2).

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
31a422b7e5 dcesrv_core: alter_context logon failures should result in DCERPC_FAULT_ACCESS_DENIED
We should use DCERPC_FAULT_ACCESS_DENIED as default for
gensec status results of e.g. NT_STATUS_LOGON_FAILURE or
NT_STATUS_INVALID_PARAMTER.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
97545873eb dcesrv_core: a failure from gensec_update results in NAK_REASON_INVALID_CHECKSUM
We already report that for gensec_start_mech_by_authtype() failures,
but we also need to do that for any invalid authentication.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
85b2dcd938 dcerpc_util: let dcerpc_pull_auth_trailer() ignore data_and_pad for bind, alter, auth3
Sometimes Windows sends 3 presentation contexts (NDR32, NDR64,
BindTimeFeatureNegotiation) in the first BIND of an association.

Binding an additional connection to the association seems to
reuse the BIND buffer and just changes the num_contexts field from
3 to 2 and leaves the BindTimeFeatureNegotiation context as padding
in places.

Note, the auth_pad_length field is send as 0 in that case,
which means we need to ignore it completely, as well as any
padding before the auth header.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
7a6a1aae6f dcerpc_util: let dcerpc_pull_auth_trailer() expose the reject reason
If dcerpc_pull_auth_trailer() returns NT_STATUS_RPC_PROTOCOL_ERROR
it will return the BIND reject code in auth->auth_context_id.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
890fff1ca0 dcerpc_util: let dcerpc_pull_auth_trailer() check that auth_offset is 4 bytes aligned
That what Windows also asserts.

It also makes sure that ndr_pull_dcerpc_auth() will
start with ndr->offset = 0 and don't tries to eat
possible padding.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
f7a3827010 tests/dcerpc/raw_protocol: test invalid schannel binds
Note the ad_member will keep these as expected failures,
as it doesn't provide the netlogon service,
while the knownfail for the ADDC is only temporary.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
0bc562eb26 tests/dcerpc/raw_protocol: add more tests for auth_pad alignment
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14356

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
0da9e4d743 tests/dcerpc/raw_protocol: add tests for max auth_padding, auth_len or auth_offset
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14356

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
09e8dd23ce dcesrv_core: introduce dcesrv_connection->transport_max_recv_frag
The max fragment size depends on the transport.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
a2d894fd37 tests/dcerpc/raw_protocol: run test_neg_xmit_ffff_ffff over tcp and smb
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14356

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
31c2f35bba dcesrv_core: disconnect after a fault with non AUTH_LEVEL_CONNECT bind
Without an auth context using DCERPC_AUTH_LEVEL_PACKET or higher
the fault to reject requests with an invalid auth level
should trigger a disconnect after sending the fault to
the client.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
93bd5ba609 s4:selftest: only run ad_member with AUTH_LEVEL_CONNECT_LSA=1
We only want to test against
'allow dcerpc auth level connect:lsarpc = yes' once
in order to have the related code tests.
We use the ad_memeber for that special test and
use the default on the tested ADDC.

This reveals some knownfails, which will be fixed in
the next commit...

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
8c6b5b8743 s4:tortore/rpc: let rpc.backupkey without privacy pass against Windows 2022
The server disconnects after the first fault.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-10-10 14:01:04 +00:00
Stefan Metzmacher
dd5b9e08c7 s3:smbd: avoid false positives for got_oplock and have_other_lease in delay_for_oplock_fn
stat opens should not cause a oplock/lease downgrade if
they don't have a lease attached to itself.

Note that opens broken to NONE still count if they are
non-stat opens...

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

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): Thu Oct 10 13:59:18 UTC 2024 on atb-devel-224
2024-10-10 13:59:18 +00:00
Stefan Metzmacher
b1e5f5d8d2 s3:smbd: allow reset_share_mode_entry() to handle more than one durable handle
This means that multiple durable handles with RH leases can
co-exist now... Before only the last remaining durable handle
was able to pass the SMB_VFS_DURABLE_DISCONNECT() step.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-10-10 12:47:33 +00:00
Stefan Metzmacher
9e98cd5c7a s4:torture/smb2: add smb2.durable-v2-open.{keep,purge}-disconnected-* tests
These demonstrate which durables handles are kept and which are purged
because of various opens, writes or renames.

smb2.durable-v2-open.keep-disconnected-rh-with-stat-open
smb2.durable-v2-open.keep-disconnected-rh-with-rh-open
smb2.durable-v2-open.keep-disconnected-rh-with-rwh-open
smb2.durable-v2-open.keep-disconnected-rwh-with-stat-open

smb2.durable-v2-open.purge-disconnected-rwh-with-rwh-open
smb2.durable-v2-open.purge-disconnected-rwh-with-rh-open
smb2.durable-v2-open.purge-disconnected-rh-with-share-none-open
smb2.durable-v2-open.purge-disconnected-rh-with-write
smb2.durable-v2-open.purge-disconnected-rh-with-rename

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15649
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15651
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15708

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-10-10 12:47:33 +00:00
Stefan Metzmacher
77c7741f39 s4:torture/smb2: add smb2.durable-v2-open.{[non]stat[RH]-and,two-same,two-different}-lease
These show that it's possible to have durable handles in addition
of stat opens, as well as multiple durable opens with RH leases.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-10-10 12:47:33 +00:00
Stefan Metzmacher
0893ae8818 s3:smbd: only store durable handles with byte range locks when having WRITE lease
This simplifies the reconnect assumptions, when we want to allow
more than one durable handle on a file for multiple clients with
READ+HANDLE leases.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-10-10 12:47:33 +00:00
Stefan Metzmacher
8884d61731 s4:torture/smb2: add smb2.durable-v2-open.lock-{oplock,lease,noW-lease}
This demonstrates that a W lease is required for a
durable handle to be durable when it has byte range locks.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-10-10 12:47:33 +00:00
Stefan Metzmacher
1cc1586d84 s4:torture/smb2: add smb2.durable-open.lock-noW-lease
This demonstrates that a W lease is required for a
durable handle to be durable when it has byte range locks.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-10-10 12:47:33 +00:00
Andreas Schneider
dc6927fdca lib:ldb: Don't use RTLD_DEEPBIND by default
It should be off by default, as this is not needed by default. It
crashes named on startup, if bind is built with jemalloc support.

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

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2024-09-27 07:55:29 +00:00
Volker Lendecke
342d45eddb posix_ext: Pass unix file type through posix query_directory
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-09-26 15:22:46 +00:00
Volker Lendecke
83794d08bc tests: Check that we return the posix type in query_directory
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-09-26 15:22:46 +00:00
Volker Lendecke
529951ade5 smbd: Return file type in the posix create context response
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-09-26 15:22:46 +00:00
Volker Lendecke
3b63170f50 tests: Check that posix extensions return the file type
We'll need to check more, but this is a start

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-09-26 15:22:46 +00:00
Douglas Bagnall
c706021d55 ldb:tests: move api.py to api_misc
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-09-24 09:14:38 +00:00
Douglas Bagnall
081125f3b3 ldb:tests: make api_simple module
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-09-24 09:14:38 +00:00
Douglas Bagnall
565eba4120 ldb:tests: make api_add_modify module
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-09-24 09:14:38 +00:00
Douglas Bagnall
e4410ada21 ldb:tests: make api_search module
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-09-24 09:14:38 +00:00
Stefan Metzmacher
f9ee4db2ba s3:utils: use the correct secrets.tdb in net_use_krb_machine_account()
On a cluster we need to use the ctdb controlled database and not
a local secrets.tdb...

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Martin Schwenke <martin@meltin.net>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Fri Sep 20 05:54:43 UTC 2024 on atb-devel-224
2024-09-20 05:54:43 +00:00
Stefan Metzmacher
ab3fc1595c s3:utils: let 'net ads testjoin' fail without valid machine credentials
This will allow doing tests and make sure using anonymous credentials
doesn't cause false positive results...

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Martin Schwenke <martin@meltin.net>
2024-09-20 04:41:37 +00:00
Ralph Boehme
2ff3b9bc0d smbd: remove just created sharemode entry in the error codepaths
Without this we leave stale sharemode entries around that can lead to all sorts
of havoc.

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

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

Autobuild-User(master): Günther Deschner <gd@samba.org>
Autobuild-Date(master): Thu Sep 19 19:36:19 UTC 2024 on atb-devel-224
2024-09-19 19:36:19 +00:00
Stefan Metzmacher
14875448ca s3:tests: let test_durable_handle_reconnect.sh run smb2.durable-v2-regressions.durable_v2_reconnect_bug15624
This demonstrates the dead lock after a durable reconnect failed
because the stat info changed, the file can't be accessed anymore
as we leak the incomplete share mode entry in a still running
process.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
2024-09-19 18:27:33 +00:00
Stefan Metzmacher
ef4ef04e7f s4:torture/smb2: add smb2.durable-v2-regressions.durable_v2_reconnect_bug15624
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
2024-09-19 18:27:33 +00:00
Jeremy Allison
7da019d614 s4: torture: Add a new test lease_rename_with_overwrite.
Creates and opens two files with leases, then tries
rename-with-overwrite on file_src -> file_dst.

Ensures we get a lease break on file_dst before
getting the access denied response.

Passes against Windows, fails against Samba.

Add knownfail.

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

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Wed Sep 18 05:42:15 UTC 2024 on atb-devel-224
2024-09-18 05:42:15 +00:00
Ralph Boehme
ecb8a99a2c smbd: use metadata_fsp(fsp) in copy_access_posix_acl() for SMB_VFS_SYS_ACL_SET_FD
When inherting permissions on the created stream, we call into the VFS to fetch
the streams security descriptor via inherit_access_posix_acl() ->
copy_access_posix_acl() -> SMB_VFS_SYS_ACL_SET_FD() passing the stream fsp which
triggers the assert SMB_ASSERT(!fsp_is_alternate_stream(fsp)) in
vfswrap_sys_acl_set_fd() in vfs_default.

Just passing the base fsp to the VFS fixes this.

vfs_streams_depot which *does use* distinct backend filesystem files for the
streams, currently does not apply permissions to the stream files at all, so the
incomplete behaviour of vfs_streams_depot is not affected by this change.

If in the future someone want to fix this defficiency in vfs_streams_depot, the
module code can use fsp->stream_fsp to base decisions in VFS ops whether the
module should carry out some action.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>

Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Mon Sep  2 08:55:28 UTC 2024 on atb-devel-224
2024-09-02 08:55:28 +00:00
Ralph Boehme
0983560830 smbtorture: test creating stream doesn't crash when using "inherit permissions = yes"
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15695

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
2024-09-02 07:19:38 +00:00
David Disseldorp
5e278a5264 smb2_ioctl: fix truncated FSCTL_QUERY_ALLOCATED_RANGES responses
As per MS-FSA 2.1.5.10.22 FSCTL_QUERY_ALLOCATED_RANGES, if response
range entries exceed in_max_output, then we should respond with
STATUS_BUFFER_OVERFLOW and a truncated output buffer.

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

Reported-by: David Howells <dhowells@redhat.com>
Signed-off-by: David Disseldorp <ddiss@samba.org>
Reviewed-by: Noel Power <npower@samba.org>

Autobuild-User(master): David Disseldorp <ddiss@samba.org>
Autobuild-Date(master): Wed Aug 28 08:54:11 UTC 2024 on atb-devel-224
2024-08-28 08:54:11 +00:00
David Disseldorp
5cf57f1f53 s4:torture/smb2: test FSCTL_QUERY_ALLOCATED_RANGES truncation
FSCTL_QUERY_ALLOCATED_RANGES responses with more than one range should
be truncated to account for a ioctl.smb2.in.max_output_response limit.
Add a test for this.

Flag the new test knownfail; fix in subsequent commit.

Signed-off-by: David Disseldorp <ddiss@samba.org>
Reviewed-by: Noel Power <npower@samba.org>
2024-08-28 07:37:41 +00:00
Douglas Bagnall
7dac035896 s4:drs:test:getncchanges skips some tests with reserved_usn = 0
These tests are not affected by the reserved_usn change, so there is
no need to run them twice.

The test_repl_get_tgt_multivalued_links fails with or without
reserved_usn set to zero, but it fails differently in either case.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton@catalyst.net.nz>
2024-08-28 04:24:39 +00:00
Douglas Bagnall
7a623d8d56 s4:drsuapi:getncchanges: allow 0 reserved_usn reply
Azure AD will set reserved_usn to zero when we expect it to be
the number we gave them.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton@catalyst.net.nz>
2024-08-28 04:24:39 +00:00
Douglas Bagnall
67c7609ab7 s4:drs:tests: repeat getncchanges test with zero reserved_usn
This emulates the behaviour of Azure AD.

As this is quite slow we will later reduce the test load in this case,
but for now we want to run all the getncchanges tests this way.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton@catalyst.net.nz>
2024-08-28 04:24:39 +00:00
Stefan Metzmacher
7baeeece2d s3:smbd: let mkdir_internal() work more atomically using a temporary name
Currently we do this in mkdir_internal():

mkdirat(client_name);
if (EEXIST) {
   return EEXIST;
}
prepare_acls(client_name);

Note 'prepare_acls()' is a placeholder for the complex steps
it is doing to prepare the directory. During these steps
we have the problem that other clients already see
the directory and are able to create files or subdirectories
in it and these may not inherit the correct acls as
the their parent directory is not created completely.

I think I found a good strategie even without relying on
renameat2(RENAME_NOREPLACE).

We would do this instead:

tmp_name = ".::TMPNAME:D:$PID:client_name"
mkdirat(tmp_name, mode=client_mode);
prepare_acls(tmp_name);
mkdirat(client_name, mode=0);
if (EEXIST) {
   unlinkat(tmp_name);
   return EEXIST;
}
renameat(tmp_name, client_name);

So instead of having a long windows during prepare_acls,
we just have a short window between mkdirat(client_name, mode=0)
and renameat(tmp_name, client_name);
And in that short window the directory with the client_name
has a mode of 0, so it's not possible for other clients
to create files or subdirs in it.

As the mkdirat(client_name, mode=0) still catches
EEXIST the race where two clients try to create
the same client_name is closed as before,
so we don't need any other protection.

Following patches will make use of renameat2(RENAME_NOREPLACE),
but this already a very good improvement.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-08-21 08:02:30 +00:00
Stefan Metzmacher
f9d7a930f0 s4:torture/smb2: add smb2.create.mkdir-visible
This reproduces a race where one client creates
a directory and other clients see it before
the directory is fully setup including the correct
permissions and similar things.

We have a DENY ACE for SEC_DIR_ADD_FILE, which means
that files can't be created. This is set on
a base directory 'mkdir_visible'.

Then we have a lot of async loops trying to create
a file called 'mkdir_visible\dir\file_NR'. These loop
as fast as possible expecting OBJECT_PATH_NOT_FOUND,
because 'mkdir_visible\dir' is not there.

Then we send a create for 'mkdir_visible\dir' and
expect that to work.

This should turn the 'mkdir_visible\dir\file_NR' loop
into getting ACCESS_DENIED, because the
DENY ACE for SEC_DIR_ADD_FILE should be inherited
before 'mkdir_visible\dir' is visible to other clients.

Because of the complex steps in mkdir_internal(),
smbd allows the creation 'mkdir_visible\dir\file_NR',
as 'mkdir_visible\dir' is already visible after the
mkdirat(), before the DENY ACE is inherited.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-08-21 08:02:30 +00:00