All of lore.kernel.org
 help / color / mirror / Atom feed
* virtiofs and its optional xattr support vs. fs_use_xattr
@ 2020-12-07 14:42 Ondrej Mosnacek
  2020-12-07 15:03 ` Paul Moore
  2020-12-07 17:17 ` James Carter
  0 siblings, 2 replies; 26+ messages in thread
From: Ondrej Mosnacek @ 2020-12-07 14:42 UTC (permalink / raw)
  To: SElinux list
  Cc: Stephen Smalley, Paul Moore, Vivek Goyal, Daniel Walsh, Zdenek Pytela

Hi everyone,

In [1] we ran into a problem with the current handling of filesystem
labeling rules. Basically, it is only possible to specify either
genfscon or fs_use_xattr for a given filesystem, but in the case of
virtiofs, certain mounts may support security xattrs, while other ones
may not.

So we can't use the xattr support by adding fs_use_xattr virtiofs
(...); to the policy, because then a non-xattr mount will fail
(SELinux does a mount-time check on the root inode to make sure that
the xattr handler works), but we also don't want to stay on genfscon,
because then we can't relabel files.

So my question is how to best address this? One option is to use a
similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
labeling, but that's ugly and requires hard-coding another FS name in
the selinux code. The only other alternative I could come up with is
to add a new FS labeling statement that would specify some kind of
mixed genfscon / fs_use_xattr behavior. That would be a better
long-term solution, but leads to more questions on how such statement
should actually work... Should it work the cgroupfs way, giving a
default label to everything and allowing to set/change labels via
xattrs? Or should it rather just detect xattrs support and switch
between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
on that? In the latter case, should the statement specify two contexts
(one for fs_use_xattr and another one for genfscon) or just one for
both behaviors?

Any thoughts/pointers welcome.

[1] https://github.com/fedora-selinux/selinux-policy/pull/478

--
Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 14:42 virtiofs and its optional xattr support vs. fs_use_xattr Ondrej Mosnacek
@ 2020-12-07 15:03 ` Paul Moore
  2020-12-07 20:52     ` [Virtio-fs] " Vivek Goyal
  2020-12-07 17:17 ` James Carter
  1 sibling, 1 reply; 26+ messages in thread
From: Paul Moore @ 2020-12-07 15:03 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: SElinux list, Stephen Smalley, Vivek Goyal, Daniel Walsh, Zdenek Pytela

On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> Hi everyone,
>
> In [1] we ran into a problem with the current handling of filesystem
> labeling rules. Basically, it is only possible to specify either
> genfscon or fs_use_xattr for a given filesystem, but in the case of
> virtiofs, certain mounts may support security xattrs, while other ones
> may not.

Quickly skimming the linked GH issue, it appears that the problem
really lies in the fact that virtiofs allows one to enable/disable
xattrs at mount time.  What isn't clear to me is why one would need to
disable xattrs, can you explain that use case?  Why does enabling
xattrs in virtiofs cause problems?

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 14:42 virtiofs and its optional xattr support vs. fs_use_xattr Ondrej Mosnacek
  2020-12-07 15:03 ` Paul Moore
@ 2020-12-07 17:17 ` James Carter
  2020-12-08 23:45   ` Paul Moore
  1 sibling, 1 reply; 26+ messages in thread
From: James Carter @ 2020-12-07 17:17 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: SElinux list, Stephen Smalley, Paul Moore, Vivek Goyal,
	Daniel Walsh, Zdenek Pytela

On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> Hi everyone,
>
> In [1] we ran into a problem with the current handling of filesystem
> labeling rules. Basically, it is only possible to specify either
> genfscon or fs_use_xattr for a given filesystem, but in the case of
> virtiofs, certain mounts may support security xattrs, while other ones
> may not.
>
> So we can't use the xattr support by adding fs_use_xattr virtiofs
> (...); to the policy, because then a non-xattr mount will fail
> (SELinux does a mount-time check on the root inode to make sure that
> the xattr handler works), but we also don't want to stay on genfscon,
> because then we can't relabel files.
>
> So my question is how to best address this? One option is to use a
> similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> labeling, but that's ugly and requires hard-coding another FS name in
> the selinux code. The only other alternative I could come up with is
> to add a new FS labeling statement that would specify some kind of
> mixed genfscon / fs_use_xattr behavior. That would be a better
> long-term solution, but leads to more questions on how such statement
> should actually work... Should it work the cgroupfs way, giving a
> default label to everything and allowing to set/change labels via
> xattrs? Or should it rather just detect xattrs support and switch
> between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> on that? In the latter case, should the statement specify two contexts
> (one for fs_use_xattr and another one for genfscon) or just one for
> both behaviors?
>

I don't think adding a new statement is necessary. It seems like
allowing both fs_use_xattr and genfscon rules for the filesystem in
policy and then using the fs_use_xattr rule if xattrs are supported
while falling back to the genfscon rule if they are not would do what
you need.

Jim

> Any thoughts/pointers welcome.
>
> [1] https://github.com/fedora-selinux/selinux-policy/pull/478
>
> --
> Ondrej Mosnacek
> Software Engineer, Platform Security - SELinux kernel
> Red Hat, Inc.
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 15:03 ` Paul Moore
@ 2020-12-07 20:52     ` Vivek Goyal
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2020-12-07 20:52 UTC (permalink / raw)
  To: Paul Moore
  Cc: Ondrej Mosnacek, SElinux list, Stephen Smalley, Daniel Walsh,
	Zdenek Pytela, virtio-fs-list, Miklos Szeredi

On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > Hi everyone,
> >
> > In [1] we ran into a problem with the current handling of filesystem
> > labeling rules. Basically, it is only possible to specify either
> > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > virtiofs, certain mounts may support security xattrs, while other ones
> > may not.
> 

[ cc virtio-fs list and miklos ]
> Quickly skimming the linked GH issue, it appears that the problem
> really lies in the fact that virtiofs allows one to enable/disable
> xattrs at mount time.  What isn't clear to me is why one would need to
> disable xattrs, can you explain that use case?  Why does enabling
> xattrs in virtiofs cause problems?

Its not exactly a mount time option. Its a virtiofs file server option.

xattr support by default is disabled because it has performance
penalty. Users can enable it if they want to.

So if virtiofsd starts without xattr support and somebody runs a
VM with SELinux enabled, they should still be able to mount virtiofs,
I guess (instead of failing it).

Thanks
Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Virtio-fs] virtiofs and its optional xattr support vs. fs_use_xattr
@ 2020-12-07 20:52     ` Vivek Goyal
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2020-12-07 20:52 UTC (permalink / raw)
  To: Paul Moore
  Cc: SElinux list, Stephen Smalley, Ondrej Mosnacek, virtio-fs-list,
	Zdenek Pytela

On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > Hi everyone,
> >
> > In [1] we ran into a problem with the current handling of filesystem
> > labeling rules. Basically, it is only possible to specify either
> > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > virtiofs, certain mounts may support security xattrs, while other ones
> > may not.
> 

[ cc virtio-fs list and miklos ]
> Quickly skimming the linked GH issue, it appears that the problem
> really lies in the fact that virtiofs allows one to enable/disable
> xattrs at mount time.  What isn't clear to me is why one would need to
> disable xattrs, can you explain that use case?  Why does enabling
> xattrs in virtiofs cause problems?

Its not exactly a mount time option. Its a virtiofs file server option.

xattr support by default is disabled because it has performance
penalty. Users can enable it if they want to.

So if virtiofsd starts without xattr support and somebody runs a
VM with SELinux enabled, they should still be able to mount virtiofs,
I guess (instead of failing it).

Thanks
Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Virtio-fs] virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 20:52     ` [Virtio-fs] " Vivek Goyal
  (?)
