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)

diff --git a/selftest/knownfail.d/samba3.blackbox.delete_stream b/selftest/knownfail.d/samba3.blackbox.delete_stream
new file mode 100644 (file)
index 0000000..22c996a
--- /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 43bce06c6d99e8e1e8dd2ef4df1e84e2d8dcc1fc..fdb550a8f66c00c02f37f46d266b914c5bd8488f 100755 (executable)
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -3214,6 +3214,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 (executable)
index 0000000..43d591e
--- /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 c53b2815eba2212498827ce9320f3f5f44b8a942..05690b75c8430ca342de6deb941eb49611c03845 100755 (executable)
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -554,6 +554,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"),