diff mbox series

[v1,2/2] generic/513,675: check for security namespace support

Message ID 20220317100842.1237893-3-bxue@redhat.com (mailing list archive)
State New, archived
Headers show
Series check for dedupe and security namespace support | expand

Commit Message

Boyang Xue March 17, 2022, 10:08 a.m. UTC 
From: Boyang Xue <bxue@redhat.com>

Signed-off-by: Boyang Xue <bxue@redhat.com>
---
 tests/generic/513 | 2 ++
 tests/generic/675 | 2 ++
 2 files changed, 4 insertions(+)

Comments

Darrick J. Wong March 17, 2022, 3:30 p.m. UTC | #1 
On Thu, Mar 17, 2022 at 06:08:42PM +0800, bxue@redhat.com wrote:
> From: Boyang Xue <bxue@redhat.com>
> 

Er... what does this patch do, and why?

--D

> Signed-off-by: Boyang Xue <bxue@redhat.com>
> ---
>  tests/generic/513 | 2 ++
>  tests/generic/675 | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/tests/generic/513 b/tests/generic/513
> index dc082787..6e897fa0 100755
> --- a/tests/generic/513
> +++ b/tests/generic/513
> @@ -12,10 +12,12 @@ _begin_fstest auto quick clone
>  # Import common functions.
>  . ./common/filter
>  . ./common/reflink
> +. ./common/attr
>  
>  # real QA test starts here
>  _supported_fs generic
>  _require_scratch_reflink
> +_require_attrs security
>  _require_command "$GETCAP_PROG" getcap
>  _require_command "$SETCAP_PROG" setcap
>  
> diff --git a/tests/generic/675 b/tests/generic/675
> index 23b7e545..311860dc 100755
> --- a/tests/generic/675
> +++ b/tests/generic/675
> @@ -12,12 +12,14 @@ _begin_fstest auto clone quick
>  # Import common functions.
>  . ./common/filter
>  . ./common/reflink
> +. ./common/attr
>  
>  # real QA test starts here
>  
>  # Modify as appropriate.
>  _supported_fs generic
>  _require_user
> +_require_attrs security
>  _require_command "$GETCAP_PROG" getcap
>  _require_command "$SETCAP_PROG" setcap
>  _require_scratch_reflink
> -- 
> 2.27.0
>
Boyang Xue March 17, 2022, 4:31 p.m. UTC | #2 
(Resend due to previous email being rejected. Sorry)

Hi Darrick,

I find the problem when I run xfstests generic/513,675 on nfs

```
FSTYP         -- nfs
PLATFORM      -- Linux/x86_64 kvm103 5.14.0-71.el9.x86_64 #1 SMP
PREEMPT Tue Mar 8 17:49:53 EST 2022
MKFS_OPTIONS  -- localhost:/scratch_TMP
MOUNT_OPTIONS -- -o vers=4.2 -o context=system_u:object_r:root_t:s0
localhost:/scratch_TMP /scratch

generic/513     - output mismatch (see
/root/repo_xfstests/results//generic/513.out.bad)
    --- tests/generic/513.out   2022-03-09 05:26:45.450331405 -0500
    +++ /root/repo_xfstests/results//generic/513.out.bad
2022-03-17 09:34:17.190818281 -0400
    @@ -1,2 +1,18 @@
     QA output created by 513
    +Failed to set capabilities on file '/scratch/bar' (Operation not supported)
```

So nfs doesn't support capabilities, I think we should run a check and
not run the test on filesystems like nfs that don't support it.
I find the file capabilities are implemented by using "security
attribute namespace", and running "_require_attrs security" can do the
check.
After applying this patch, running generic/513, 675 on nfs will be like

```
FSTYP         -- nfs
PLATFORM      -- Linux/x86_64 kvm103 5.14.0-71.el9.x86_64 #1 SMP
PREEMPT Tue Mar 8 17:49:53 EST 2022
MKFS_OPTIONS  -- localhost:/scratch_TMP
MOUNT_OPTIONS -- -o vers=4.2 -o context=system_u:object_r:root_t:s0
localhost:/scratch_TMP /scratch

generic/513     [not run] attr namespace security not supported by
this filesystem type: nfs
generic/675     [not run] attr namespace security not supported by
this filesystem type: nfs
Ran: generic/513 generic/675
Not run: generic/513 generic/675
Passed all 2 tests
```

Thanks,
Boyang


