diff options
author | Ralph Boehme <slow@samba.org> | 2022-07-27 13:37:32 +0200 |
---|---|---|
committer | Jule Anger <janger@samba.org> | 2022-09-06 07:54:13 +0000 |
commit | 3e6566222c98cb42d7d60b5a3507fb8e294a1482 (patch) | |
tree | 0090f970559de7875a0c3960ce24b7aed6c34d33 | |
parent | 00ce839865c7b08c57211817ae14009f02ee44f9 (diff) | |
download | samba-3e6566222c98cb42d7d60b5a3507fb8e294a1482.tar.gz |
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 23bc760ec5d61208c2d8778991e3d7e202eab352)
-rw-r--r-- | selftest/knownfail.d/samba3.blackbox.delete_stream | 1 | ||||
-rwxr-xr-x | selftest/target/Samba3.pm | 7 | ||||
-rwxr-xr-x | source3/script/tests/test_delete_stream.sh | 123 | ||||
-rwxr-xr-x | source3/selftest/tests.py | 1 |
4 files changed, 132 insertions, 0 deletions
diff --git a/selftest/knownfail.d/samba3.blackbox.delete_stream b/selftest/knownfail.d/samba3.blackbox.delete_stream new file mode 100644 index 00000000000..22c996aa63b --- /dev/null +++ b/selftest/knownfail.d/samba3.blackbox.delete_stream @@ -0,0 +1 @@ +^samba3.blackbox.delete_stream.delete stream\(fileserver\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 595be223dce..976afe89186 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -3255,6 +3255,13 @@ sub provision($$) copy = tmp vfs objects = streams_xattr xattr_tdb +[acl_streams_xattr] + copy = tmp + vfs objects = acl_xattr streams_xattr fake_acls xattr_tdb + acl_xattr:ignore system acls = yes + acl_xattr:security_acl_name = user.acl + xattr_tdb:ignore_user_xattr = yes + [compound_find] copy = tmp smbd:find async delay usec = 10000 diff --git a/source3/script/tests/test_delete_stream.sh b/source3/script/tests/test_delete_stream.sh new file mode 100755 index 00000000000..43d591ecfdf --- /dev/null +++ b/source3/script/tests/test_delete_stream.sh @@ -0,0 +1,123 @@ +#!/bin/sh +# +# this verifies that deleting a stream uses the correct ACL +# when using vfs_acl_xattr. +# + +if [ $# -lt 9 ]; then + echo "Usage: $0 SERVER SERVER_IP USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS NET SHARE" + exit 1 +fi + +SERVER="$1" +SERVER_IP="$2" +USERNAME="$3" +PASSWORD="$4" +PREFIX="$5" +SMBCLIENT="$6" +SMBCACLS="$7" +NET="$8" +SHARE="$9" + +SMBCLIENT="$VALGRIND ${SMBCLIENT}" +SMBCACLS="$VALGRIND ${SMBCACLS}" +NET="$VALGRIND ${NET}" + +incdir=$(dirname $0)/../../../testprogs/blackbox +. $incdir/subunit.sh + + +setup_testfile() +{ + touch $PREFIX/file + echo stream > $PREFIX/stream + + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "mkdir dir" || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "lcd $PREFIX; put file dir/file" || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "lcd $PREFIX; put stream dir/file:stream" || return 1 + + rm $PREFIX/file + rm $PREFIX/stream + + # + # Add full control ACE to the file and an ACL without "DELETE" on the + # parent directory + # + + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -S "ACL:Everyone:ALLOWED/0x0/0x1bf" dir || return 1 + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -a "ACL:Everyone:ALLOWED/0x0/0x101ff" dir/file || return 1 +} + +remove_testfile() +{ + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -S "ACL:Everyone:ALLOWED/0x0/0x101ff" dir/file || return 1 + $SMBCACLS //$SERVER/$SHARE -U $USERNAME%$PASSWORD -S "ACL:Everyone:ALLOWED/0x0/0x101ff" dir || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "rm dir/file" || return 1 + $SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "rmdir dir" || return 1 +} + +set_win_owner() +{ + local owner=$1 + + $SMBCACLS //$SERVER/$SHARE dir/file -U $USERNAME%$PASSWORD -C "$owner" || return 1 +} + +delete_stream() +{ + # + # Setup a file with a stream where we're not the owner and + # have delete rights. Bug 15126 would trigger a fallback to + # "acl_xattr:default acl style" because fetching the stored + # ACL would fail. The stored ACL allows deleting the stream + # but the synthesized default ACL does not, so the deletion + # of the stream should work, but it fails if we have the bug. + # + + # Now try deleting the stream + out=$($SMBCLIENT //$SERVER/$SHARE -U $USERNAME%$PASSWORD -c "wdel 0x20 dir/file:stream") || return 1 + + # + # Bail out in case we get any sort of NT_STATUS_* error, should be + # NT_STATUS_ACCESS_DENIED, but let's not slip through any other error. + # + echo "$out" | grep NT_STATUS_ && return 1 + + return 0 +} + +win_owner_is() +{ + local expected_owner=$1 + local actual_owner + + $SMBCACLS //$SERVER/$SHARE dir/file -U $USERNAME%$PASSWORD + actual_owner=$($SMBCACLS //$SERVER/$SHARE dir/file -U $USERNAME%$PASSWORD | sed -rn 's/^OWNER:(.*)/\1/p') + echo "actual_owner = $actual_owner" + if ! test "x$actual_owner" = "x$expected_owner"; then + echo "Actual owner of dir/file is $actual_owner', expected $expected_owner" + return 1 + fi + return 0 +} + +# Create a testfile +testit "create testfile" setup_testfile $SHARE || exit 1 + +# Grant SeRestorePrivilege to the user so we can change the owner +testit "grant SeRestorePrivilege" $NET rpc rights grant $USERNAME SeRestorePrivilege -U $USERNAME%$PASSWORD -I $SERVER_IP || exit 1 + +# We have SeRestorePrivilege, so both give and take ownership must succeed +testit "give owner with SeRestorePrivilege" set_win_owner "$SERVER\user1" || exit 1 +testit "verify owner" win_owner_is "$SERVER/user1" || exit 1 + +# Now try to remove the stream on the testfile +testit "delete stream" delete_stream $SHARE afile || exit 1 + +# Remove testfile +testit "remove testfile" remove_testfile $SHARE || exit 1 + +# Revoke SeRestorePrivilege, give ownership must fail now with NT_STATUS_INVALID_OWNER +testit "revoke SeRestorePrivilege" $NET rpc rights revoke $USERNAME SeRestorePrivilege -U $USERNAME%$PASSWORD -I $SERVER_IP || exit 1 + +exit 0 diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 3bc2b08f8a7..bec30df50b1 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -562,6 +562,7 @@ for env in ["fileserver"]: plantestsuite("samba3.blackbox.large_acl.NT1", env + "_smb1_done", [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls, '-m', 'NT1']) plantestsuite("samba3.blackbox.large_acl.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls, '-m', 'SMB3']) plantestsuite("samba3.blackbox.give_owner", env, [os.path.join(samba3srcdir, "script/tests/test_give_owner.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'tmp']) + plantestsuite("samba3.blackbox.delete_stream", env, [os.path.join(samba3srcdir, "script/tests/test_delete_stream.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'acl_streams_xattr']) plantestsuite("samba3.blackbox.homes", env, [os.path.join(samba3srcdir, "script/tests/test_homes.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', '$PREFIX', smbclient3, configuration]) plantestsuite("samba3.blackbox.force_group_change", env, [os.path.join(samba3srcdir, "script/tests/test_force_group_change.sh"), |