@ 2020-12-07 21:06     ` Harry G. Coin
  -1 siblings, 0 replies; 26+ messages in thread
From: Harry G. Coin @ 2020-12-07 21:06 UTC (permalink / raw)
  To: virtio-fs


On 12/7/20 2:52 PM, Vivek Goyal wrote:
> On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
>> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>> Hi everyone,
>>>
>>> In [1] we ran into a problem with the current handling of filesystem
>>> labeling rules. Basically, it is only possible to specify either
>>> genfscon or fs_use_xattr for a given filesystem, but in the case of
>>> virtiofs, certain mounts may support security xattrs, while other ones
>>> may not.
> [ cc virtio-fs list and miklos ]
>> Quickly skimming the linked GH issue, it appears that the problem
>> really lies in the fact that virtiofs allows one to enable/disable
>> xattrs at mount time.  What isn't clear to me is why one would need to
>> disable xattrs, can you explain that use case?  Why does enabling
>> xattrs in virtiofs cause problems?
> Its not exactly a mount time option. Its a virtiofs file server option.
>
> xattr support by default is disabled because it has performance
> penalty. Users can enable it if they want to.
>
> So if virtiofsd starts without xattr support and somebody runs a
> VM with SELinux enabled, they should still be able to mount virtiofs,
> I guess (instead of failing it).

I think the earlier virtio mount docs permitted an immutable SElinux
spec for everything in a virtiofs mount , whether or not the the
underlying host had xattrs enabled.  Should the mount fail or should
there be a default SELinux spec for the case there are no xattrs in the
host and SELinux is running?   There's a question for which good
arguments exist on both sides.    Case:  I installed a package that had
'restorecon' commands in the install script, but that otherwise
presupposed no SELinux awareness on the user's part.   All the 'I want
it to just work' users saw was a failed package installation.  The post
of a bug to the package maintainers, who then post a bug to the selinux
devs. The SELinux support folks all say 'well it worked for us', and all
the virtiofs users running on xattr enabled hosts 'see no problem'. 
Prevent that frustration:  it's better to fail at mount time on the
guest if selinux is enabled on the guest, xattrs and are not available
in the host, with a very visible 'do this to fix it' error message. 
That way you generate awareness of a 'selinux issue' before it's a mystery.


Harry





^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 20:52     ` [Virtio-fs] " Vivek Goyal
@ 2020-12-07 21:22       ` Dominick Grift
  -1 siblings, 0 replies; 26+ messages in thread
From: Dominick Grift @ 2020-12-07 21:22 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Paul Moore, Ondrej Mosnacek, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela, virtio-fs-list, Miklos Szeredi

Vivek Goyal <vgoyal@redhat.com> writes:

> On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
>> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>> >
>> > Hi everyone,
>> >
>> > In [1] we ran into a problem with the current handling of filesystem
>> > labeling rules. Basically, it is only possible to specify either
>> > genfscon or fs_use_xattr for a given filesystem, but in the case of
>> > virtiofs, certain mounts may support security xattrs, while other ones
>> > may not.
>> 
>
> [ cc virtio-fs list and miklos ]
>> Quickly skimming the linked GH issue, it appears that the problem
>> really lies in the fact that virtiofs allows one to enable/disable
>> xattrs at mount time.  What isn't clear to me is why one would need to
>> disable xattrs, can you explain that use case?  Why does enabling
>> xattrs in virtiofs cause problems?
>
> Its not exactly a mount time option. Its a virtiofs file server option.
>
> xattr support by default is disabled because it has performance
> penalty. Users can enable it if they want to.

if SELinux is enabled then you should preferably just use fs_use xattr unconditionally

>
> So if virtiofsd starts without xattr support and somebody runs a
> VM with SELinux enabled, they should still be able to mount virtiofs,
> I guess (instead of failing it).

SELinux requires that everything is always labeled one way or another
and so if SELinux is enabled one should either use genfscon or fs_use xattr

Since is support fs_use xattr that should be the default and if any
would for any reason want to replace that with genfscon then that is
something they have to address (by excluding the fs_use xattr rule and
replacing it with a genfscon rule (not sure why anyone would ever want
that)

Gist is that if SELinux is enabled then one of the two should be
present, preferably fs_use xattr (so thats the default).

>
> Thanks
> Vivek
>

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Virtio-fs] virtiofs and its optional xattr support vs. fs_use_xattr
@ 2020-12-07 21:22       ` Dominick Grift
  0 siblings, 0 replies; 26+ messages in thread
From: Dominick Grift @ 2020-12-07 21:22 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Paul Moore, SElinux list, Smalley, Ondrej Mosnacek,
	virtio-fs-list, Stephen, Zdenek Pytela

Vivek Goyal <vgoyal@redhat.com> writes:

> On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
>> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>> >
>> > Hi everyone,
>> >
>> > In [1] we ran into a problem with the current handling of filesystem
>> > labeling rules. Basically, it is only possible to specify either
>> > genfscon or fs_use_xattr for a given filesystem, but in the case of
>> > virtiofs, certain mounts may support security xattrs, while other ones
>> > may not.
>> 
>
> [ cc virtio-fs list and miklos ]
>> Quickly skimming the linked GH issue, it appears that the problem
>> really lies in the fact that virtiofs allows one to enable/disable
>> xattrs at mount time.  What isn't clear to me is why one would need to
>> disable xattrs, can you explain that use case?  Why does enabling
>> xattrs in virtiofs cause problems?
>
> Its not exactly a mount time option. Its a virtiofs file server option.
>
> xattr support by default is disabled because it has performance
> penalty. Users can enable it if they want to.

if SELinux is enabled then you should preferably just use fs_use xattr unconditionally

>
> So if virtiofsd starts without xattr support and somebody runs a
> VM with SELinux enabled, they should still be able to mount virtiofs,
> I guess (instead of failing it).

SELinux requires that everything is always labeled one way or another
and so if SELinux is enabled one should either use genfscon or fs_use xattr

Since is support fs_use xattr that should be the default and if any
would for any reason want to replace that with genfscon then that is
something they have to address (by excluding the fs_use xattr rule and
replacing it with a genfscon rule (not sure why anyone would ever want
that)

Gist is that if SELinux is enabled then one of the two should be
present, preferably fs_use xattr (so thats the default).

>
> Thanks
> Vivek
>

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 21:22       ` [Virtio-fs] " Dominick Grift
@ 2020-12-08 14:33         ` Vivek Goyal
  -1 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2020-12-08 14:33 UTC (permalink / raw)
  To: Dominick Grift
  Cc: Paul Moore, Ondrej Mosnacek, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela, virtio-fs-list, Miklos Szeredi

On Mon, Dec 07, 2020 at 10:22:35PM +0100, Dominick Grift wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
> >> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >> >
> >> > Hi everyone,
> >> >
> >> > In [1] we ran into a problem with the current handling of filesystem
> >> > labeling rules. Basically, it is only possible to specify either
> >> > genfscon or fs_use_xattr for a given filesystem, but in the case of
> >> > virtiofs, certain mounts may support security xattrs, while other ones
> >> > may not.
> >> 
> >
> > [ cc virtio-fs list and miklos ]
> >> Quickly skimming the linked GH issue, it appears that the problem
> >> really lies in the fact that virtiofs allows one to enable/disable
> >> xattrs at mount time.  What isn't clear to me is why one would need to
> >> disable xattrs, can you explain that use case?  Why does enabling
> >> xattrs in virtiofs cause problems?
> >
> > Its not exactly a mount time option. Its a virtiofs file server option.
> >
> > xattr support by default is disabled because it has performance
> > penalty. Users can enable it if they want to.
> 
> if SELinux is enabled then you should preferably just use fs_use xattr unconditionally
> 
> >
> > So if virtiofsd starts without xattr support and somebody runs a
> > VM with SELinux enabled, they should still be able to mount virtiofs,
> > I guess (instead of failing it).
> 
> SELinux requires that everything is always labeled one way or another
> and so if SELinux is enabled one should either use genfscon or fs_use xattr
> 
> Since is support fs_use xattr that should be the default and if any
> would for any reason want to replace that with genfscon then that is
> something they have to address (by excluding the fs_use xattr rule and
> replacing it with a genfscon rule (not sure why anyone would ever want
> that)
> 
> Gist is that if SELinux is enabled then one of the two should be
> present, preferably fs_use xattr (so thats the default).

I understand that current state is that one needs to choose either
genfscon or fs_use_xattr depending on filesystem type. Will be nice
if this was more flexibile.

If virtiofsd is running on top of a filesystem which does not support
xattr, then also virtiofs mount will fail.

IOW, with virtiofs both kind of configurations can be easily produed
(xattr enabled or disabled). So none of the defaults (genfscon or
fs_use_xattr) seems to be ideal.

IIUC, policy is assuming that virtiofs will either always support xattr
or will not always support xattrs. Which probably is true for many
filesystems. But not necessarily in this case. So hard coding one
assumption will break other configurations. It will be nice if we there
is a way to fix this in policy.

Thanks
Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Virtio-fs] virtiofs and its optional xattr support vs. fs_use_xattr
@ 2020-12-08 14:33         ` Vivek Goyal
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2020-12-08 14:33 UTC (permalink / raw)
  To: Dominick Grift
  Cc: Paul Moore, SElinux list, Stephen Smalley, Ondrej Mosnacek,
	virtio-fs-list, Zdenek Pytela

On Mon, Dec 07, 2020 at 10:22:35PM +0100, Dominick Grift wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
> >> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >> >
> >> > Hi everyone,
> >> >
> >> > In [1] we ran into a problem with the current handling of filesystem
> >> > labeling rules. Basically, it is only possible to specify either
> >> > genfscon or fs_use_xattr for a given filesystem, but in the case of
> >> > virtiofs, certain mounts may support security xattrs, while other ones
> >> > may not.
> >> 
> >
> > [ cc virtio-fs list and miklos ]
> >> Quickly skimming the linked GH issue, it appears that the problem
> >> really lies in the fact that virtiofs allows one to enable/disable
> >> xattrs at mount time.  What isn't clear to me is why one would need to
> >> disable xattrs, can you explain that use case?  Why does enabling
> >> xattrs in virtiofs cause problems?
> >
> > Its not exactly a mount time option. Its a virtiofs file server option.
> >
> > xattr support by default is disabled because it has performance
> > penalty. Users can enable it if they want to.
> 
> if SELinux is enabled then you should preferably just use fs_use xattr unconditionally
> 
> >
> > So if virtiofsd starts without xattr support and somebody runs a
> > VM with SELinux enabled, they should still be able to mount virtiofs,
> > I guess (instead of failing it).
> 
> SELinux requires that everything is always labeled one way or another
> and so if SELinux is enabled one should either use genfscon or fs_use xattr
> 
> Since is support fs_use xattr that should be the default and if any
> would for any reason want to replace that with genfscon then that is
> something they have to address (by excluding the fs_use xattr rule and
> replacing it with a genfscon rule (not sure why anyone would ever want
> that)
> 
> Gist is that if SELinux is enabled then one of the two should be
> present, preferably fs_use xattr (so thats the default).

I understand that current state is that one needs to choose either
genfscon or fs_use_xattr depending on filesystem type. Will be nice
if this was more flexibile.

If virtiofsd is running on top of a filesystem which does not support
xattr, then also virtiofs mount will fail.

IOW, with virtiofs both kind of configurations can be easily produed
(xattr enabled or disabled). So none of the defaults (genfscon or
fs_use_xattr) seems to be ideal.

IIUC, policy is assuming that virtiofs will either always support xattr
or will not always support xattrs. Which probably is true for many
filesystems. But not necessarily in this case. So hard coding one
assumption will break other configurations. It will be nice if we there
is a way to fix this in policy.

Thanks
Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-08 14:33         ` [Virtio-fs] " Vivek Goyal
@ 2020-12-08 15:13           ` Dominick Grift
  -1 siblings, 0 replies; 26+ messages in thread
From: Dominick Grift @ 2020-12-08 15:13 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Paul Moore, Ondrej Mosnacek, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela, virtio-fs-list, Miklos Szeredi

Vivek Goyal <vgoyal@redhat.com> writes:

> On Mon, Dec 07, 2020 at 10:22:35PM +0100, Dominick Grift wrote:
>> Vivek Goyal <vgoyal@redhat.com> writes:
>> 
>> > On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
>> >> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>> >> >
>> >> > Hi everyone,
>> >> >
>> >> > In [1] we ran into a problem with the current handling of filesystem
>> >> > labeling rules. Basically, it is only possible to specify either
>> >> > genfscon or fs_use_xattr for a given filesystem, but in the case of
>> >> > virtiofs, certain mounts may support security xattrs, while other ones
>> >> > may not.
>> >> 
>> >
>> > [ cc virtio-fs list and miklos ]
>> >> Quickly skimming the linked GH issue, it appears that the problem
>> >> really lies in the fact that virtiofs allows one to enable/disable
>> >> xattrs at mount time.  What isn't clear to me is why one would need to
>> >> disable xattrs, can you explain that use case?  Why does enabling
>> >> xattrs in virtiofs cause problems?
>> >
>> > Its not exactly a mount time option. Its a virtiofs file server option.
>> >
>> > xattr support by default is disabled because it has performance
>> > penalty. Users can enable it if they want to.
>> 
>> if SELinux is enabled then you should preferably just use fs_use xattr unconditionally
>> 
>> >
>> > So if virtiofsd starts without xattr support and somebody runs a
>> > VM with SELinux enabled, they should still be able to mount virtiofs,
>> > I guess (instead of failing it).
>> 
>> SELinux requires that everything is always labeled one way or another
>> and so if SELinux is enabled one should either use genfscon or fs_use xattr
>> 
>> Since is support fs_use xattr that should be the default and if any
>> would for any reason want to replace that with genfscon then that is
>> something they have to address (by excluding the fs_use xattr rule and
>> replacing it with a genfscon rule (not sure why anyone would ever want
>> that)
>> 
>> Gist is that if SELinux is enabled then one of the two should be
>> present, preferably fs_use xattr (so thats the default).
>
> I understand that current state is that one needs to choose either
> genfscon or fs_use_xattr depending on filesystem type. Will be nice
> if this was more flexibile.

>
> If virtiofsd is running on top of a filesystem which does not support
> xattr, then also virtiofs mount will fail.

>
> IOW, with virtiofs both kind of configurations can be easily produed
> (xattr enabled or disabled). So none of the defaults (genfscon or
> fs_use_xattr) seems to be ideal.
>
> IIUC, policy is assuming that virtiofs will either always support xattr
> or will not always support xattrs. Which probably is true for many
> filesystems. But not necessarily in this case. So hard coding one
> assumption will break other configurations. It will be nice if we there
> is a way to fix this in policy.

Sorry I think I misunderstood the issue. James Carter's solution sounds
like the way to go.

Either that or just dont support fs_use xattr and always mount the whole
location with a context specified from configuration (mount -t virtiofs -o context=).


>
> Thanks
> Vivek
>

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Virtio-fs] virtiofs and its optional xattr support vs. fs_use_xattr
@ 2020-12-08 15:13           ` Dominick Grift
  0 siblings, 0 replies; 26+ messages in thread
From: Dominick Grift @ 2020-12-08 15:13 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Paul Moore, SElinux list, Smalley, Ondrej Mosnacek,
	virtio-fs-list, Stephen, Zdenek Pytela

Vivek Goyal <vgoyal@redhat.com> writes:

> On Mon, Dec 07, 2020 at 10:22:35PM +0100, Dominick Grift wrote:
>> Vivek Goyal <vgoyal@redhat.com> writes:
>> 
>> > On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
>> >> On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>> >> >
>> >> > Hi everyone,
>> >> >
>> >> > In [1] we ran into a problem with the current handling of filesystem
>> >> > labeling rules. Basically, it is only possible to specify either
>> >> > genfscon or fs_use_xattr for a given filesystem, but in the case of
>> >> > virtiofs, certain mounts may support security xattrs, while other ones
>> >> > may not.
>> >> 
>> >
>> > [ cc virtio-fs list and miklos ]
>> >> Quickly skimming the linked GH issue, it appears that the problem
>> >> really lies in the fact that virtiofs allows one to enable/disable
>> >> xattrs at mount time.  What isn't clear to me is why one would need to
>> >> disable xattrs, can you explain that use case?  Why does enabling
>> >> xattrs in virtiofs cause problems?
>> >
>> > Its not exactly a mount time option. Its a virtiofs file server option.
>> >
>> > xattr support by default is disabled because it has performance
>> > penalty. Users can enable it if they want to.
>> 
>> if SELinux is enabled then you should preferably just use fs_use xattr unconditionally
>> 
>> >
>> > So if virtiofsd starts without xattr support and somebody runs a
>> > VM with SELinux enabled, they should still be able to mount virtiofs,
>> > I guess (instead of failing it).
>> 
>> SELinux requires that everything is always labeled one way or another
>> and so if SELinux is enabled one should either use genfscon or fs_use xattr
>> 
>> Since is support fs_use xattr that should be the default and if any
>> would for any reason want to replace that with genfscon then that is
>> something they have to address (by excluding the fs_use xattr rule and
>> replacing it with a genfscon rule (not sure why anyone would ever want
>> that)
>> 
>> Gist is that if SELinux is enabled then one of the two should be
>> present, preferably fs_use xattr (so thats the default).
>
> I understand that current state is that one needs to choose either
> genfscon or fs_use_xattr depending on filesystem type. Will be nice
> if this was more flexibile.

>
> If virtiofsd is running on top of a filesystem which does not support
> xattr, then also virtiofs mount will fail.

>
> IOW, with virtiofs both kind of configurations can be easily produed
> (xattr enabled or disabled). So none of the defaults (genfscon or
> fs_use_xattr) seems to be ideal.
>
> IIUC, policy is assuming that virtiofs will either always support xattr
> or will not always support xattrs. Which probably is true for many
> filesystems. But not necessarily in this case. So hard coding one
> assumption will break other configurations. It will be nice if we there
> is a way to fix this in policy.

Sorry I think I misunderstood the issue. James Carter's solution sounds
like the way to go.

Either that or just dont support fs_use xattr and always mount the whole
location with a context specified from configuration (mount -t virtiofs -o context=).


>
> Thanks
> Vivek
>

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 20:52     ` [Virtio-fs] " Vivek Goyal
@ 2020-12-08 23:41       ` Paul Moore
  -1 siblings, 0 replies; 26+ messages in thread
From: Paul Moore @ 2020-12-08 23:41 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Ondrej Mosnacek, SElinux list, Stephen Smalley, Daniel Walsh,
	Zdenek Pytela, virtio-fs-list, Miklos Szeredi

On Mon, Dec 7, 2020 at 3:52 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
> > On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > Hi everyone,
> > >
> > > In [1] we ran into a problem with the current handling of filesystem
> > > labeling rules. Basically, it is only possible to specify either
> > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > virtiofs, certain mounts may support security xattrs, while other ones
> > > may not.
>
> [ cc virtio-fs list and miklos ]
> > Quickly skimming the linked GH issue, it appears that the problem
> > really lies in the fact that virtiofs allows one to enable/disable
> > xattrs at mount time.  What isn't clear to me is why one would need to
> > disable xattrs, can you explain that use case?  Why does enabling
> > xattrs in virtiofs cause problems?
>
> Its not exactly a mount time option. Its a virtiofs file server option.
>
> xattr support by default is disabled because it has performance
> penalty. Users can enable it if they want to.

Oh the number of sins against security that have been committed under
the banner of performance! ;)

Regardless, thanks for the explanation, that helps.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Virtio-fs] virtiofs and its optional xattr support vs. fs_use_xattr
@ 2020-12-08 23:41       ` Paul Moore
  0 siblings, 0 replies; 26+ messages in thread
From: Paul Moore @ 2020-12-08 23:41 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: SElinux list, Stephen Smalley, Ondrej Mosnacek, virtio-fs-list,
	Zdenek Pytela

On Mon, Dec 7, 2020 at 3:52 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> On Mon, Dec 07, 2020 at 10:03:24AM -0500, Paul Moore wrote:
> > On Mon, Dec 7, 2020 at 9:43 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > Hi everyone,
> > >
> > > In [1] we ran into a problem with the current handling of filesystem
> > > labeling rules. Basically, it is only possible to specify either
> > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > virtiofs, certain mounts may support security xattrs, while other ones
> > > may not.
>
> [ cc virtio-fs list and miklos ]
> > Quickly skimming the linked GH issue, it appears that the problem
> > really lies in the fact that virtiofs allows one to enable/disable
> > xattrs at mount time.  What isn't clear to me is why one would need to
> > disable xattrs, can you explain that use case?  Why does enabling
> > xattrs in virtiofs cause problems?
>
> Its not exactly a mount time option. Its a virtiofs file server option.
>
> xattr support by default is disabled because it has performance
> penalty. Users can enable it if they want to.

Oh the number of sins against security that have been committed under
the banner of performance! ;)

Regardless, thanks for the explanation, that helps.

-- 
paul moore
www.paul-moore.com


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-07 17:17 ` James Carter
@ 2020-12-08 23:45   ` Paul Moore
  2020-12-09 15:37     ` James Carter
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Moore @ 2020-12-08 23:45 UTC (permalink / raw)
  To: James Carter
  Cc: Ondrej Mosnacek, SElinux list, Stephen Smalley, Vivek Goyal,
	Daniel Walsh, Zdenek Pytela

