The Samba-Bugzilla – Attachment 13675 Details for
Bug 7933
samba fails to honor SEC_STD_WRITE_OWNER bit with the acl_xattr module
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for 4.7 cherry-picked from master
bug7933-v47.patch (text/plain), 28.02 KB, created by
Ralph Böhme
on 2017-10-10 11:45:21 UTC
(
hide
)
Description:
Patch for 4.7 cherry-picked from master
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2017-10-10 11:45:21 UTC
Size:
28.02 KB
patch
obsolete
>From 5dbcdfcb963b8ded173dda307a859dc2d7d673b0 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/11] 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 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 440a505114b2752f0a98673d5fa3bdb964038e24 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/11] 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 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 492848022dbac2f5d6ea6aaa84eec3cfdf5d7dad Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Sun, 8 Oct 2017 11:16:27 +0200 >Subject: [PATCH 03/11] 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 <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(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 78a6cbaa5fbbf0ec01f0c33ffba157cb5ae8e988 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Sun, 8 Oct 2017 08:51:05 +0200 >Subject: [PATCH 04/11] 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 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 382ffde18f2d7925280abf865886025f683338b2 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Sun, 8 Oct 2017 11:17:12 +0200 >Subject: [PATCH 05/11] 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 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 fa39443874312a504033f1c02c26a7ae7e55d460 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Sat, 7 Oct 2017 09:11:56 +0200 >Subject: [PATCH 06/11] 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> >(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 <<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 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 448d9ac220ad03af36349dcfb215f43513f18f04 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Fri, 6 Oct 2017 15:31:20 +0200 >Subject: [PATCH 07/11] 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> >(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 64e726270ed8ba931b8e3225ed549d057d5190e0 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 4 Oct 2017 15:45:54 +0200 >Subject: [PATCH 08/11] 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 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 1aebed90180ba0f37bbfdee2015c44a91f1e2cb4 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 4 Oct 2017 12:51:29 +0200 >Subject: [PATCH 09/11] 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 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 bfb5b4792eb4ac600333e22cf32667fee50f1266 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 4 Oct 2017 22:27:24 +0200 >Subject: [PATCH 10/11] 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 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 a9a6154894c2720bb04861440628650a05113501 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Fri, 6 Oct 2017 15:25:54 +0200 >Subject: [PATCH 11/11] 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 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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
jra
:
review+
Actions:
View
Attachments on
bug 7933
:
7140
|
7141
|
7142
|
7143
|
7292
|
7384
|
7581
|
13652
|
13674
|
13675
|
13689
|
13690