Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Aug 2008 12:59:54 GMT
From:      Edward Tomasz Napierala <trasz@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 147150 for review
Message-ID:  <200808111259.m7BCxs0r013050@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=147150

Change 147150 by trasz@trasz_traszkan on 2008/08/11 12:59:01

	Fix one failing test (return EACCES instead of EPERM for
	DELETE/DELETE_CHILD); introduce several other ones.

Affected files ...

.. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_nfs4.c#18 edit
.. //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/fstest.c#3 edit
.. //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/tests/chmod/12.t#1 add
.. //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/tests/granular/00.t#2 edit
.. //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/tests/granular/02.t#2 edit
.. //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/tests/granular/03.t#2 edit

Differences ...

==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_nfs4.c#18 (text+ko) ====

@@ -249,7 +249,7 @@
 		return (0);
 	}
 
-	if (acc_mode & (VADMIN_PERMS | VDELETE_CHILD | VDELETE))
+	if (acc_mode & VADMIN_PERMS)
 		denied = EPERM;
 	else
 		denied = EACCES;

==== //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/fstest.c#3 (text+ko) ====

@@ -82,6 +82,7 @@
 	ACTION_PREPENDACL,
 	ACTION_READACL,
 #endif
+	ACTION_WRITE,
 };
 
 #define	TYPE_NONE	0x0000
@@ -127,6 +128,7 @@
 	{ "prependacl", ACTION_PREPENDACL, { TYPE_STRING, TYPE_STRING, TYPE_NONE } },
 	{ "readacl", ACTION_READACL, { TYPE_STRING, TYPE_NONE } },
 #endif
+	{ "write", ACTION_WRITE, { TYPE_STRING, TYPE_NONE } },
 	{ NULL, -1, { TYPE_NONE } }
 };
 
@@ -533,6 +535,15 @@
 			rval = 0;
 		break;
 #endif
+
+	case ACTION_WRITE:
+		rval = open(STR(0), O_WRONLY);
+		if (rval < 0)
+			break;
+
+		rval = write(rval, "x", 1);
+		break;
+
 	default:
 		fprintf(stderr, "unsupported syscall\n");
 		exit(1);

==== //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/tests/granular/00.t#2 (text+ko) ====

@@ -5,13 +5,15 @@
 dir=`dirname $0`
 . ${dir}/../misc.sh
 
-echo "1..27"
+echo "1..49"
 
 n0=`namegen`
 n1=`namegen`
 n2=`namegen`
+n3=`namegen`
 
 expect 0 mkdir ${n2} 0755
+expect 0 mkdir ${n3} 0777
 cdir=`pwd`
 cd ${n2}
 
@@ -28,7 +30,7 @@
 expect 0 mkdir ${n0} 0755
 expect 0 rmdir ${n0}
 
-# Tests 8..16 - check out whether user 65534 is permitted to create and remove
+# Check whether user 65534 is permitted to create and remove
 # files, but not subdirectories.
 expect 0 prependacl . user:65534:write_data::allow,user:65534:append_data::deny
 
@@ -46,9 +48,25 @@
 expect 0 mkdir ${n0} 0755
 expect 0 -u 65534 -g 65534 rmdir ${n0}
 
-# XXX: Check for moving things from/to ${n0}.
+# Can move files from other directory?
+expect 0 create ../${n3}/${n1} 0644
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+
+# Can move files from other directory overwriting existing files?
+expect 0 create ../${n3}/${n1} 0644
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+
+expect 0 -u 65534 -g 65534 unlink ${n0}
+
+# Can move directories from other directory?
+expect 0 mkdir ../${n3}/${n1} 0777
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+
+# Can move directories from other directory overwriting existing directory?
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+expect 0 -u 65534 -g 65534 rmdir ../${n3}/${n1}
 
-# Tests 17..27 - check out whether user 65534 is permitted to create
+# Check whether user 65534 is permitted to create
 # subdirectories, but not files - and to remove neither of them.
 expect 0 prependacl . user:65534:write_data::deny,user:65534:append_data::allow
 
@@ -67,7 +85,25 @@
 expect EACCES -u 65534 -g 65534 rmdir ${n0}
 expect 0 rmdir ${n0}
 
-# XXX: Check for moving things from/to ${n0}.
+# Can move files from other directory?
+expect 0 create ../${n3}/${n1} 0644
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+
+# Can move files from other directory overwriting existing files?
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+expect 0 -u 65534 -g 65534 unlink ../${n3}/${n1}
+
+# Can move directories from other directory?
+expect 0 mkdir ../${n3}/${n1} 0777
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+
+# Can move directories from other directory overwriting existing directory?
+expect 0 mkdir ../${n3}/${n1} 0777
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+expect 0 prependacl . user:65534:delete_child::allow
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n1} ${n0}
+expect 0 -u 65534 -g 65534 rmdir ${n0}
 
 cd ${cdir}
 expect 0 rmdir ${n2}
+expect 0 rmdir ${n3}

==== //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/tests/granular/02.t#2 (text+ko) ====