On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > Hi everyone,
> >
> > In [1] we ran into a problem with the current handling of filesystem
> > labeling rules. Basically, it is only possible to specify either
> > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > virtiofs, certain mounts may support security xattrs, while other ones
> > may not.
> >
> > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > (...); to the policy, because then a non-xattr mount will fail
> > (SELinux does a mount-time check on the root inode to make sure that
> > the xattr handler works), but we also don't want to stay on genfscon,
> > because then we can't relabel files.
> >
> > So my question is how to best address this? One option is to use a
> > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > labeling, but that's ugly and requires hard-coding another FS name in
> > the selinux code. The only other alternative I could come up with is
> > to add a new FS labeling statement that would specify some kind of
> > mixed genfscon / fs_use_xattr behavior. That would be a better
> > long-term solution, but leads to more questions on how such statement
> > should actually work... Should it work the cgroupfs way, giving a
> > default label to everything and allowing to set/change labels via
> > xattrs? Or should it rather just detect xattrs support and switch
> > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > on that? In the latter case, should the statement specify two contexts
> > (one for fs_use_xattr and another one for genfscon) or just one for
> > both behaviors?
>
> I don't think adding a new statement is necessary. It seems like
> allowing both fs_use_xattr and genfscon rules for the filesystem in
> policy and then using the fs_use_xattr rule if xattrs are supported
> while falling back to the genfscon rule if they are not would do what
> you need.

