Attachment 13689 Details for Bug 7933 – Patch for 4.6 backported from master

[patch] Patch for 4.6 backported from master

bug7933-v46.patch (text/plain), 27.99 KB, created by Ralph Böhme on 2017-10-15 09:26:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2017-10-15 09:26:02 UTC

Size: 27.99 KB

patch

obsolete

>From e8f8ad5fdee3e21d53f77aa2d693e14813cf470d Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 8 Oct 2017 11:12:48 +0200
>Subject: [PATCH 01/12] selftest: fix acl_xattr test: changing owner
>
>Don't give ownership to user "force_user" as user "$USERNAME", this
>would fail with NT_STATUS_INVALID_OWNER, instead just take ownership as
>user "force_user". Adding a corresponding ACE for "force_user" with FULL
>rights ensures this works.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 0f8de2dee5451c9791f96050f85e4f007bec2819)
>---
> source3/script/tests/test_acl_xattr.sh | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh
>index 9b8808df78a..0d00bf0934f 100755
>--- a/source3/script/tests/test_acl_xattr.sh
>+++ b/source3/script/tests/test_acl_xattr.sh
>@@ -72,7 +72,8 @@ nt_affects_chown() {
> 
>     b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1
>     b4_actual=$(echo "$b4_actual" | sed -rn 's/^# owner: (.*)/\1/p')
>-    $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -C force_user 2>/dev/null || exit 1
>+    $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/FULL" || exit 1
>+    $SMBCACLS //$SERVER/$share $fname -U force_user%$PASSWORD -C force_user 2>/dev/null || exit 1
>     af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1
>     af_actual=$(echo "$af_actual" | sed -rn 's/^# owner: (.*)/\1/p')
>     echo "before: $b4_actual"
>-- 
>2.13.5
>
>
>From 67b23f68a11cd1e273321bb5379387b02166ea5f Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 8 Oct 2017 11:13:46 +0200
>Subject: [PATCH 02/12] selftest: fix acl_xattr test: group, not user
>
>In nt_affects_chgrp() check for domadmins *group*, not user. This didn't
>trigger an error as nt_affects_chgrp() isn't actually called, see next
>commit.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 71a2d06a1e41a1c412c82e58b3966e14c29c6159)
>---
> source3/script/tests/test_acl_xattr.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh
>index 0d00bf0934f..1b423f21876 100755
>--- a/source3/script/tests/test_acl_xattr.sh
>+++ b/source3/script/tests/test_acl_xattr.sh
>@@ -94,8 +94,8 @@ nt_affects_chgrp() {
>     b4_expected=$(echo "$b4_expected" | awk -F: '{print $3}')
>     echo "$b4_expected"
> 
>-    echo -n "determining uid of domadmins..."
>-    af_expected=$(getent passwd domadmins) || exit 1
>+    echo -n "determining gid of domadmins..."
>+    af_expected=$(getent group domadmins) || exit 1
>     af_expected=$(echo "$af_expected" | awk -F: '{print $3}')
>     echo "$af_expected"
> 
>-- 
>2.13.5
>
>
>From 5aead9bdba58d672d9f73790c6350f6923cdb235 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 8 Oct 2017 08:51:05 +0200
>Subject: [PATCH 03/12] selftest: fix acl_xattr test: sn-devel unreliable gid
>
>The "nt_affects_chgrp" kept failing in a full autobuild on sn-devel
>because the actual gid of the created file as returned by smbclient -c
>getfacl was reliably the unix gid of my account. It should have been the
>mapped domusers group for the primary users "Domain Users"
>group. Running the test individually or even the full set of
>"samba3.blackbox" tests didn't trigger the error.
>
>Looks like an issue with vfs_fake_acls and vfs_xattr_tdb, but I wasn't
>able to track it down. As the test only really want to ensure that
>smbcacls -G set the gid to the requested value, just remove the check
>for the actual initial gid.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit ea0ea829f5af63ab9638e758631c3002cbb6b4ce)
>---
> source3/script/tests/test_acl_xattr.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh
>index 1b423f21876..82dd741776e 100755
>--- a/source3/script/tests/test_acl_xattr.sh
>+++ b/source3/script/tests/test_acl_xattr.sh
>@@ -109,7 +109,7 @@ nt_affects_chgrp() {
>     af_actual=$(echo "$af_actual" | sed -rn 's/^# group: (.*)/\1/p')
>     echo "before: $b4_actual"
>     echo "after: $af_actual"
>-    test "$b4_expected" = "$b4_actual" && test "$af_expected" = "$af_actual"
>+    test "$af_expected" != "$b4_actual" && test "$af_expected" = "$af_actual"
> }
> 
> testit "setup remote file tmp" setup_remote_file tmp
>-- 
>2.13.5
>
>
>From b61e74c2bc48c7f86e190b7583e4a9baea7012ce Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 8 Oct 2017 11:17:12 +0200
>Subject: [PATCH 04/12] selftest: fix acl_xattr test script test_acl_xattr.sh
>
>The two "nt_affects_chgrp" tests called the wrong function so the
>function nt_affects_chgrp() was never run.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 3aff6315097cc9db0216bc18aa793a996930b0f4)
>---
> source3/script/tests/test_acl_xattr.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh
>index 82dd741776e..e4c556d08d4 100755
>--- a/source3/script/tests/test_acl_xattr.sh
>+++ b/source3/script/tests/test_acl_xattr.sh
>@@ -122,5 +122,5 @@ testit "nt_affects_chown tmp" nt_affects_chown tmp
> testit "nt_affects_chown ign_sysacls" nt_affects_chown ign_sysacls
> testit "setup remote file tmp" setup_remote_file tmp
> testit "setup remote file ign_sysacls" setup_remote_file ign_sysacls
>-testit "nt_affects_chgrp tmp" nt_affects_chown tmp
>-testit "nt_affects_chgrp ign_sysacls" nt_affects_chown ign_sysacls
>+testit "nt_affects_chgrp tmp" nt_affects_chgrp tmp
>+testit "nt_affects_chgrp ign_sysacls" nt_affects_chgrp ign_sysacls
>-- 
>2.13.5
>
>
>From 999c933d6a8890cf1cf29b1c55a6c7362d05d802 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 7 Oct 2017 09:11:56 +0200
>Subject: [PATCH 05/12] selftest: fix samba3.blackbox.inherit_owner.default
> test script test_inherit_owner.sh
>
>Grant the test-user SeRestorePrivilege, this is needed for
>give-ownership operations. And then granting SeRestorePrivilege requires
>`net`, so add that as an additional argument to the script.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(backported from commit ff199d8e3ea7bd1ed12de8c39340ba640a2b83ca)
>---
> source3/script/tests/test_inherit_owner.sh | 17 +++++++++++------
> source3/selftest/tests.py                  |  6 +++---
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
>diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh
>index 5146d406546..9f22a2c1cba 100755
>--- a/source3/script/tests/test_inherit_owner.sh
>+++ b/source3/script/tests/test_inherit_owner.sh
>@@ -4,9 +4,9 @@
> # currently needs to run in SMB1 mode, because it uses UNIX
> # extensions to fetch the UNIX owner of a file.
> 
>-if [ $# -lt 9 ]; then
>+if [ $# -lt 10 ]; then
> cat <<EOF
>-Usage: $0 SERVER USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS SHARE INH_WIN INH_UNIX <additional args>
>+Usage: $0 SERVER USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS NET SHARE INH_WIN INH_UNIX <additional args>
> EOF
> exit 1;
> fi
>@@ -17,13 +17,15 @@ PASSWORD="$3"
> PREFIX="$4"
> SMBCLIENT="$5"
> SMBCACLS="$6"
>-SHARE="$7"
>-INH_WIN="$8"
>-INH_UNIX="$9"
>-shift 9
>+NET="$7"
>+SHARE="$8"
>+INH_WIN="$9"
>+INH_UNIX="${10}"
>+shift 10
> ADDARGS="$*"
> SMBCLIENT="$VALGRIND ${SMBCLIENT} ${ADDARGS}"
> SMBCACLS="$VALGRIND ${SMBCACLS} ${ADDARGS}"
>+NET="$VALGRIND ${NET}"
> 
> incdir=`dirname $0`/../../../testprogs/blackbox
> . $incdir/subunit.sh
>@@ -137,6 +139,7 @@ fi
> 
> # SETUP
> testit "$TEST_LABEL - setup root dir" create_dir tmp tmp.$$
>+testit "grant SeRestorePrivilege" $NET rpc rights grant $USERNAME SeRestorePrivilege -U $USERNAME%$PASSWORD -I $SERVER || exit 1
> testit "$TEST_LABEL - assign default ACL" $SMBCACLS //$SERVER/tmp tmp.$$ -U $USERNAME%$PASSWORD -S "REVISION:1,OWNER:$SERVER\force_user,GROUP:$SERVER\domusers,ACL:Everyone:ALLOWED/0x3/FULL" 2>/dev/null
> # END SETUP
> 
>@@ -155,3 +158,5 @@ testit "$TEST_LABEL - verify file unix owner after change" unix_owner_id_is $SHA
> testit "$TEST_LABEL - cleanup subdir" cleanup_dir $SHARE tmp.$$/subdir
> testit "$TEST_LABEL - cleanup file" cleanup_file $SHARE tmp.$$/afile
> testit "$TEST_LABEL - cleanup root" cleanup_dir $SHARE tmp.$$
>+
>+testit "revoke SeRestorePrivilege" $NET rpc rights revoke $USERNAME SeRestorePrivilege -U $USERNAME%$PASSWORD -I $SERVER || exit 1
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 746f3ea0bf6..f9280d6c6d0 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -221,9 +221,9 @@ for env in ["fileserver"]:
>     plantestsuite("samba3.blackbox.netshareenum (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_shareenum.sh"), '$SERVER', '$USERNAME', '$PASSWORD', rpcclient])
>     plantestsuite("samba3.blackbox.acl_xattr (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_acl_xattr.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls])
>     plantestsuite("samba3.blackbox.smb2.not_casesensitive (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smb2_not_casesensitive.sh"), '//$SERVER/tmp', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', smbclient3])
>-    plantestsuite("samba3.blackbox.inherit_owner.default(%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'tmp', '0', '0', '-m', 'NT1'])
>-    plantestsuite("samba3.blackbox.inherit_owner.full (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner', '1', '1', '-m', 'NT1'])
>-    plantestsuite("samba3.blackbox.inherit_owner.unix (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner_u', '0', '1', '-m', 'NT1'])
>+    plantestsuite("samba3.blackbox.inherit_owner.default(%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'tmp', '0', '0', '-m', 'NT1'])
>+    plantestsuite("samba3.blackbox.inherit_owner.full (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'inherit_owner', '1', '1', '-m', 'NT1'])
>+    plantestsuite("samba3.blackbox.inherit_owner.unix (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'inherit_owner_u', '0', '1', '-m', 'NT1'])
>     plantestsuite("samba3.blackbox.large_acl (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls])
> 
>     #
>-- 
>2.13.5
>
>
>From 6fdc05b785096c72ae873ad22d19222a4cb409e4 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Fri, 6 Oct 2017 15:31:20 +0200
>Subject: [PATCH 06/12] selftest: tests for change ownership on a file
>
>This test verifies that SEC_STD_WRITE_OWNER only effectively grants
>take-ownership permissions but NOT give-ownership. The latter requires
>SeRestorePrivilege privilege.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(backported from commit 4b2e171e6b90f9c3594ebb705a28c66b981c2bf5)
>---
> selftest/knownfail.d/samba3.blackbox.give_owner |   1 +
> source3/script/tests/test_give_owner.sh         | 121 ++++++++++++++++++++++++
> source3/selftest/tests.py                       |   1 +
> 3 files changed, 123 insertions(+)
> create mode 100644 selftest/knownfail.d/samba3.blackbox.give_owner
> create mode 100755 source3/script/tests/test_give_owner.sh
>
>diff --git a/selftest/knownfail.d/samba3.blackbox.give_owner b/selftest/knownfail.d/samba3.blackbox.give_owner
>new file mode 100644
>index 00000000000..28fc0c03236
>--- /dev/null
>+++ b/selftest/knownfail.d/samba3.blackbox.give_owner
>@@ -0,0 +1 @@
>+samba3.blackbox.give_owner.give owner without SeRestorePrivilege$fileserver$
>diff --git a/source3/script/tests/test_give_owner.sh b/source3/script/tests/test_give_owner.sh
>new file mode 100755
>index 00000000000..64e09f3c2b4
>--- /dev/null
>+++ b/source3/script/tests/test_give_owner.sh
>@@ -0,0 +1,121 @@
>+#!/bin/sh
>+#
>+# this verifies that SEC_STD_WRITE_OWNER only effectively grants take-ownership
>+# permissions but NOT give-ownership.
>+#
>+
>+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}"
>+failed=0
>+
>+incdir=`dirname $0`/../../../testprogs/blackbox
>+. $incdir/subunit.sh
>+
>+setup_testfile() {
>+    local share=$1
>+    local fname=$2
>+    touch $PREFIX/$fname
>+    $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "rm $fname"
>+    $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "ls" | grep "$fname" && return 1
>+    $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "lcd $PREFIX; put $fname" || return 1
>+}
>+
>+remove_testfile() {
>+    local share=$1
>+    local fname=$2
>+    $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "rm $fname"
>+}
>+
>+set_win_owner() {
>+    local share=$1
>+    local fname=$2
>+    local owner=$3
>+    echo "$SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -C '$owner'"
>+    $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -C "$owner" || return 1
>+}
>+
>+win_owner_is() {
>+    local share=$1
>+    local fname=$2
>+    local expected_owner=$3
>+    local actual_owner
>+
>+    echo "$SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD"
>+    $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD
>+    actual_owner=$($SMBCACLS //$SERVER/$share $fname -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 $share/$fname is [$actual_owner] expected [$expected_owner]"
>+        return 1
>+    fi
>+    return 0
>+}
>+
>+add_ace() {
>+    local share=$1
>+    local fname=$2
>+    local ace=$3
>+
>+    local_ace=$(echo $ace | sed 's|\\|/|')
>+
>+    # avoid duplicate
>+    out=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD)
>+    echo "$out" | grep "$local_ace" && return 0
>+
>+    # add it
>+    $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "$ace" || return 1
>+
>+    # check it's there
>+    out=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD) || return 1
>+    echo "$out" | grep "$local_ace" || return 1
>+}
>+
>+chown_give_fails() {
>+    local share=$1
>+    local fname=$2
>+    local user=$3
>+    local expected_error=$4
>+
>+    # this must fail
>+    out=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -C "$user") && return 1
>+    # it failed, now check it returned the expected error code
>+    echo "$out" | grep $expected_error || return 1
>+}
>+
>+# Create a testfile
>+testit "create testfile" setup_testfile $SHARE afile || failed=`expr $failed + 1`
>+testit "verify owner" win_owner_is $SHARE afile "$SERVER/$USERNAME" || failed=`expr $failed + 1`
>+
>+# Grant SeRestorePrivilege to the user and full rights on the file
>+testit "grant SeRestorePrivilege" $NET rpc rights grant $USERNAME SeRestorePrivilege -U $USERNAME%$PASSWORD -I $SERVER_IP || failed=`expr $failed + 1`
>+testit "grant full rights" add_ace $SHARE afile "ACL:$SERVER\\$USERNAME:ALLOWED/0x0/FULL" || failed=`expr $failed + 1`
>+
>+# We have SeRestorePrivilege, so both give and take ownership must succeed
>+testit "give owner with SeRestorePrivilege" set_win_owner $SHARE afile "$SERVER\user1" || failed=`expr $failed + 1`
>+testit "verify owner" win_owner_is $SHARE afile "$SERVER/user1" || failed=`expr $failed + 1`
>+testit "take owner" set_win_owner $SHARE afile "$SERVER\\$USERNAME" || failed=`expr $failed + 1`
>+testit "verify owner" win_owner_is $SHARE afile "$SERVER/$USERNAME" || failed=`expr $failed + 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 || failed=`expr $failed + 1`
>+testit "give owner without SeRestorePrivilege" chown_give_fails $SHARE afile "$SERVER\user1" NT_STATUS_INVALID_OWNER || failed=`expr $failed + 1`
>+
>+testit "delete testfile" remove_testfile $SHARE afile || failed=`expr $failed + 1`
>+
>+exit $failed
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index f9280d6c6d0..b7540d5cf59 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -225,6 +225,7 @@ for env in ["fileserver"]:
>     plantestsuite("samba3.blackbox.inherit_owner.full (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'inherit_owner', '1', '1', '-m', 'NT1'])
>     plantestsuite("samba3.blackbox.inherit_owner.unix (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'inherit_owner_u', '0', '1', '-m', 'NT1'])
>     plantestsuite("samba3.blackbox.large_acl (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls])
>+    plantestsuite("samba3.blackbox.give_owner (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_give_owner.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'tmp'])
> 
>     #
>     # tar command tests
>-- 
>2.13.5
>
>
>From 93a98dae6df6eb95cac07050e97dcbaa0b2593f6 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 4 Oct 2017 15:45:54 +0200
>Subject: [PATCH 07/12] s3/smbd/posix_acls: return correct status in try_chown
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit cc555be4d01c4140445bd30e16be3fe8343d3872)
>---
> source3/smbd/posix_acls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
>index 6999c830c96..26ef113874e 100644
>--- a/source3/smbd/posix_acls.c
>+++ b/source3/smbd/posix_acls.c
>@@ -3665,7 +3665,7 @@ NTSTATUS try_chown(files_struct *fsp, uid_t uid, gid_t gid)
> 	   a local SID on the users workstation
> 	*/
> 	if (uid != get_current_uid(fsp->conn)) {
>-		return NT_STATUS_ACCESS_DENIED;
>+		return NT_STATUS_INVALID_OWNER;
> 	}
> 
> 	become_root();
>-- 
>2.13.5
>
>
>From 01b2fd12df9ec7461a01109f49e763af035e6081 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 4 Oct 2017 12:51:29 +0200
>Subject: [PATCH 08/12] vfs_acl_common: factor out a variable declaration
>
>Just some refactoring, no change in behaviour.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit e62f90a6d15626a2e20ba92f5cd552101ec4afe0)
>---
> source3/modules/vfs_acl_common.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
>index fa65fd19cfd..b887e0ac9b1 100644
>--- a/source3/modules/vfs_acl_common.c
>+++ b/source3/modules/vfs_acl_common.c
>@@ -1056,8 +1056,9 @@ static NTSTATUS set_underlying_acl(vfs_handle_struct *handle, files_struct *fsp,
> 				   uint32_t security_info_sent,
> 				   bool chown_needed)
> {
>-	NTSTATUS status =
>-	    SMB_VFS_NEXT_FSET_NT_ACL(handle, fsp, security_info_sent, psd);
>+	NTSTATUS status;
>+
>+	status = SMB_VFS_NEXT_FSET_NT_ACL(handle, fsp, security_info_sent, psd);
> 	if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
> 		return status;
> 	}
>-- 
>2.13.5
>
>
>From 8b6a98973d285343ed19b3a339f92275018905f5 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 4 Oct 2017 22:27:24 +0200
>Subject: [PATCH 09/12] vfs_acl_common: fix take ownership vs give ownership
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 7e7afef819b4a858e6de48389c6f4fa7510cf5c6)
>---
> source3/modules/vfs_acl_common.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
>index b887e0ac9b1..42eed028ed9 100644
>--- a/source3/modules/vfs_acl_common.c
>+++ b/source3/modules/vfs_acl_common.c
>@@ -1057,6 +1057,7 @@ static NTSTATUS set_underlying_acl(vfs_handle_struct *handle, files_struct *fsp,
> 				   bool chown_needed)
> {
> 	NTSTATUS status;
>+	const struct security_token *token = NULL;
> 
> 	status = SMB_VFS_NEXT_FSET_NT_ACL(handle, fsp, security_info_sent, psd);
> 	if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
>@@ -1071,6 +1072,18 @@ static NTSTATUS set_underlying_acl(vfs_handle_struct *handle, files_struct *fsp,
> 		return NT_STATUS_ACCESS_DENIED;
> 	}
> 
>+	/*
>+	 * Only allow take-ownership, not give-ownership. That's the way Windows
>+	 * implements SEC_STD_WRITE_OWNER. MS-FSA 2.1.5.16 just states: If
>+	 * InputBuffer.OwnerSid is not a valid owner SID for a file in the
>+	 * objectstore, as determined in an implementation specific manner, the
>+	 * object store MUST return STATUS_INVALID_OWNER.
>+	 */
>+	token = get_current_nttok(fsp->conn);
>+	if (!security_token_is_sid(token, psd->owner_sid)) {
>+		return NT_STATUS_INVALID_OWNER;
>+	}
>+
> 	DBG_DEBUG("overriding chown on file %s for sid %s\n",
> 		   fsp_str_dbg(fsp), sid_string_tos(psd->owner_sid));
> 
>-- 
>2.13.5
>
>
>From 93f9f9c9d0e0649421bf646ee03c1bdd51b475fd Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Fri, 6 Oct 2017 15:25:54 +0200
>Subject: [PATCH 10/12] vfs_fake_acls: deny give-ownership
>
>Windows doesn't allow giving ownership away unless the user has
>SEC_PRIV_RESTORE privilege.
>
>This follows from MS-FSA 2.1.5.1, so it's a property of the filesystem
>layer, not the SMB layer. By implementing this restriction here, we can
>now have test for this restriction.
>
>Other filesystems may want to deliberately allow this behaviour --
>although I'm not aware of any that does -- therefor I'm putting in this
>restriction in the implementation of the chmod VFS function and not into
>the caller.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 0666093cb0d820cc27a265c1f0a87bc76cd3c167)
>---
> selftest/knownfail.d/samba3.blackbox.give_owner |  1 -
> source3/modules/vfs_fake_acls.c                 | 18 ++++++++++++++++++
> 2 files changed, 18 insertions(+), 1 deletion(-)
> delete mode 100644 selftest/knownfail.d/samba3.blackbox.give_owner
>
>diff --git a/selftest/knownfail.d/samba3.blackbox.give_owner b/selftest/knownfail.d/samba3.blackbox.give_owner
>deleted file mode 100644
>index 28fc0c03236..00000000000
>--- a/selftest/knownfail.d/samba3.blackbox.give_owner
>+++ /dev/null
>@@ -1 +0,0 @@
>-samba3.blackbox.give_owner.give owner without SeRestorePrivilege$fileserver$
>diff --git a/source3/modules/vfs_fake_acls.c b/source3/modules/vfs_fake_acls.c
>index 55ff7db37df..b1584ad5274 100644
>--- a/source3/modules/vfs_fake_acls.c
>+++ b/source3/modules/vfs_fake_acls.c
>@@ -401,6 +401,12 @@ static int fake_acls_chown(vfs_handle_struct *handle,
> 	int ret;
> 	uint8_t id_buf[4];
> 	if (uid != -1) {
>+		uid_t current_uid = get_current_uid(handle->conn);
>+
>+		if (current_uid != 0 && current_uid != uid) {
>+			return EACCES;
>+		}
>+
> 		SIVAL(id_buf, 0, uid);
> 		ret = SMB_VFS_NEXT_SETXATTR(handle,
> 				smb_fname->base_name,
>@@ -435,6 +441,12 @@ static int fake_acls_lchown(vfs_handle_struct *handle,
> 	int ret;
> 	uint8_t id_buf[4];
> 	if (uid != -1) {
>+		uid_t current_uid = get_current_uid(handle->conn);
>+
>+		if (current_uid != 0 && current_uid != uid) {
>+			return EACCES;
>+		}
>+
> 		/* This isn't quite right (calling setxattr not
> 		 * lsetxattr), but for the test purposes of this
> 		 * module (fake NT ACLs from windows clients), it is
>@@ -474,6 +486,12 @@ static int fake_acls_fchown(vfs_handle_struct *handle, files_struct *fsp, uid_t
> 	int ret;
> 	uint8_t id_buf[4];
> 	if (uid != -1) {
>+		uid_t current_uid = get_current_uid(handle->conn);
>+
>+		if (current_uid != 0 && current_uid != uid) {
>+			return EACCES;
>+		}
>+
> 		SIVAL(id_buf, 0, uid);
> 		ret = SMB_VFS_NEXT_FSETXATTR(handle, fsp, FAKE_UID, id_buf, sizeof(id_buf), 0);
> 		if (ret != 0) {
>-- 
>2.13.5
>
>
>From 4057c2ba0782e4204b50a6f443a2238fabad24d6 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 12 Oct 2017 17:07:15 +0200
>Subject: [PATCH 11/12] selftest: add some debugging to test_give_owner.sh
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Fri Oct 13 01:22:05 CEST 2017 on sn-devel-144
>
>(cherry picked from commit 156015aed0b5a72b2f7150beb5cdaffa32b554e5)
>---
> source3/script/tests/test_give_owner.sh | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
>diff --git a/source3/script/tests/test_give_owner.sh b/source3/script/tests/test_give_owner.sh
>index 64e09f3c2b4..c8f437ebd8b 100755
>--- a/source3/script/tests/test_give_owner.sh
>+++ b/source3/script/tests/test_give_owner.sh
>@@ -76,14 +76,34 @@ add_ace() {
> 
>     # avoid duplicate
>     out=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD)
>+    if [ $? -ne 0 ] ; then
>+	echo "get acl failed"
>+	echo "$out"
>+	return 1
>+    fi
>+    echo "Original ACL"
>+    echo $out
>     echo "$out" | grep "$local_ace" && return 0
> 
>     # add it
>-    $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "$ace" || return 1
>+    $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "$ace"
>+    if [ $? -ne 0 ] ; then
>+	echo "add acl failed"
>+	return 1
>+    fi
> 
>     # check it's there
>-    out=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD) || return 1
>+    out=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD)
>+    if [ $? -ne 0 ] ; then
>+	echo "get new acl failed"
>+	echo "$out"
>+	return 1
>+    fi
>+    echo "New ACL"
>+    echo $out
>+    echo "Checking if new ACL has \"$local_ace\""
>     echo "$out" | grep "$local_ace" || return 1
>+    echo "ok"
> }
> 
> chown_give_fails() {
>-- 
>2.13.5
>
>
>From 0366de5b64b2b8cb198add9faffacff7508e98b3 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Fri, 13 Oct 2017 14:32:58 +0200
>Subject: [PATCH 12/12] selftest: prevent interpretation of escape sequences in
> test_give_owner.sh
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>
>Autobuild-User(master): Ralph BÃ¶hme <slow@samba.org>
>Autobuild-Date(master): Sat Oct 14 06:02:50 CEST 2017 on sn-devel-144
>
>(cherry picked from commit 7abf0acef48cb585fa8e5666fd4c27692b9c8ae3)
>---
> source3/script/tests/test_give_owner.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/script/tests/test_give_owner.sh b/source3/script/tests/test_give_owner.sh
>index c8f437ebd8b..7ee37341b42 100755
>--- a/source3/script/tests/test_give_owner.sh
>+++ b/source3/script/tests/test_give_owner.sh
>@@ -72,7 +72,7 @@ add_ace() {
>     local fname=$2
>     local ace=$3
> 
>-    local_ace=$(echo $ace | sed 's|\\|/|')
>+    local_ace=$(printf '%s' "$ace" | sed 's|\\|/|')
> 
>     # avoid duplicate
>     out=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD)
>-- 
>2.13.5
>

Flags:

jra: review+

Actions: View

Attachments on bug 7933: 7140 | 7141 | 7142 | 7143 | 7292 | 7384 | 7581 | 13652 | 13674 | 13675 | 13689 | 13690