@@ -5,7 +5,7 @@
 dir=`dirname $0`
 . ${dir}/../misc.sh
 
-echo "1..17"
+echo "1..52"
 
 n0=`namegen`
 n1=`namegen`
@@ -15,7 +15,7 @@
 cdir=`pwd`
 cd ${n2}
 
-# Tests 1..12 - check out whether user 65534 is permitted to read ACL.
+# Check whether user 65534 is permitted to read ACL.
 expect 0 create ${n0} 0644
 expect 0 readacl ${n0}
 expect 0 -u 65534 -g 65534 readacl ${n0}
@@ -27,12 +27,77 @@
 expect 0 readacl ${n0}
 expect 0 unlink ${n0}
 
-# Tests 12..17 - check out whether user 65534 is permitted to write ACL.
+# Check whether user 65534 is permitted to write ACL.
 expect 0 create ${n0} 0644
 expect EPERM -u 65534 -g 65534 prependacl ${n0} user:65534:read_data::allow
 expect 0 prependacl ${n0} user:65534:write_acl::allow
 expect 0 -u 65534 -g 65534 prependacl ${n0} user:65534:read_data::allow
 expect 0 unlink ${n0}
 
+# Check whether user 65534 is permitted to write mode.
+expect 0 create ${n0} 0755
+expect EPERM -u 65534 -g 65534 chmod ${n0} 0777
+expect 0 prependacl ${n0} user:65534:write_acl::allow
+expect 0 -u 65534 -g 65534 chmod ${n0} 0777
+expect 0 unlink ${n0}
+
+# There is an interesting problem with interaction between ACL_WRITE_ACL
+# and SUID/SGID bits.  In case user does have ACL_WRITE_ACL, but is not
+# a file owner, Solaris does the following:
+# 1. Setting SUID fails with EPERM.
+# 2. Setting SGID succeeds, but mode is not changed.
+# 3. Modifying ACL does not clear SUID nor SGID bits.
+# 4. Writing the file does clear both SUID and SGID bits.
+#
+# What we are doing is the following:
+# 1. Setting SUID or SGID fails with EPERM.
+# 2. Modifying ACL does not clear SUID nor SGID bits.
+# 3. Writing the file does clear both SUID and SGID bits.
+#
+# Check whether user 65534 is denied to write mode with SUID bit.
+expect 0 create ${n0} 0755
+expect EPERM -u 65534 -g 65534 chmod ${n0} 04777
+expect 0 prependacl ${n0} user:65534:write_acl::allow
+expect EPERM -u 65534 -g 65534 chmod ${n0} 04777
+expect 0 unlink ${n0}
+
+# Check whether user 65534 is denied to write mode with SGID bit.
+expect 0 create ${n0} 0755
+expect EPERM -u 65534 -g 65534 chmod ${n0} 02777
+expect 0 prependacl ${n0} user:65534:write_acl::allow
+expect EPERM -u 65534 -g 65534 chmod ${n0} 02777
+expect 0 unlink ${n0}
+
+# Check whether user 65534 is allowed to write mode with sticky bit.
+expect 0 mkdir ${n0} 0755
+expect EPERM -u 65534 -g 65534 chmod ${n0} 01777
+expect 0 prependacl ${n0} user:65534:write_acl::allow
+expect 0 -u 65534 -g 65534 chmod ${n0} 01777
+expect 0 rmdir ${n0}
+
+# Check whether modifying the ACL by not-owner preserves the SUID.
+expect 0 create ${n0} 04755
+expect 0 prependacl ${n0} user:65534:write_acl::allow
+expect 0 -u 65534 -g 65534 prependacl ${n0} user:65534:write_data::allow
+expect 04755 stat ${n0} mode
+expect 0 unlink ${n0}
+
+# Check whether modifying the ACL by not-owner preserves the SGID.
+expect 0 create ${n0} 02755
+expect 0 prependacl ${n0} user:65534:write_acl::allow
+expect 0 -u 65534 -g 65534 prependacl ${n0} user:65534:write_data::allow
+expect 02755 stat ${n0} mode
+expect 0 unlink ${n0}
+
+# Check whether modifying the ACL by not-owner preserves the sticky bit.
+expect 0 mkdir ${n0} 01755
+expect 0 prependacl ${n0} user:65534:write_acl::allow
+expect 0 -u 65534 -g 65534 prependacl ${n0} user:65534:write_data::allow
+expect 01755 stat ${n0} mode
+expect 0 rmdir ${n0}
+
+# Clearing the SUID and SGID bits when being written to by non-owner
+# is checked in chmod/12.t.
+
 cd ${cdir}
 expect 0 rmdir ${n2}

==== //depot/projects/soc2008/trasz_nfs4acl/tools/regression/fstest/tests/granular/03.t#2 (text+ko) ====

@@ -5,34 +5,127 @@
 dir=`dirname $0`
 . ${dir}/../misc.sh
 
-echo "1..15"
+echo "1..65"
 
 n0=`namegen`
 n1=`namegen`
 n2=`namegen`
+n3=`namegen`
 
 expect 0 mkdir ${n2} 0755