That seems reasonable to me so long as this ambiguity is okay with the
folks who do policy analysis.  Thinking quickly I'm not sure why it
would be a problem, but the thought did occur while I was typing up
this reply ...

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-08 23:45   ` Paul Moore
@ 2020-12-09 15:37     ` James Carter
  2020-12-10  2:39       ` Paul Moore
  0 siblings, 1 reply; 26+ messages in thread
From: James Carter @ 2020-12-09 15:37 UTC (permalink / raw)
  To: Paul Moore
  Cc: Ondrej Mosnacek, SElinux list, Stephen Smalley, Vivek Goyal,
	Daniel Walsh, Zdenek Pytela

On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > Hi everyone,
> > >
> > > In [1] we ran into a problem with the current handling of filesystem
> > > labeling rules. Basically, it is only possible to specify either
> > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > virtiofs, certain mounts may support security xattrs, while other ones
> > > may not.
> > >
> > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > (...); to the policy, because then a non-xattr mount will fail
> > > (SELinux does a mount-time check on the root inode to make sure that
> > > the xattr handler works), but we also don't want to stay on genfscon,
> > > because then we can't relabel files.
> > >
> > > So my question is how to best address this? One option is to use a
> > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > labeling, but that's ugly and requires hard-coding another FS name in
> > > the selinux code. The only other alternative I could come up with is
> > > to add a new FS labeling statement that would specify some kind of
> > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > long-term solution, but leads to more questions on how such statement
> > > should actually work... Should it work the cgroupfs way, giving a
> > > default label to everything and allowing to set/change labels via
> > > xattrs? Or should it rather just detect xattrs support and switch
> > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > on that? In the latter case, should the statement specify two contexts
> > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > both behaviors?
> >
> > I don't think adding a new statement is necessary. It seems like
> > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > policy and then using the fs_use_xattr rule if xattrs are supported
> > while falling back to the genfscon rule if they are not would do what
> > you need.
>
> That seems reasonable to me so long as this ambiguity is okay with the
> folks who do policy analysis.  Thinking quickly I'm not sure why it
> would be a problem, but the thought did occur while I was typing up
> this reply ...
>

I don't think that it would cause a problem with policy analysis. I
think that you would just assume the genfscon rule is being used,
since it is less fine-grained. It wouldn't be much different from how
booleans are handled.

Jim

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-09 15:37     ` James Carter
@ 2020-12-10  2:39       ` Paul Moore
  2020-12-10  9:29         ` Ondrej Mosnacek
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Moore @ 2020-12-10  2:39 UTC (permalink / raw)
  To: James Carter
  Cc: Ondrej Mosnacek, SElinux list, Stephen Smalley, Vivek Goyal,
	Daniel Walsh, Zdenek Pytela

On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > >
> > > > Hi everyone,
> > > >
> > > > In [1] we ran into a problem with the current handling of filesystem
> > > > labeling rules. Basically, it is only possible to specify either
> > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > may not.
> > > >
> > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > (...); to the policy, because then a non-xattr mount will fail
> > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > because then we can't relabel files.
> > > >
> > > > So my question is how to best address this? One option is to use a
> > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > the selinux code. The only other alternative I could come up with is
> > > > to add a new FS labeling statement that would specify some kind of
> > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > long-term solution, but leads to more questions on how such statement
> > > > should actually work... Should it work the cgroupfs way, giving a
> > > > default label to everything and allowing to set/change labels via
> > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > on that? In the latter case, should the statement specify two contexts
> > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > both behaviors?
> > >
> > > I don't think adding a new statement is necessary. It seems like
> > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > while falling back to the genfscon rule if they are not would do what
> > > you need.
> >
> > That seems reasonable to me so long as this ambiguity is okay with the
> > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > would be a problem, but the thought did occur while I was typing up
> > this reply ...
>
> I don't think that it would cause a problem with policy analysis. I
> think that you would just assume the genfscon rule is being used,
> since it is less fine-grained. It wouldn't be much different from how
> booleans are handled.

Makes sense to me.  Thanks Jim.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-10  2:39       ` Paul Moore
@ 2020-12-10  9:29         ` Ondrej Mosnacek
  2020-12-10 22:17           ` Vivek Goyal
  0 siblings, 1 reply; 26+ messages in thread
From: Ondrej Mosnacek @ 2020-12-10  9:29 UTC (permalink / raw)
  To: Paul Moore
  Cc: James Carter, SElinux list, Stephen Smalley, Vivek Goyal,
	Daniel Walsh, Zdenek Pytela

On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > >
> > > > > Hi everyone,
> > > > >
> > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > labeling rules. Basically, it is only possible to specify either
> > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > may not.
> > > > >
> > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > because then we can't relabel files.
> > > > >
> > > > > So my question is how to best address this? One option is to use a
> > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > the selinux code. The only other alternative I could come up with is
> > > > > to add a new FS labeling statement that would specify some kind of
> > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > long-term solution, but leads to more questions on how such statement
> > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > default label to everything and allowing to set/change labels via
> > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > on that? In the latter case, should the statement specify two contexts
> > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > both behaviors?
> > > >
> > > > I don't think adding a new statement is necessary. It seems like
> > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > while falling back to the genfscon rule if they are not would do what
> > > > you need.
> > >
> > > That seems reasonable to me so long as this ambiguity is okay with the
> > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > would be a problem, but the thought did occur while I was typing up
> > > this reply ...
> >
> > I don't think that it would cause a problem with policy analysis. I
> > think that you would just assume the genfscon rule is being used,
> > since it is less fine-grained. It wouldn't be much different from how
> > booleans are handled.
>
> Makes sense to me.  Thanks Jim.

Okay, so I'll look into switching between use_xattr and use_genfs
based on the availability of xattr support and the presence of
corresponding rules in the policy. Thanks everyone for the fruitful
discussion!

-- 
Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-10  9:29         ` Ondrej Mosnacek
@ 2020-12-10 22:17           ` Vivek Goyal
  2020-12-10 22:24             ` Ondrej Mosnacek
  0 siblings, 1 reply; 26+ messages in thread
From: Vivek Goyal @ 2020-12-10 22:17 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Thu, Dec 10, 2020 at 10:29:02AM +0100, Ondrej Mosnacek wrote:
> On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> > On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > > >
> > > > > > Hi everyone,
> > > > > >
> > > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > > labeling rules. Basically, it is only possible to specify either
> > > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > > may not.
> > > > > >
> > > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > > because then we can't relabel files.
> > > > > >
> > > > > > So my question is how to best address this? One option is to use a
> > > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > > the selinux code. The only other alternative I could come up with is
> > > > > > to add a new FS labeling statement that would specify some kind of
> > > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > > long-term solution, but leads to more questions on how such statement
> > > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > > default label to everything and allowing to set/change labels via
> > > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > > on that? In the latter case, should the statement specify two contexts
> > > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > > both behaviors?
> > > > >
> > > > > I don't think adding a new statement is necessary. It seems like
> > > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > > while falling back to the genfscon rule if they are not would do what
> > > > > you need.
> > > >
> > > > That seems reasonable to me so long as this ambiguity is okay with the
> > > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > > would be a problem, but the thought did occur while I was typing up
> > > > this reply ...
> > >
> > > I don't think that it would cause a problem with policy analysis. I
> > > think that you would just assume the genfscon rule is being used,
> > > since it is less fine-grained. It wouldn't be much different from how
> > > booleans are handled.
> >
> > Makes sense to me.  Thanks Jim.
> 
> Okay, so I'll look into switching between use_xattr and use_genfs
> based on the availability of xattr support and the presence of
> corresponding rules in the policy. Thanks everyone for the fruitful
> discussion!

Hi Ondrej,

So this is now purely a policy change and no changes required in kernel?
If yes, then the patch Dan Walsh proposed, is that good enough or
it needs to be done in a different way.