On Thu, Mar 17, 2022 at 11:30 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Thu, Mar 17, 2022 at 06:08:42PM +0800, bxue@redhat.com wrote:
> > From: Boyang Xue <bxue@redhat.com>
> >
>
> Er... what does this patch do, and why?
>
> --D
>
> > Signed-off-by: Boyang Xue <bxue@redhat.com>
> > ---
> >  tests/generic/513 | 2 ++
> >  tests/generic/675 | 2 ++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/tests/generic/513 b/tests/generic/513
> > index dc082787..6e897fa0 100755
> > --- a/tests/generic/513
> > +++ b/tests/generic/513
> > @@ -12,10 +12,12 @@ _begin_fstest auto quick clone
> >  # Import common functions.
> >  . ./common/filter
> >  . ./common/reflink
> > +. ./common/attr
> >
> >  # real QA test starts here
> >  _supported_fs generic
> >  _require_scratch_reflink
> > +_require_attrs security
> >  _require_command "$GETCAP_PROG" getcap
> >  _require_command "$SETCAP_PROG" setcap
> >
> > diff --git a/tests/generic/675 b/tests/generic/675
> > index 23b7e545..311860dc 100755
> > --- a/tests/generic/675
> > +++ b/tests/generic/675
> > @@ -12,12 +12,14 @@ _begin_fstest auto clone quick
> >  # Import common functions.
> >  . ./common/filter
> >  . ./common/reflink
> > +. ./common/attr
> >
> >  # real QA test starts here
> >
> >  # Modify as appropriate.
> >  _supported_fs generic
> >  _require_user
> > +_require_attrs security
> >  _require_command "$GETCAP_PROG" getcap
> >  _require_command "$SETCAP_PROG" setcap
> >  _require_scratch_reflink
> > --
> > 2.27.0
> >
>
Zorro Lang March 23, 2022, 2:38 a.m. UTC | #3 
On Thu, Mar 17, 2022 at 08:30:38AM -0700, Darrick J. Wong wrote:
> On Thu, Mar 17, 2022 at 06:08:42PM +0800, bxue@redhat.com wrote:
> > From: Boyang Xue <bxue@redhat.com>
> > 
> 
> Er... what does this patch do, and why?