+expect 0 mkdir ${n3} 0777
 cdir=`pwd`
 cd ${n2}
 
-# Tests 2..8 DELETE_CHILD denied on writable directory.
+# Unlink allowed on writable directory.
 expect 0 create ${n0} 0644
 expect EACCES -u 65534 -g 65534 unlink ${n0}
 expect 0 prependacl . user:65534:write_data::allow
 expect 0 -u 65534 -g 65534 unlink ${n0}
+
+# Moving file elsewhere allowed on writable directory.
 expect 0 create ${n0} 0644
+expect 0 prependacl . user:65534:write_data::deny
+expect EACCES -u 65534 -g 65534 rename ${n0} ../${n3}/${n0}
+expect 0 prependacl . user:65534:write_data::allow
+expect 0 -u 65534 -g 65534 rename ${n0} ../${n3}/${n0}
+
+# Moving file from elsewhere allowed on writable directory.
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+expect 0 -u 65534 -g 65534 unlink ${n0}
+
+# Moving file from elsewhere overwriting local file allowed
+# on writable directory.
+expect 0 create ${n0} 0644
+expect 0 create ../${n3}/${n0} 0644
+expect 0 prependacl . user:65534:write_data::deny
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+expect 0 prependacl . user:65534:write_data::allow
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+expect 0 -u 65534 -g 65534 unlink ${n0}
+
+# Denied DELETE changes nothing wrt removing.
+expect 0 create ${n0} 0644
+expect 0 prependacl ${n0} user:65534:delete::deny
+expect 0 -u 65534 -g 65534 unlink ${n0}
+
+# Denied DELETE changes nothing wrt moving elsewhere or from elsewhere.
+expect 0 create ${n0} 0644
+expect 0 -u 65534 -g 65534 rename ${n0} ../${n3}/${n0}
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+expect 0 -u 65534 -g 65534 unlink ${n0}
+
+# DELETE_CHILD denies unlink on writable directory.
+expect 0 create ${n0} 0644
 expect 0 prependacl . user:65534:delete_child::deny
-expect EPERM -u 65534 -g 65534 unlink ${n0}
+expect EACCES -u 65534 -g 65534 unlink ${n0}
+expect 0 unlink ${n0}
+
+# DELETE_CHILD denies moving file elsewhere.
+expect 0 create ${n0} 0644
+expect EACCES -u 65534 -g 65534 rename ${n0} ../${n3}/${n0}
+expect 0 rename ${n0} ../${n3}/${n0}
+
+# DELETE_CHILD does not deny moving file from elsewhere
+# to a writable directory.
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+
+# DELETE_CHILD denies moving file from elsewhere
+# to a writable directory overwriting local file.
+expect 0 create ../${n3}/${n0} 0644
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+
+# DELETE allowed on file allows for unlinking, no matter
+# what permissions on containing directory are.
+expect 0 prependacl ${n0} user:65534:delete::allow
+expect 0 -u 65534 -g 65534 unlink ${n0}
+
+# Same for moving the file elsewhere.
+expect 0 create ${n0} 0644
+expect 0 prependacl ${n0} user:65534:delete::allow
+expect 0 -u 65534 -g 65534 rename ${n0} ../${n3}/${n0}
+
+# Same for moving the file from elsewhere into a writable
+# directory with DELETE_CHILD denied.
+expect 0 -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+expect 0 unlink ${n0}
 
-# Tests 9..15 DELETE allowed on file.
-expect EPERM -u 65534 -g 65534 unlink ${n0}
+# DELETE does not allow for overwriting a file in a unwritable
+# directory with DELETE_CHILD denied.
+expect 0 create ${n0} 0644
+expect 0 create ../${n3}/${n0} 0644
+expect 0 prependacl . user:65534:write_data::deny
+expect 0 prependacl . user:65534:delete_child::deny
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
 expect 0 prependacl ${n0} user:65534:delete::allow
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+
+# But it allows for plain deletion.
 expect 0 -u 65534 -g 65534 unlink ${n0}
 
-# DELETE_CHILD allowed on directory.
+# DELETE_CHILD allowed on unwritable directory.
 expect 0 create ${n0} 0644
 expect 0 prependacl . user:65534:delete_child::allow
 expect 0 -u 65534 -g 65534 unlink ${n0}
 
+# Moving things elsewhere is allowed.
+expect 0 create ${n0} 0644
+expect 0 -u 65534 -g 65534 rename ${n0} ../${n3}/${n0}
+
+# Moving things back is not.
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+
+# Even if we're overwriting.
+expect 0 create ${n0} 0644
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+
+# Even if we have DELETE on the existing file.
+expect 0 prependacl ${n0} user:65534:delete::allow
+expect EACCES -u 65534 -g 65534 rename ../${n3}/${n0} ${n0}
+
+# Denied DELETE changes nothing wrt removing.
+expect 0 prependacl ${n0} user:65534:delete::deny
+expect 0 -u 65534 -g 65534 unlink ${n0}
+
 cd ${cdir}
 expect 0 rmdir ${n2}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808111259.m7BCxs0r013050>