Thanks
Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-10 22:17           ` Vivek Goyal
@ 2020-12-10 22:24             ` Ondrej Mosnacek
  2020-12-10 22:30               ` Vivek Goyal
  0 siblings, 1 reply; 26+ messages in thread
From: Ondrej Mosnacek @ 2020-12-10 22:24 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Thu, Dec 10, 2020 at 11:17 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> On Thu, Dec 10, 2020 at 10:29:02AM +0100, Ondrej Mosnacek wrote:
> > On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> > > On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > > > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > > > >
> > > > > > > Hi everyone,
> > > > > > >
> > > > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > > > labeling rules. Basically, it is only possible to specify either
> > > > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > > > may not.
> > > > > > >
> > > > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > > > because then we can't relabel files.
> > > > > > >
> > > > > > > So my question is how to best address this? One option is to use a
> > > > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > > > the selinux code. The only other alternative I could come up with is
> > > > > > > to add a new FS labeling statement that would specify some kind of
> > > > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > > > long-term solution, but leads to more questions on how such statement
> > > > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > > > default label to everything and allowing to set/change labels via
> > > > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > > > on that? In the latter case, should the statement specify two contexts
> > > > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > > > both behaviors?
> > > > > >
> > > > > > I don't think adding a new statement is necessary. It seems like
> > > > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > > > while falling back to the genfscon rule if they are not would do what
> > > > > > you need.
> > > > >
> > > > > That seems reasonable to me so long as this ambiguity is okay with the
> > > > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > > > would be a problem, but the thought did occur while I was typing up
> > > > > this reply ...
> > > >
> > > > I don't think that it would cause a problem with policy analysis. I
> > > > think that you would just assume the genfscon rule is being used,
> > > > since it is less fine-grained. It wouldn't be much different from how
> > > > booleans are handled.
> > >
> > > Makes sense to me.  Thanks Jim.
> >
> > Okay, so I'll look into switching between use_xattr and use_genfs
> > based on the availability of xattr support and the presence of
> > corresponding rules in the policy. Thanks everyone for the fruitful
> > discussion!
>
> Hi Ondrej,
>
> So this is now purely a policy change and no changes required in kernel?
> If yes, then the patch Dan Walsh proposed, is that good enough or
> it needs to be done in a different way.

No, this needs a kernel change in SELinux to interpret the policy
rules slightly differently *and* basically Dan's patch (modulo the
typo in the genfscon keyword).

-- 
Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-10 22:24             ` Ondrej Mosnacek
@ 2020-12-10 22:30               ` Vivek Goyal
  2020-12-11  9:15                 ` Ondrej Mosnacek
  0 siblings, 1 reply; 26+ messages in thread
From: Vivek Goyal @ 2020-12-10 22:30 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Thu, Dec 10, 2020 at 11:24:30PM +0100, Ondrej Mosnacek wrote:
> On Thu, Dec 10, 2020 at 11:17 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Thu, Dec 10, 2020 at 10:29:02AM +0100, Ondrej Mosnacek wrote:
> > > On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > > > > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > > > > >
> > > > > > > > Hi everyone,
> > > > > > > >
> > > > > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > > > > labeling rules. Basically, it is only possible to specify either
> > > > > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > > > > may not.
> > > > > > > >
> > > > > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > > > > because then we can't relabel files.
> > > > > > > >
> > > > > > > > So my question is how to best address this? One option is to use a
> > > > > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > > > > the selinux code. The only other alternative I could come up with is
> > > > > > > > to add a new FS labeling statement that would specify some kind of
> > > > > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > > > > long-term solution, but leads to more questions on how such statement
> > > > > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > > > > default label to everything and allowing to set/change labels via
> > > > > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > > > > on that? In the latter case, should the statement specify two contexts
> > > > > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > > > > both behaviors?
> > > > > > >
> > > > > > > I don't think adding a new statement is necessary. It seems like
> > > > > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > > > > while falling back to the genfscon rule if they are not would do what
> > > > > > > you need.
> > > > > >
> > > > > > That seems reasonable to me so long as this ambiguity is okay with the
> > > > > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > > > > would be a problem, but the thought did occur while I was typing up
> > > > > > this reply ...
> > > > >
> > > > > I don't think that it would cause a problem with policy analysis. I
> > > > > think that you would just assume the genfscon rule is being used,
> > > > > since it is less fine-grained. It wouldn't be much different from how
> > > > > booleans are handled.
> > > >
> > > > Makes sense to me.  Thanks Jim.
> > >
> > > Okay, so I'll look into switching between use_xattr and use_genfs
> > > based on the availability of xattr support and the presence of
> > > corresponding rules in the policy. Thanks everyone for the fruitful
> > > discussion!
> >
> > Hi Ondrej,
> >
> > So this is now purely a policy change and no changes required in kernel?
> > If yes, then the patch Dan Walsh proposed, is that good enough or
> > it needs to be done in a different way.
> 
> No, this needs a kernel change in SELinux to interpret the policy
> rules slightly differently *and* basically Dan's patch (modulo the
> typo in the genfscon keyword).

Ok, thanks. Is this kernel change something you will be able to take
care of. I am afraid that I don't know enough to make this change.

Thanks
Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-10 22:30               ` Vivek Goyal
@ 2020-12-11  9:15                 ` Ondrej Mosnacek
  2020-12-11 13:29                   ` Vivek Goyal
  2021-01-04 20:14                   ` Vivek Goyal
  0 siblings, 2 replies; 26+ messages in thread
From: Ondrej Mosnacek @ 2020-12-11  9:15 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Thu, Dec 10, 2020 at 11:31 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> On Thu, Dec 10, 2020 at 11:24:30PM +0100, Ondrej Mosnacek wrote:
> > On Thu, Dec 10, 2020 at 11:17 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > On Thu, Dec 10, 2020 at 10:29:02AM +0100, Ondrej Mosnacek wrote:
> > > > On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > > On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > > > > > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi everyone,
> > > > > > > > >
> > > > > > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > > > > > labeling rules. Basically, it is only possible to specify either
> > > > > > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > > > > > may not.
> > > > > > > > >
> > > > > > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > > > > > because then we can't relabel files.
> > > > > > > > >
> > > > > > > > > So my question is how to best address this? One option is to use a
> > > > > > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > > > > > the selinux code. The only other alternative I could come up with is
> > > > > > > > > to add a new FS labeling statement that would specify some kind of
> > > > > > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > > > > > long-term solution, but leads to more questions on how such statement
> > > > > > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > > > > > default label to everything and allowing to set/change labels via
> > > > > > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > > > > > on that? In the latter case, should the statement specify two contexts
> > > > > > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > > > > > both behaviors?
> > > > > > > >
> > > > > > > > I don't think adding a new statement is necessary. It seems like
> > > > > > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > > > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > > > > > while falling back to the genfscon rule if they are not would do what
> > > > > > > > you need.
> > > > > > >
> > > > > > > That seems reasonable to me so long as this ambiguity is okay with the
> > > > > > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > > > > > would be a problem, but the thought did occur while I was typing up
> > > > > > > this reply ...
> > > > > >
> > > > > > I don't think that it would cause a problem with policy analysis. I
> > > > > > think that you would just assume the genfscon rule is being used,
> > > > > > since it is less fine-grained. It wouldn't be much different from how
> > > > > > booleans are handled.
> > > > >
> > > > > Makes sense to me.  Thanks Jim.
> > > >
> > > > Okay, so I'll look into switching between use_xattr and use_genfs
> > > > based on the availability of xattr support and the presence of
> > > > corresponding rules in the policy. Thanks everyone for the fruitful
> > > > discussion!
> > >
> > > Hi Ondrej,
> > >
> > > So this is now purely a policy change and no changes required in kernel?
> > > If yes, then the patch Dan Walsh proposed, is that good enough or
> > > it needs to be done in a different way.
> >
> > No, this needs a kernel change in SELinux to interpret the policy
> > rules slightly differently *and* basically Dan's patch (modulo the
> > typo in the genfscon keyword).
>
> Ok, thanks. Is this kernel change something you will be able to take
> care of. I am afraid that I don't know enough to make this change.

Yes, it's already on my todo list ;) But it might take some time as
there are a lot of things competing for my attention right now...

