* [PATCH] xfsqa: add testcase for ->setattr permission checking
@ 2008-12-02 14:20 Christoph Hellwig
2008-12-03 7:24 ` Timothy Shimmin
2008-12-08 5:48 ` Timothy Shimmin
0 siblings, 2 replies; 6+ messages in thread
From: Christoph Hellwig @ 2008-12-02 14:20 UTC (permalink / raw)
To: Timothy Shimmin, xfs
Index: xfs-cmds-git/xfstests/192
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ xfs-cmds-git/xfstests/192 2008-12-02 14:16:12.000000000 +0000
@@ -0,0 +1,177 @@
+#! /bin/sh
+# FS QA Test No. 192
+#
+# Test permission checks in ->setattr
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2008 Christoph Hellwig.
+#-----------------------------------------------------------------------
+#
+# creator
+owner=hch@lst.de
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup_files; exit \$status" 0 1 2 3 15
+tag="added by qa $seq"
+
+#
+# For some tests we need a secondary group for the qa_user. Currently
+# that's not available in the framework, so the tests using it are
+# commented out.
+#
+#group2=foo
+
+#
+# Create two files, one owned by root, one by the qa_user
+#
+_create_files()
+{
+ touch test.root
+ touch test.${qa_user}
+ chown ${qa_user}:${qa_user} test.${qa_user}
+}
+
+#
+# Remove our files again
+#
+_cleanup_files()
+{
+ rm -f test.${qa_user}
+ rm -f test.root
+}
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+_supported_fs xfs nfs udf
+_supported_os Linux
+
+_require_user
+_need_to_be_root
+
+
+#
+# make sure we have a normal umask set
+#
+umask 022
+
+
+#
+# Test the ATTR_UID case
+#
+echo
+echo "testing ATTR_UID"
+echo
+
+_create_files
+
+echo "user: chown root owned file to qa_user (should fail)"
+su ${qa_user} -c "chown root test.${qa_user}"
+
+echo "user: chown root owned file to root (should fail)"
+su ${qa_user} -c "chown root test.root"
+
+echo "user: chown qa_user owned file to qa_user (should succeed)"
+su ${qa_user} -c "chown ${qa_user} test.${qa_user}"
+
+# this would work without _POSIX_CHOWN_RESTRICTED
+echo "user: chown qa_user owned file to root (should fail)"
+su ${qa_user} -c "chown ${qa_user} test.root"
+
+_cleanup_files
+
+#
+# Test the ATTR_GID case
+#
+echo
+echo "testing ATTR_GID"
+echo
+
+_create_files
+
+echo "user: chgrp root owned file to root (should fail)"
+su ${qa_user} -c "chgrp root test.root"
+
+echo "user: chgrp qa_user owned file to root (should fail)"
+su ${qa_user} -c "chgrp root test.${qa_user}"
+
+echo "user: chgrp root owned file to qa_user (should fail)"
+su ${qa_user} -c "chgrp ${qa_user} test.root"
+
+echo "user: chgrp qa_user owned file to qa_user (should suceed)"
+su ${qa_user} -c "chgrp ${qa_user} test.${qa_user}"
+
+#echo "user: chgrp qa_user owned file to secondary group (should suceed)"
+#su ${qa_user} -c "chgrp ${group2} test.${qa_user}"
+
+_cleanup_files
+
+
+#
+# Test the ATTR_MODE case
+#
+echo
+echo "testing ATTR_MODE"
+echo
+
+_create_files
+
+echo "user: chmod a+r on qa_user owned file (should succeed)"
+su ${qa_user} -c "chmod a+r test.${qa_user}"
+
+echo "user: chmod a+r on root owned file (should fail)"
+su ${qa_user} -c "chmod a+r test.root"
+
+#
+# Setup a file owned by the qa_user, but with a group ID that
+# is not present in the qa_users group list (use root to make it easier for it)
+# and mark it with set sgid bit
+#
+echo "check that the sgid bit is cleared"
+chown ${qa_user}:root test.${qa_user}
+chmod g+s test.${qa_user}
+
+# and let the qa_user change permission bits
+su ${qa_user} -c "chmod a+w test.${qa_user}"
+stat -c '%A' test.${qa_user}
+
+#
+# Setup a file owned by the qa_user and with the suid bit set.
+# A chown by root should not clean the suid bit.
+#
+echo "check that suid bit is not cleared"
+chmod u+s test.${qa_user}
+chmod a+w test.${qa_user}
+stat -c '%A' test.${qa_user}
+
+_cleanup_files
+
+
+#
+# Test ATTR_*TIMES_SET
+#
+echo
+echo "testing ATTR_*TIMES_SET"
+echo
+
+_create_files
+
+echo "user: touch qa_user file (should succeed)"
+su ${qa_user} -c "touch test.${qa_user}"
+
+echo "user: touch root file (should fail)"
+su ${qa_user} -c "touch test.root"
+
+_cleanup_files
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
Index: xfs-cmds-git/xfstests/group
===================================================================
--- xfs-cmds-git.orig/xfstests/group 2008-12-02 14:00:49.000000000 +0000
+++ xfs-cmds-git/xfstests/group 2008-12-02 14:01:01.000000000 +0000
@@ -291,3 +291,4 @@
189 mount auto
190 rw auto
191 nfs4acl auto
+192 auto metadata
Index: xfs-cmds-git/xfstests/192.out
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ xfs-cmds-git/xfstests/192.out 2008-12-02 14:14:49.000000000 +0000
@@ -0,0 +1,38 @@
+QA output created by 192
+
+testing ATTR_UID
+
+user: chown root owned file to qa_user (should fail)
+chown: changing ownership of `test.fsgqa': Operation not permitted
+user: chown root owned file to root (should fail)
+chown: changing ownership of `test.root': Operation not permitted
+user: chown qa_user owned file to qa_user (should succeed)
+user: chown qa_user owned file to root (should fail)
+chown: changing ownership of `test.root': Operation not permitted
+
+testing ATTR_GID
+
+user: chgrp root owned file to root (should fail)
+chgrp: changing group of `test.root': Operation not permitted
+user: chgrp qa_user owned file to root (should fail)
+chgrp: changing group of `test.fsgqa': Operation not permitted
+user: chgrp root owned file to qa_user (should fail)
+chgrp: changing group of `test.root': Operation not permitted
+user: chgrp qa_user owned file to qa_user (should suceed)
+
+testing ATTR_MODE
+
+user: chmod a+r on qa_user owned file (should succeed)
+user: chmod a+r on root owned file (should fail)
+chmod: changing permissions of `test.root': Operation not permitted
+check that the sgid bit is cleared
+-rw-rw-rw-
+check that suid bit is not cleared
+-rwSrw-rw-
+
+testing ATTR_*TIMES_SET
+
+user: touch qa_user file (should succeed)
+user: touch root file (should fail)
+touch: cannot touch `test.root': Permission denied
+*** done
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfsqa: add testcase for ->setattr permission checking
2008-12-02 14:20 [PATCH] xfsqa: add testcase for ->setattr permission checking Christoph Hellwig
@ 2008-12-03 7:24 ` Timothy Shimmin
2008-12-08 5:48 ` Timothy Shimmin
1 sibling, 0 replies; 6+ messages in thread
From: Timothy Shimmin @ 2008-12-03 7:24 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
Hi Christoph,
Thanks for that. I noticed you wanted a 2nd group.
I'd probably prefer to do the test like the acl tests (e.g. 051)
which uses:
- _acl_setup_ids
- _acl_filter_id
- src/runas
They could probably be put in a different common file instead
of common.attr and have the function names changed to be without
_acl as they are not acl specific.
I'm also wondering how I can get away with cat'ing /etc/passwd
and /etc/group. I would have thought it would make more sense
to have a user1, user2, user3, group1, group2, group3 etc..
I can change the above for the acl tests and perhaps create
a common.ids for the funcs which can then be used elsewhere.
(The $qa_user came from the CXFSQA test suite.)
I'll review the actual test tomorrow.
--Tim
Christoph Hellwig wrote:
> Index: xfs-cmds-git/xfstests/192
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ xfs-cmds-git/xfstests/192 2008-12-02 14:16:12.000000000 +0000
> @@ -0,0 +1,177 @@
> +#! /bin/sh
> +# FS QA Test No. 192
> +#
> +# Test permission checks in ->setattr
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2008 Christoph Hellwig.
> +#-----------------------------------------------------------------------
> +#
> +# creator
> +owner=hch@lst.de
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup_files; exit \$status" 0 1 2 3 15
> +tag="added by qa $seq"
> +
> +#
> +# For some tests we need a secondary group for the qa_user. Currently
> +# that's not available in the framework, so the tests using it are
> +# commented out.
> +#
> +#group2=foo
> +
> +#
> +# Create two files, one owned by root, one by the qa_user
> +#
> +_create_files()
> +{
> + touch test.root
> + touch test.${qa_user}
> + chown ${qa_user}:${qa_user} test.${qa_user}
> +}
> +
> +#
> +# Remove our files again
> +#
> +_cleanup_files()
> +{
> + rm -f test.${qa_user}
> + rm -f test.root
> +}
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +# real QA test starts here
> +_supported_fs xfs nfs udf
> +_supported_os Linux
> +
> +_require_user
> +_need_to_be_root
> +
> +
> +#
> +# make sure we have a normal umask set
> +#
> +umask 022
> +
> +
> +#
> +# Test the ATTR_UID case
> +#
> +echo
> +echo "testing ATTR_UID"
> +echo
> +
> +_create_files
> +
> +echo "user: chown root owned file to qa_user (should fail)"
> +su ${qa_user} -c "chown root test.${qa_user}"
> +
> +echo "user: chown root owned file to root (should fail)"
> +su ${qa_user} -c "chown root test.root"
> +
> +echo "user: chown qa_user owned file to qa_user (should succeed)"
> +su ${qa_user} -c "chown ${qa_user} test.${qa_user}"
> +
> +# this would work without _POSIX_CHOWN_RESTRICTED
> +echo "user: chown qa_user owned file to root (should fail)"
> +su ${qa_user} -c "chown ${qa_user} test.root"
> +
> +_cleanup_files
> +
> +#
> +# Test the ATTR_GID case
> +#
> +echo
> +echo "testing ATTR_GID"
> +echo
> +
> +_create_files
> +
> +echo "user: chgrp root owned file to root (should fail)"
> +su ${qa_user} -c "chgrp root test.root"
> +
> +echo "user: chgrp qa_user owned file to root (should fail)"
> +su ${qa_user} -c "chgrp root test.${qa_user}"
> +
> +echo "user: chgrp root owned file to qa_user (should fail)"
> +su ${qa_user} -c "chgrp ${qa_user} test.root"
> +
> +echo "user: chgrp qa_user owned file to qa_user (should suceed)"
> +su ${qa_user} -c "chgrp ${qa_user} test.${qa_user}"
> +
> +#echo "user: chgrp qa_user owned file to secondary group (should suceed)"
> +#su ${qa_user} -c "chgrp ${group2} test.${qa_user}"
> +
> +_cleanup_files
> +
> +
> +#
> +# Test the ATTR_MODE case
> +#
> +echo
> +echo "testing ATTR_MODE"
> +echo
> +
> +_create_files
> +
> +echo "user: chmod a+r on qa_user owned file (should succeed)"
> +su ${qa_user} -c "chmod a+r test.${qa_user}"
> +
> +echo "user: chmod a+r on root owned file (should fail)"
> +su ${qa_user} -c "chmod a+r test.root"
> +
> +#
> +# Setup a file owned by the qa_user, but with a group ID that
> +# is not present in the qa_users group list (use root to make it easier for it)
> +# and mark it with set sgid bit
> +#
> +echo "check that the sgid bit is cleared"
> +chown ${qa_user}:root test.${qa_user}
> +chmod g+s test.${qa_user}
> +
> +# and let the qa_user change permission bits
> +su ${qa_user} -c "chmod a+w test.${qa_user}"
> +stat -c '%A' test.${qa_user}
> +
> +#
> +# Setup a file owned by the qa_user and with the suid bit set.
> +# A chown by root should not clean the suid bit.
> +#
> +echo "check that suid bit is not cleared"
> +chmod u+s test.${qa_user}
> +chmod a+w test.${qa_user}
> +stat -c '%A' test.${qa_user}
> +
> +_cleanup_files
> +
> +
> +#
> +# Test ATTR_*TIMES_SET
> +#
> +echo
> +echo "testing ATTR_*TIMES_SET"
> +echo
> +
> +_create_files
> +
> +echo "user: touch qa_user file (should succeed)"
> +su ${qa_user} -c "touch test.${qa_user}"
> +
> +echo "user: touch root file (should fail)"
> +su ${qa_user} -c "touch test.root"
> +
> +_cleanup_files
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> Index: xfs-cmds-git/xfstests/group
> ===================================================================
> --- xfs-cmds-git.orig/xfstests/group 2008-12-02 14:00:49.000000000 +0000
> +++ xfs-cmds-git/xfstests/group 2008-12-02 14:01:01.000000000 +0000
> @@ -291,3 +291,4 @@
> 189 mount auto
> 190 rw auto
> 191 nfs4acl auto
> +192 auto metadata
> Index: xfs-cmds-git/xfstests/192.out
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ xfs-cmds-git/xfstests/192.out 2008-12-02 14:14:49.000000000 +0000
> @@ -0,0 +1,38 @@
> +QA output created by 192
> +
> +testing ATTR_UID
> +
> +user: chown root owned file to qa_user (should fail)
> +chown: changing ownership of `test.fsgqa': Operation not permitted
> +user: chown root owned file to root (should fail)
> +chown: changing ownership of `test.root': Operation not permitted
> +user: chown qa_user owned file to qa_user (should succeed)
> +user: chown qa_user owned file to root (should fail)
> +chown: changing ownership of `test.root': Operation not permitted
> +
> +testing ATTR_GID
> +
> +user: chgrp root owned file to root (should fail)
> +chgrp: changing group of `test.root': Operation not permitted
> +user: chgrp qa_user owned file to root (should fail)
> +chgrp: changing group of `test.fsgqa': Operation not permitted
> +user: chgrp root owned file to qa_user (should fail)
> +chgrp: changing group of `test.root': Operation not permitted
> +user: chgrp qa_user owned file to qa_user (should suceed)
> +
> +testing ATTR_MODE
> +
> +user: chmod a+r on qa_user owned file (should succeed)
> +user: chmod a+r on root owned file (should fail)
> +chmod: changing permissions of `test.root': Operation not permitted
> +check that the sgid bit is cleared
> +-rw-rw-rw-
> +check that suid bit is not cleared
> +-rwSrw-rw-
> +
> +testing ATTR_*TIMES_SET
> +
> +user: touch qa_user file (should succeed)
> +user: touch root file (should fail)
> +touch: cannot touch `test.root': Permission denied
> +*** done
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfsqa: add testcase for ->setattr permission checking
2008-12-02 14:20 [PATCH] xfsqa: add testcase for ->setattr permission checking Christoph Hellwig
2008-12-03 7:24 ` Timothy Shimmin
@ 2008-12-08 5:48 ` Timothy Shimmin
2008-12-09 9:55 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Timothy Shimmin @ 2008-12-08 5:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
I'd like to check this in.
I can do the uid/gid test allocation later.
There are a few things below that
I want to check with you...
Christoph Hellwig wrote:
> Index: xfs-cmds-git/xfstests/192
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ xfs-cmds-git/xfstests/192 2008-12-02 14:16:12.000000000 +0000
> @@ -0,0 +1,177 @@
> +#! /bin/sh
> +# FS QA Test No. 192
> +#
> +# Test permission checks in ->setattr
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2008 Christoph Hellwig.
> +#-----------------------------------------------------------------------
> +#
> +# creator
> +owner=hch@lst.de
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup_files; exit \$status" 0 1 2 3 15
> +tag="added by qa $seq"
> +
> +#
> +# For some tests we need a secondary group for the qa_user. Currently
> +# that's not available in the framework, so the tests using it are
> +# commented out.
> +#
> +#group2=foo
> +
> +#
> +# Create two files, one owned by root, one by the qa_user
> +#
> +_create_files()
> +{
> + touch test.root
> + touch test.${qa_user}
> + chown ${qa_user}:${qa_user} test.${qa_user}
> +}
> +
> +#
> +# Remove our files again
> +#
> +_cleanup_files()
> +{
> + rm -f test.${qa_user}
> + rm -f test.root
> +}
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +# real QA test starts here
> +_supported_fs xfs nfs udf
> +_supported_os Linux
> +
> +_require_user
> +_need_to_be_root
> +
> +
> +#
> +# make sure we have a normal umask set
> +#
> +umask 022
> +
> +
> +#
> +# Test the ATTR_UID case
> +#
> +echo
> +echo "testing ATTR_UID"
> +echo
> +
> +_create_files
> +
1.
> +echo "user: chown root owned file to qa_user (should fail)"
> +su ${qa_user} -c "chown root test.${qa_user}"
> +
I think the description and command above don't match.
I think we have a swap with subtest 4 below.
Need to either swap descriptions or commands.
2.
> +echo "user: chown root owned file to root (should fail)"
> +su ${qa_user} -c "chown root test.root"
> +
3.
> +echo "user: chown qa_user owned file to qa_user (should succeed)"
> +su ${qa_user} -c "chown ${qa_user} test.${qa_user}"
> +
4.
> +# this would work without _POSIX_CHOWN_RESTRICTED
> +echo "user: chown qa_user owned file to root (should fail)"
> +su ${qa_user} -c "chown ${qa_user} test.root"
> +
> +#
> +# Setup a file owned by the qa_user and with the suid bit set.
> +# A chown by root should not clean the suid bit.
> +#
Typos:
s/clean/clear/
s/suceed/succeed/ in a couple of places.
* It looks like you test the clearing of suid/sgid bits
for setting the mode permission bits and not
for setting ownership as the description suggests;
i.e. you test with chmod instead of chown for clearing of suid/sgid bits
Ideally in the future it would be good to test a few other things too:
* CAP_FOWNER
* CAP_FSETID
* CAP_CHOWN
--Tim
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfsqa: add testcase for ->setattr permission checking
2008-12-08 5:48 ` Timothy Shimmin
@ 2008-12-09 9:55 ` Christoph Hellwig
2008-12-10 3:17 ` Timothy Shimmin
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2008-12-09 9:55 UTC (permalink / raw)
To: Timothy Shimmin; +Cc: Christoph Hellwig, xfs
On Mon, Dec 08, 2008 at 04:48:08PM +1100, Timothy Shimmin wrote:
> 1.
> > +echo "user: chown root owned file to qa_user (should fail)"
> > +su ${qa_user} -c "chown root test.${qa_user}"
> > +
> I think the description and command above don't match.
> I think we have a swap with subtest 4 below.
> Need to either swap descriptions or commands.
Yes, I swapped the descriptions.
> > +#
> > +# Setup a file owned by the qa_user and with the suid bit set.
> > +# A chown by root should not clean the suid bit.
> > +#
>
> Typos:
> s/clean/clear/
>
> s/suceed/succeed/ in a couple of places.
Yeah.
> * It looks like you test the clearing of suid/sgid bits
> for setting the mode permission bits and not
> for setting ownership as the description suggests;
> i.e. you test with chmod instead of chown for clearing of suid/sgid bits
Yes, that's also what I intended too, as XFS had some code to clear
the suid bits for changing permissions, but those shouldn't happen
for the restricted_chown case (and don't even happen in the XFS code,
it's just not obvious when reading the old setattr implementation).
While for sgid we want to clear it on mode changes if the gid
is not in the group list. So what needs fixing here is once again
the comment.
Btw, I just noticed you checked in another testcase as 192. Do you want
a respin or do you want to fix it up yourself?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfsqa: add testcase for ->setattr permission checking
2008-12-09 9:55 ` Christoph Hellwig
@ 2008-12-10 3:17 ` Timothy Shimmin
2008-12-10 4:36 ` Timothy Shimmin
0 siblings, 1 reply; 6+ messages in thread
From: Timothy Shimmin @ 2008-12-10 3:17 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
Christoph Hellwig wrote:
> On Mon, Dec 08, 2008 at 04:48:08PM +1100, Timothy Shimmin wrote:
>> 1.
>>> +echo "user: chown root owned file to qa_user (should fail)"
>>> +su ${qa_user} -c "chown root test.${qa_user}"
>>> +
>> I think the description and command above don't match.
>> I think we have a swap with subtest 4 below.
>> Need to either swap descriptions or commands.
>
> Yes, I swapped the descriptions.
>
>>> +#
>>> +# Setup a file owned by the qa_user and with the suid bit set.
>>> +# A chown by root should not clean the suid bit.
>>> +#
>> Typos:
>> s/clean/clear/
>>
>> s/suceed/succeed/ in a couple of places.
>
> Yeah.
>
>> * It looks like you test the clearing of suid/sgid bits
>> for setting the mode permission bits and not
>> for setting ownership as the description suggests;
>> i.e. you test with chmod instead of chown for clearing of suid/sgid bits
>
> Yes, that's also what I intended too,
Oh okay.
> as XFS had some code to clear
> the suid bits for changing permissions, but those shouldn't happen
> for the restricted_chown case (and don't even happen in the XFS code,
> it's just not obvious when reading the old setattr implementation).
Yeah, I didn't see anything about this in posix under chmod.
But reasonable to test this too - I agree.
> While for sgid we want to clear it on mode changes if the gid
> is not in the group list. So what needs fixing here is once again
> the comment.
>
Ok have found it in posix (thanks to gnb mentioning www.opengroup.org).
For chmod:
"If the calling process does not have appropriate privileges, and
if the group ID of the file does not match the effective group ID
or one of the supplementary group IDs and if the file is a regular
file, bit S_ISGID (set-group-ID on execution) in the file's mode
shall be cleared upon successful return from chmod()."
reg file + file's gid not in process' group set + no approp. privileges
-> clear sgid
Which is what your test did. Cool.
For chown:
"If the specified file is a regular file, one or more of the S_IXUSR,
S_IXGRP, or S_IXOTH bits of the file mode are set, and the process
does not have appropriate privileges, the set-user-ID (S_ISUID) and
set-group-ID (S_ISGID) bits of the file mode shall be cleared upon
successful return from chown(). If the specified file is a regular
file, one or more of the S_IXUSR, S_IXGRP, or S_IXOTH bits of the
file mode are set, and the process has appropriate privileges, it
is implementation-defined whether the set-user-ID and set-group-ID
bits are altered. If the chown() function is successfully invoked
on a file that is not a regular file and one or more of the S_IXUSR,
S_IXGRP, or S_IXOTH bits of the file mode are set, the set-user-ID
and set-group-ID bits may be cleared."
reg file + mode-bits set + no appropriate privileges -> clear suid,sgid
reg file + mode-bits set + appropriate privileges -> maybe clear suid,sgid
non reg file + mode-bits set + chown success on file (??) -> maybe clear suid/sgid
By appropriate privileges, I presume they are referring to extra
capabilities.
I will add a couple of tests for this suid/sgid clear after chown.
> Btw, I just noticed you checked in another testcase as 192. Do you want
> a respin or do you want to fix it up yourself?
Don't worry about any respin.
I'll fix up based on your reply etc. and check in myself.
Thanks,
Tim.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfsqa: add testcase for ->setattr permission checking
2008-12-10 3:17 ` Timothy Shimmin
@ 2008-12-10 4:36 ` Timothy Shimmin
0 siblings, 0 replies; 6+ messages in thread
From: Timothy Shimmin @ 2008-12-10 4:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
Timothy Shimmin wrote:
> For chown:
> "If the specified file is a regular file, one or more of the S_IXUSR,
> S_IXGRP, or S_IXOTH bits of the file mode are set, and the process
> does not have appropriate privileges, the set-user-ID (S_ISUID) and
> set-group-ID (S_ISGID) bits of the file mode shall be cleared upon
> successful return from chown(). If the specified file is a regular
> file, one or more of the S_IXUSR, S_IXGRP, or S_IXOTH bits of the
> file mode are set, and the process has appropriate privileges, it
> is implementation-defined whether the set-user-ID and set-group-ID
> bits are altered. If the chown() function is successfully invoked
> on a file that is not a regular file and one or more of the S_IXUSR,
> S_IXGRP, or S_IXOTH bits of the file mode are set, the set-user-ID
> and set-group-ID bits may be cleared."
>
> reg file + mode-bits set + no appropriate privileges -> clear suid,sgid
> reg file + mode-bits set + appropriate privileges -> maybe clear suid,sgid
> non reg file + mode-bits set + chown success on file (??) -> maybe clear suid/sgid
>
s/mode-bits/exec-mode-bits/
--Tim
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-12-10 4:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-02 14:20 [PATCH] xfsqa: add testcase for ->setattr permission checking Christoph Hellwig
2008-12-03 7:24 ` Timothy Shimmin
2008-12-08 5:48 ` Timothy Shimmin
2008-12-09 9:55 ` Christoph Hellwig
2008-12-10 3:17 ` Timothy Shimmin
2008-12-10 4:36 ` Timothy Shimmin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox