From 35572b12bb94dc4d54e71ce09011853da7c8d0e0 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 8 Oct 2017 11:12:48 +0200 Subject: [PATCH 01/13] 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 Reviewed-by: Jeremy Allison (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 ddccf4fce8a..9697f06a43b 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -77,7 +77,8 @@ nt_affects_chown() { b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# owner:" || 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 echo "${af_actual}" | grep -q "^# owner:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# owner: (.*)/\1/p') -- 2.13.5 From 99a13f69b32d6582bbcfcd589a934a7f42de2389 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 8 Oct 2017 11:13:46 +0200 Subject: [PATCH 02/13] 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 Reviewed-by: Jeremy Allison (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 9697f06a43b..bc3326cf919 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -100,8 +100,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 8406f682181bb7cb0e1177bad29d74a2d7e6f061 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 8 Oct 2017 11:16:27 +0200 Subject: [PATCH 03/13] selftest: fix acl_xattr test: grep ouput before munging The check of the smbclient getfacl output for presence of a "^# group:" line must be done before munging the saved output with a sed filter. Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 1fabe25339166e381ab0e118667f9c19781b49df) --- 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 bc3326cf919..463969363fb 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -109,8 +109,8 @@ nt_affects_chgrp() { test "$b4_expected" != "$af_expected" || exit 1 b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 - b4_actual=$(echo "$b4_actual" | sed -rn 's/^# group: (.*)/\1/p') echo "${b4_actual}" | grep -q "^# group:" || exit 1 + b4_actual=$(echo "$b4_actual" | sed -rn 's/^# group: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -G domadmins 2>/dev/null || exit 1 af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# group:" || exit 1 -- 2.13.5 From ed51a5848f86dfb8b19f7c2618358e4638f21bf1 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 8 Oct 2017 08:51:05 +0200 Subject: [PATCH 04/13] 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 Reviewed-by: Jeremy Allison (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 463969363fb..facd1dad5af 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -117,7 +117,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 273e726bee7fe39d7bf5d2c55579698beadf96b2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 8 Oct 2017 11:17:12 +0200 Subject: [PATCH 05/13] 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 Reviewed-by: Jeremy Allison (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 facd1dad5af..cba79122045 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -130,5 +130,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 0b19c41666f67163be6479ad1a081ad0b4716a51 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 7 Oct 2017 09:11:56 +0200 Subject: [PATCH 06/13] 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 Reviewed-by: Jeremy Allison (cherry picked from commit ff199d8e3ea7bd1ed12de8c39340ba640a2b83ca) --- source3/script/tests/test_inherit_owner.sh | 17 +++++++++++------ source3/selftest/tests.py | 12 ++++++------ 2 files changed, 17 insertions(+), 12 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 < +Usage: $0 SERVER USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS NET SHARE INH_WIN INH_UNIX 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 d459edeca63..e4cbd94f959 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -245,12 +245,12 @@ for env in ["fileserver"]: plantestsuite("samba3.blackbox.acl_xattr.NT1", env, [os.path.join(samba3srcdir, "script/tests/test_acl_xattr.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, '-mNT1']) plantestsuite("samba3.blackbox.acl_xattr.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_acl_xattr.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, '-mSMB3']) 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.NT1", 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.default.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'tmp', '0', '0', '-m', 'SMB3']) - plantestsuite("samba3.blackbox.inherit_owner.full.NT1", 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.full.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner', '1', '1', '-m', 'SMB3']) - plantestsuite("samba3.blackbox.inherit_owner.unix.NT1", 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.unix.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner_u', '0', '1', '-m', 'SMB3']) + plantestsuite("samba3.blackbox.inherit_owner.default.NT1", 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.default.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'tmp', '0', '0', '-m', 'SMB3']) + plantestsuite("samba3.blackbox.inherit_owner.full.NT1", 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.full.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'inherit_owner', '1', '1', '-m', 'SMB3']) + plantestsuite("samba3.blackbox.inherit_owner.unix.NT1", 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.inherit_owner.unix.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'inherit_owner_u', '0', '1', '-m', 'SMB3']) plantestsuite("samba3.blackbox.large_acl.NT1", env, [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']) -- 2.13.5 From be18d1445f6bcd1659ac0da5a8ef87045847bf23 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 6 Oct 2017 15:31:20 +0200 Subject: [PATCH 07/13] 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 Reviewed-by: Jeremy Allison (cherry picked 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 e4cbd94f959..68a3ae2c5df 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -253,6 +253,7 @@ for env in ["fileserver"]: plantestsuite("samba3.blackbox.inherit_owner.unix.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'inherit_owner_u', '0', '1', '-m', 'SMB3']) plantestsuite("samba3.blackbox.large_acl.NT1", env, [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']) # # tar command tests -- 2.13.5 From 5cc7004caf10958b4fc723cb61d033ce77109cd4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 4 Oct 2017 15:45:54 +0200 Subject: [PATCH 08/13] 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 Reviewed-by: Jeremy Allison (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 e4403458495..7bd65390406 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -3671,7 +3671,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 20a97a27f2aa32e0c36e854482bca509b82779e4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 4 Oct 2017 12:51:29 +0200 Subject: [PATCH 09/13] 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 Reviewed-by: Jeremy Allison (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 6abf1e312fb..7898b091345 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 972dd4f4f82136a77061acab51eccf97ad24dbe6 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 4 Oct 2017 22:27:24 +0200 Subject: [PATCH 10/13] vfs_acl_common: fix take ownership vs give ownership Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (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 7898b091345..08021b1c3bc 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 9947ff4a6b60cf517988e911d85d8b3ab7fdd817 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 6 Oct 2017 15:25:54 +0200 Subject: [PATCH 11/13] 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 Reviewed-by: Jeremy Allison (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 7de5cf00bd6..0f539d1f29c 100644 --- a/source3/modules/vfs_fake_acls.c +++ b/source3/modules/vfs_fake_acls.c @@ -413,6 +413,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, @@ -447,6 +453,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 @@ -486,6 +498,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 5f3f86e2c6f16ae954506866ea2ec929b8df4758 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 12 Oct 2017 17:07:15 +0200 Subject: [PATCH 12/13] selftest: add some debugging to test_give_owner.sh Bug: https://bugzilla.samba.org/show_bug.cgi?id=7933 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison 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 046a23a36c1efa9ee0df64455d4c27a46d557510 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 13 Oct 2017 14:32:58 +0200 Subject: [PATCH 13/13] 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 Reviewed-by: Volker Lendecke Reviewed-by: Andreas Schneider Autobuild-User(master): Ralph Böhme 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