I think he's trying to avoid this failure on NFS:
  # setcap cap_net_raw=eip /mnt/nfs/file
  Failed to set capabilities on file `/mnt/nfs/file' (Operation not supported)

NFS doesn't support capabilities for long time, some other filesystems
might have same problem. So maybe we can have a helper [1], then call it as:

_require_capabilities

or

_require_capabilities cap_setuid cap_setgid

And test cases need capabilites as [2], I think we can add this requirement
in generic test at least (or all of them). What do you think?

Thanks,
Zorro


[1]
_require_capabilities()
{
	local capabilities=$*
	local tmp=`mktemp -u $TEST_DIR/capbility-XXXXXX`

	# Try all capabilities by default
	if [ -z "$capabilities" ];then
		capabilities="all"
	fi

	_require_command "$SETCAP_PROG" setcap
	rm -f $tmp.test
	rm -f $tmp.out

	for cap in $capabilities; do
		touch $tmp.test
		$SETCAP_PROG ${cap}=eip $tmp.test > $tmp.out 2>&1
		cat $tmp.out >> $seqres.full
		if grep -q 'Operation not supported' $tmp.out; then
			_notrun "Filesystem $FSTYP does not support to set $cap capability"
		fi
		if grep -q 'Invalid argument' $tmp.out; then
			_notrun "Capability $cap might not be supported by current system"
		fi
		rm -f $tmp.out
		rm -f $tmp.test
	done
	
}

[2]
[zorro@zlang-laptop xfstests-dev]$ grep -rsni SETCAP tests/
tests/btrfs/214:19:_require_command "$SETCAP_PROG" setcap
tests/btrfs/214:75:     $SETCAP_PROG "cap_sys_ptrace+ep cap_sys_nice+ep" "$FS1/foo.bar"
tests/btrfs/214:99:     $SETCAP_PROG "cap_sys_ptrace+ep cap_sys_nice+ep" "$FS1/foo.bar"
tests/btrfs/214:106:    $SETCAP_PROG "cap_sys_ptrace+ep cap_sys_nice+ep" "$FS1/foo.bar"
tests/btrfs/214:115:    $SETCAP_PROG "cap_sys_time+ep cap_syslog+ep" "$FS1/foo.bar"
tests/btrfs/235:29:_require_command "$SETCAP_PROG" setcap
tests/btrfs/235:48:$SETCAP_PROG cap_net_raw=p $SCRATCH_MNT/foo
tests/btrfs/235:67:$SETCAP_PROG cap_net_raw=p $SCRATCH_MNT/bar
tests/btrfs/235:71:$SETCAP_PROG cap_sys_nice=ep $SCRATCH_MNT/foo2
tests/overlay/064:18:_require_command "$SETCAP_PROG" setcap
tests/overlay/064:31:$SETCAP_PROG cap_setuid+ep ${lowerdir}/file1
tests/overlay/064:32:$SETCAP_PROG cap_setuid+ep ${lowerdir}/file2
tests/xfs/296:29:_require_command "$SETCAP_PROG" setcap
tests/xfs/296:39:$SETCAP_PROG cap_setgid,cap_setuid+ep $dump_dir/testfile
tests/generic/675:22:_require_command "$SETCAP_PROG" setcap
tests/generic/675:34:   $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
tests/generic/513:20:_require_command "$SETCAP_PROG" setcap
tests/generic/513:28:$SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/bar
tests/generic/093:35:_require_command "$SETCAP_PROG" "setcap"
tests/generic/093:45:$SETCAP_PROG cap_chown+ep $file
tests/generic/093:53:$SETCAP_PROG cap_chown+ep $file
tests/generic/270:31:   $SETCAP_PROG cap_chown=epi  $tmp.fsstress.bin
tests/generic/270:60:_require_command "$SETCAP_PROG" setcap

> 
> --D
> 
> > Signed-off-by: Boyang Xue <bxue@redhat.com>
> > ---
> >  tests/generic/513 | 2 ++
> >  tests/generic/675 | 2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/tests/generic/513 b/tests/generic/513
> > index dc082787..6e897fa0 100755
> > --- a/tests/generic/513
> > +++ b/tests/generic/513
> > @@ -12,10 +12,12 @@ _begin_fstest auto quick clone
> >  # Import common functions.
> >  . ./common/filter
> >  . ./common/reflink
> > +. ./common/attr
> >  
> >  # real QA test starts here
> >  _supported_fs generic
> >  _require_scratch_reflink
> > +_require_attrs security
> >  _require_command "$GETCAP_PROG" getcap
> >  _require_command "$SETCAP_PROG" setcap
> >  
> > diff --git a/tests/generic/675 b/tests/generic/675
> > index 23b7e545..311860dc 100755
> > --- a/tests/generic/675
> > +++ b/tests/generic/675
> > @@ -12,12 +12,14 @@ _begin_fstest auto clone quick
> >  # Import common functions.
> >  . ./common/filter
> >  . ./common/reflink
> > +. ./common/attr
> >  
> >  # real QA test starts here
> >  
> >  # Modify as appropriate.
> >  _supported_fs generic
> >  _require_user
> > +_require_attrs security
> >  _require_command "$GETCAP_PROG" getcap
> >  _require_command "$SETCAP_PROG" setcap
> >  _require_scratch_reflink
> > -- 
> > 2.27.0
> > 
>
diff mbox series

Patch

diff --git a/tests/generic/513 b/tests/generic/513
index dc082787..6e897fa0 100755
--- a/tests/generic/513
+++ b/tests/generic/513
@@ -12,10 +12,12 @@  _begin_fstest auto quick clone
 # Import common functions.
 . ./common/filter
 . ./common/reflink
+. ./common/attr
 
 # real QA test starts here
 _supported_fs generic
 _require_scratch_reflink
+_require_attrs security
 _require_command "$GETCAP_PROG" getcap
 _require_command "$SETCAP_PROG" setcap
 
diff --git a/tests/generic/675 b/tests/generic/675
index 23b7e545..311860dc 100755
--- a/tests/generic/675
+++ b/tests/generic/675
@@ -12,12 +12,14 @@  _begin_fstest auto clone quick
 # Import common functions.
 . ./common/filter
 . ./common/reflink
+. ./common/attr
 
 # real QA test starts here
 
 # Modify as appropriate.
 _supported_fs generic
 _require_user
+_require_attrs security
 _require_command "$GETCAP_PROG" getcap
 _require_command "$SETCAP_PROG" setcap
 _require_scratch_reflink