-- 
Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-11  9:15                 ` Ondrej Mosnacek
@ 2020-12-11 13:29                   ` Vivek Goyal
  2021-01-04 20:14                   ` Vivek Goyal
  1 sibling, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2020-12-11 13:29 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Fri, Dec 11, 2020 at 10:15:57AM +0100, Ondrej Mosnacek wrote:
> On Thu, Dec 10, 2020 at 11:31 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Thu, Dec 10, 2020 at 11:24:30PM +0100, Ondrej Mosnacek wrote:
> > > On Thu, Dec 10, 2020 at 11:17 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > > On Thu, Dec 10, 2020 at 10:29:02AM +0100, Ondrej Mosnacek wrote:
> > > > > On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi everyone,
> > > > > > > > > >
> > > > > > > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > > > > > > labeling rules. Basically, it is only possible to specify either
> > > > > > > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > > > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > > > > > > may not.
> > > > > > > > > >
> > > > > > > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > > > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > > > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > > > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > > > > > > because then we can't relabel files.
> > > > > > > > > >
> > > > > > > > > > So my question is how to best address this? One option is to use a
> > > > > > > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > > > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > > > > > > the selinux code. The only other alternative I could come up with is
> > > > > > > > > > to add a new FS labeling statement that would specify some kind of
> > > > > > > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > > > > > > long-term solution, but leads to more questions on how such statement
> > > > > > > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > > > > > > default label to everything and allowing to set/change labels via
> > > > > > > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > > > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > > > > > > on that? In the latter case, should the statement specify two contexts
> > > > > > > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > > > > > > both behaviors?
> > > > > > > > >
> > > > > > > > > I don't think adding a new statement is necessary. It seems like
> > > > > > > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > > > > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > > > > > > while falling back to the genfscon rule if they are not would do what
> > > > > > > > > you need.
> > > > > > > >
> > > > > > > > That seems reasonable to me so long as this ambiguity is okay with the
> > > > > > > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > > > > > > would be a problem, but the thought did occur while I was typing up
> > > > > > > > this reply ...
> > > > > > >
> > > > > > > I don't think that it would cause a problem with policy analysis. I
> > > > > > > think that you would just assume the genfscon rule is being used,
> > > > > > > since it is less fine-grained. It wouldn't be much different from how
> > > > > > > booleans are handled.
> > > > > >
> > > > > > Makes sense to me.  Thanks Jim.
> > > > >
> > > > > Okay, so I'll look into switching between use_xattr and use_genfs
> > > > > based on the availability of xattr support and the presence of
> > > > > corresponding rules in the policy. Thanks everyone for the fruitful
> > > > > discussion!
> > > >
> > > > Hi Ondrej,
> > > >
> > > > So this is now purely a policy change and no changes required in kernel?
> > > > If yes, then the patch Dan Walsh proposed, is that good enough or
> > > > it needs to be done in a different way.
> > >
> > > No, this needs a kernel change in SELinux to interpret the policy
> > > rules slightly differently *and* basically Dan's patch (modulo the
> > > typo in the genfscon keyword).
> >
> > Ok, thanks. Is this kernel change something you will be able to take
> > care of. I am afraid that I don't know enough to make this change.
> 
> Yes, it's already on my todo list ;) But it might take some time as
> there are a lot of things competing for my attention right now...

Fair enough. Whenever you get a chance. Thank you.

Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2020-12-11  9:15                 ` Ondrej Mosnacek
  2020-12-11 13:29                   ` Vivek Goyal
@ 2021-01-04 20:14                   ` Vivek Goyal
  2021-01-05 14:00                     ` Ondrej Mosnacek
  1 sibling, 1 reply; 26+ messages in thread
From: Vivek Goyal @ 2021-01-04 20:14 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Fri, Dec 11, 2020 at 10:15:57AM +0100, Ondrej Mosnacek wrote:
> On Thu, Dec 10, 2020 at 11:31 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Thu, Dec 10, 2020 at 11:24:30PM +0100, Ondrej Mosnacek wrote:
> > > On Thu, Dec 10, 2020 at 11:17 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > > On Thu, Dec 10, 2020 at 10:29:02AM +0100, Ondrej Mosnacek wrote:
> > > > > On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi everyone,
> > > > > > > > > >
> > > > > > > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > > > > > > labeling rules. Basically, it is only possible to specify either
> > > > > > > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > > > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > > > > > > may not.
> > > > > > > > > >
> > > > > > > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > > > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > > > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > > > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > > > > > > because then we can't relabel files.
> > > > > > > > > >
> > > > > > > > > > So my question is how to best address this? One option is to use a
> > > > > > > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > > > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > > > > > > the selinux code. The only other alternative I could come up with is
> > > > > > > > > > to add a new FS labeling statement that would specify some kind of
> > > > > > > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > > > > > > long-term solution, but leads to more questions on how such statement
> > > > > > > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > > > > > > default label to everything and allowing to set/change labels via
> > > > > > > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > > > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > > > > > > on that? In the latter case, should the statement specify two contexts
> > > > > > > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > > > > > > both behaviors?
> > > > > > > > >
> > > > > > > > > I don't think adding a new statement is necessary. It seems like
> > > > > > > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > > > > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > > > > > > while falling back to the genfscon rule if they are not would do what
> > > > > > > > > you need.
> > > > > > > >
> > > > > > > > That seems reasonable to me so long as this ambiguity is okay with the
> > > > > > > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > > > > > > would be a problem, but the thought did occur while I was typing up
> > > > > > > > this reply ...
> > > > > > >
> > > > > > > I don't think that it would cause a problem with policy analysis. I
> > > > > > > think that you would just assume the genfscon rule is being used,
> > > > > > > since it is less fine-grained. It wouldn't be much different from how
> > > > > > > booleans are handled.
> > > > > >
> > > > > > Makes sense to me.  Thanks Jim.
> > > > >
> > > > > Okay, so I'll look into switching between use_xattr and use_genfs
> > > > > based on the availability of xattr support and the presence of
> > > > > corresponding rules in the policy. Thanks everyone for the fruitful
> > > > > discussion!
> > > >
> > > > Hi Ondrej,
> > > >
> > > > So this is now purely a policy change and no changes required in kernel?
> > > > If yes, then the patch Dan Walsh proposed, is that good enough or
> > > > it needs to be done in a different way.
> > >
> > > No, this needs a kernel change in SELinux to interpret the policy
> > > rules slightly differently *and* basically Dan's patch (modulo the
> > > typo in the genfscon keyword).
> >
> > Ok, thanks. Is this kernel change something you will be able to take
> > care of. I am afraid that I don't know enough to make this change.
> 
> Yes, it's already on my todo list ;) But it might take some time as
> there are a lot of things competing for my attention right now...

Hi Ondrej,

Sorry to bother you on this. Just curious, if you got a chance to make
progress on this. Will like to solve the issue of SELinux blocking package
installation on virtiofs in VM based containers.

Vivek

> 
> -- 
> Ondrej Mosnacek
> Software Engineer, Platform Security - SELinux kernel
> Red Hat, Inc.
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2021-01-04 20:14                   ` Vivek Goyal
@ 2021-01-05 14:00                     ` Ondrej Mosnacek
  2021-01-05 14:21                       ` Vivek Goyal
  0 siblings, 1 reply; 26+ messages in thread
