sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2025-01-11 05:18:09 +03:00
Code

CI: add a test trying to delete a stream on a pathref ("stat open") handle

Browse Source 
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)
This commit is contained in:
Ralph Boehme 2022-07-27 13:37:32 +02:00 committed by Jule Anger
parent 00ce839865
commit 3e6566222c
4 changed files with 132 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
selftest/knownfail.d/samba3.blackbox.delete_stream Normal file
View File

@ -0,0 +1 @@
^samba3.blackbox.delete_stream.delete stream\(fileserver\)

7
selftest/target/Samba3.pm
View File

@ -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

123
source3/script/tests/test_delete_stream.sh Executable file
View File

@ -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

1
source3/selftest/tests.py
View File

@ -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"),