Re: [LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[Petr Vorel <pvorel@suse.cz>]

Hi Mete, Amir, Li,

[ Cc Li who knows more about SELinux :) ]

> On Mon, Mar 11, 2024 at 4:53 PM Mete Durlu <meted@linux.ibm.com> wrote:

> > On 3/8/24 14:39, Amir Goldstein wrote:
> > > On Fri, Mar 8, 2024 at 2:43 PM Mete Durlu <meted@linux.ibm.com> wrote:

> > >> When SElinux is configured (comes out of the box on most distros) and
> > >> is configured to enforcing (the default configuration), tests related
> > >> to anonymous pipes return EACCES instead of the expected errno EINVAL.
> > >> Fix the failures caused by the above condition by checking the SElinux
> > >> configuration and adjusting the errno accordingly.

> > > Hi Mete,

> > > Isn't the outcome of the test dependent on the SEpolicy rules?
> > > Not only if it is enforced?

> > > Sorry I have very little experience with SELinux.


> > Hi Amir,

> > I don't have SElinux experience either, on my proposed patch I only
> > considered the default behavior but you are right different SElinux
> > configurations may lead to different outcomes. I skimmed over SElinux
> > wiki a little and now I think trying to verify the SElinux policy would
> > be too cumbersome. Instead I propose two different solutions.

> > 1. We can skip the anonymous pipe test cases when SElinux is in
> >     enforcing state.

> > or

> > 2. We can accept both EACESS and EINVAL as valid errnos when SElinux is
> >     in enforcing state.

> > Personally option 2 sounds better to me since we would get more coverage
> > that way. If either way sounds good I can send a v2 right away. How does
> > that sound?

> option 2 sounds good to me.

Yes, EACESS for enforced SELinux is what we want.

Mete, thank you for handling this. I can confirm it's a problem on SELinux
enforced. And I suppose the current code works, but we need some modifications
(please let me know if you don't have time for v2):

* Put tst_selinux_enforcing() function into LTP library: you need to create
lib/tst_selinux.c and include/tst_selinux.c. For inspiration have look at
lib/tst_lockdown.c vv include/tst_lockdown.h. The reason is obvious: sooner or
later we will reuse this functionality.

* use access(), print also TINFO (similarly to lib/tst_lockdown.c)

* /sys/fs/selinux vs. /selinux, selinux=1 vs. security=selinux (/proc/cmdline)
@Li: TL;DR: reading just /sys/fs/selinux/enforce LGTM, but please check

I suppose we can rely on selinuxfs being mounted on /sys/fs/selinux:

$ mount | grep -i selinux
selinuxfs on /sys/fs/selinux type selinuxfs (rw,nosuid,noexec,relatime)

Long time ago the directory was just /selinux (RHEL 5 or 6?), that's why it's
still checked in shell API testcases/lib/tst_security.sh. These systems are
quite old to run newest LTP, right? From d41415eb5edae [1] I see it was kernel
3.0 => way too old to consider.

I guess we cannot rely on selinux=1 or security=selinux to detect enforce mode.
There is SECURITY_SELINUX_BOOTPARAM, when disabled thus there is no selinux=1
variable in /proc/cmdline, thus we cannot rely on it (instead of using
/sys/fs/selinux).

Also, kernel < v5.1 had SECURITY_SELINUX_BOOTPARAM_VALUE (removed in
be6ec88f41ba94 in v5.1 [2]), another reason not to rely on selinux in
/proc/cmdline.

NOTE: as I noted previously we have support for SELinux (and AppArmor) detection
in shell API testcases/lib/tst_security.sh, we might later create simple C
binary in testcases/lib/ which will call function you create in C API (similarly
to testcases/lib/tst_lockdown_enabled.c), but we can ignore it now.

Kind regards,
Petr

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d41415eb5edae
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=be6ec88f41ba94

> Thanks,
> Amir.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp