summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRalph Boehme <slow@samba.org>2022-07-27 13:37:32 +0200
committerJule Anger <janger@samba.org>2022-09-06 07:54:13 +0000
commit3e6566222c98cb42d7d60b5a3507fb8e294a1482 (patch)
tree0090f970559de7875a0c3960ce24b7aed6c34d33
parent00ce839865c7b08c57211817ae14009f02ee44f9 (diff)
downloadsamba-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_stream1
-rwxr-xr-xselftest/target/Samba3.pm7
-rwxr-xr-xsource3/script/tests/test_delete_stream.sh123
-rwxr-xr-xsource3/selftest/tests.py1
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"),