From: Ondrej Mosnacek @ 2021-01-05 14:00 UTC (permalink / raw)
  To: Vivek Goyal
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Mon, Jan 4, 2021 at 9:15 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> On Fri, Dec 11, 2020 at 10:15:57AM +0100, Ondrej Mosnacek wrote:
> > On Thu, Dec 10, 2020 at 11:31 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > On Thu, Dec 10, 2020 at 11:24:30PM +0100, Ondrej Mosnacek wrote:
> > > > On Thu, Dec 10, 2020 at 11:17 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > > > On Thu, Dec 10, 2020 at 10:29:02AM +0100, Ondrej Mosnacek wrote:
> > > > > > On Thu, Dec 10, 2020 at 3:40 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > > On Wed, Dec 9, 2020 at 10:37 AM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > > On Tue, Dec 8, 2020 at 6:45 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > > > > On Mon, Dec 7, 2020 at 12:17 PM James Carter <jwcart2@gmail.com> wrote:
> > > > > > > > > > On Mon, Dec 7, 2020 at 9:45 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi everyone,
> > > > > > > > > > >
> > > > > > > > > > > In [1] we ran into a problem with the current handling of filesystem
> > > > > > > > > > > labeling rules. Basically, it is only possible to specify either
> > > > > > > > > > > genfscon or fs_use_xattr for a given filesystem, but in the case of
> > > > > > > > > > > virtiofs, certain mounts may support security xattrs, while other ones
> > > > > > > > > > > may not.
> > > > > > > > > > >
> > > > > > > > > > > So we can't use the xattr support by adding fs_use_xattr virtiofs
> > > > > > > > > > > (...); to the policy, because then a non-xattr mount will fail
> > > > > > > > > > > (SELinux does a mount-time check on the root inode to make sure that
> > > > > > > > > > > the xattr handler works), but we also don't want to stay on genfscon,
> > > > > > > > > > > because then we can't relabel files.
> > > > > > > > > > >
> > > > > > > > > > > So my question is how to best address this? One option is to use a
> > > > > > > > > > > similar "hack" as for cgroupfs; i.e. do a kind of mixed genfs-xattr
> > > > > > > > > > > labeling, but that's ugly and requires hard-coding another FS name in
> > > > > > > > > > > the selinux code. The only other alternative I could come up with is
> > > > > > > > > > > to add a new FS labeling statement that would specify some kind of
> > > > > > > > > > > mixed genfscon / fs_use_xattr behavior. That would be a better
> > > > > > > > > > > long-term solution, but leads to more questions on how such statement
> > > > > > > > > > > should actually work... Should it work the cgroupfs way, giving a
> > > > > > > > > > > default label to everything and allowing to set/change labels via
> > > > > > > > > > > xattrs? Or should it rather just detect xattrs support and switch
> > > > > > > > > > > between SECURITY_FS_USE_XATTR and SECURITY_FS_USE_GENFS behavior based
> > > > > > > > > > > on that? In the latter case, should the statement specify two contexts
> > > > > > > > > > > (one for fs_use_xattr and another one for genfscon) or just one for
> > > > > > > > > > > both behaviors?
> > > > > > > > > >
> > > > > > > > > > I don't think adding a new statement is necessary. It seems like
> > > > > > > > > > allowing both fs_use_xattr and genfscon rules for the filesystem in
> > > > > > > > > > policy and then using the fs_use_xattr rule if xattrs are supported
> > > > > > > > > > while falling back to the genfscon rule if they are not would do what
> > > > > > > > > > you need.
> > > > > > > > >
> > > > > > > > > That seems reasonable to me so long as this ambiguity is okay with the
> > > > > > > > > folks who do policy analysis.  Thinking quickly I'm not sure why it
> > > > > > > > > would be a problem, but the thought did occur while I was typing up
> > > > > > > > > this reply ...
> > > > > > > >
> > > > > > > > I don't think that it would cause a problem with policy analysis. I
> > > > > > > > think that you would just assume the genfscon rule is being used,
> > > > > > > > since it is less fine-grained. It wouldn't be much different from how
> > > > > > > > booleans are handled.
> > > > > > >
> > > > > > > Makes sense to me.  Thanks Jim.
> > > > > >
> > > > > > Okay, so I'll look into switching between use_xattr and use_genfs
> > > > > > based on the availability of xattr support and the presence of
> > > > > > corresponding rules in the policy. Thanks everyone for the fruitful
> > > > > > discussion!
> > > > >
> > > > > Hi Ondrej,
> > > > >
> > > > > So this is now purely a policy change and no changes required in kernel?
> > > > > If yes, then the patch Dan Walsh proposed, is that good enough or
> > > > > it needs to be done in a different way.
> > > >
> > > > No, this needs a kernel change in SELinux to interpret the policy
> > > > rules slightly differently *and* basically Dan's patch (modulo the
> > > > typo in the genfscon keyword).
> > >
> > > Ok, thanks. Is this kernel change something you will be able to take
> > > care of. I am afraid that I don't know enough to make this change.
> >
> > Yes, it's already on my todo list ;) But it might take some time as
> > there are a lot of things competing for my attention right now...
>
> Hi Ondrej,
>
> Sorry to bother you on this. Just curious, if you got a chance to make
> progress on this. Will like to solve the issue of SELinux blocking package
> installation on virtiofs in VM based containers.

Hi,

I had a go at it today and I already have a tentative patch. So far
it's passing my initial testing so I should be able to post it to the
list soon.

--
Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: virtiofs and its optional xattr support vs. fs_use_xattr
  2021-01-05 14:00                     ` Ondrej Mosnacek
@ 2021-01-05 14:21                       ` Vivek Goyal
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2021-01-05 14:21 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Paul Moore, James Carter, SElinux list, Stephen Smalley,
	Daniel Walsh, Zdenek Pytela

On Tue, Jan 05, 2021 at 03:00:31PM +0100, Ondrej Mosnacek wrote:

[..]
> > > > > > > Okay, so I'll look into switching between use_xattr and use_genfs
> > > > > > > based on the availability of xattr support and the presence of
> > > > > > > corresponding rules in the policy. Thanks everyone for the fruitful
> > > > > > > discussion!
> > > > > >
> > > > > > Hi Ondrej,
> > > > > >
> > > > > > So this is now purely a policy change and no changes required in kernel?
> > > > > > If yes, then the patch Dan Walsh proposed, is that good enough or
> > > > > > it needs to be done in a different way.
> > > > >
> > > > > No, this needs a kernel change in SELinux to interpret the policy
> > > > > rules slightly differently *and* basically Dan's patch (modulo the
> > > > > typo in the genfscon keyword).
> > > >
> > > > Ok, thanks. Is this kernel change something you will be able to take
> > > > care of. I am afraid that I don't know enough to make this change.
> > >
> > > Yes, it's already on my todo list ;) But it might take some time as
> > > there are a lot of things competing for my attention right now...
> >
> > Hi Ondrej,
> >
> > Sorry to bother you on this. Just curious, if you got a chance to make
> > progress on this. Will like to solve the issue of SELinux blocking package
> > installation on virtiofs in VM based containers.
> 
> Hi,
> 
> I had a go at it today and I already have a tentative patch. So far
> it's passing my initial testing so I should be able to post it to the
> list soon.

Awesome. Looking forward to the final patch.

Vivek


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-01-05 14:23 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07 14:42 virtiofs and its optional xattr support vs. fs_use_xattr Ondrej Mosnacek
2020-12-07 15:03 ` Paul Moore
2020-12-07 20:52   ` Vivek Goyal
2020-12-07 20:52     ` [Virtio-fs] " Vivek Goyal
2020-12-07 21:06     ` Harry G. Coin
2020-12-07 21:22     ` Dominick Grift
2020-12-07 21:22       ` [Virtio-fs] " Dominick Grift
2020-12-08 14:33       ` Vivek Goyal
2020-12-08 14:33         ` [Virtio-fs] " Vivek Goyal
2020-12-08 15:13         ` Dominick Grift
2020-12-08 15:13           ` [Virtio-fs] " Dominick Grift
2020-12-08 23:41     ` Paul Moore
2020-12-08 23:41       ` [Virtio-fs] " Paul Moore
2020-12-07 17:17 ` James Carter
2020-12-08 23:45   ` Paul Moore
2020-12-09 15:37     ` James Carter
2020-12-10  2:39       ` Paul Moore
2020-12-10  9:29         ` Ondrej Mosnacek
2020-12-10 22:17           ` Vivek Goyal
2020-12-10 22:24             ` Ondrej Mosnacek
2020-12-10 22:30               ` Vivek Goyal
2020-12-11  9:15                 ` Ondrej Mosnacek
2020-12-11 13:29                   ` Vivek Goyal
2021-01-04 20:14                   ` Vivek Goyal
2021-01-05 14:00                     ` Ondrej Mosnacek
2021-01-05 14:21                       ` Vivek Goyal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.