All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selinux: enable genfscon labeling for securityfs
@ 2021-09-15 16:23 Christian Göttsche
  2021-09-15 18:27 ` Paul Moore
  2021-09-28 15:39 ` [PATCH v2] " Christian Göttsche
  0 siblings, 2 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-09-15 16:23 UTC (permalink / raw)
  To: selinux

Add support for genfscon per-file labeling of securityfs files. This allows
for separate labels and therby permissions for different files, e.g.
/sys/kernel/security/integrity/ima/policy.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/hooks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 6517f221d52c..a18626424731 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	    !strcmp(sb->s_type->name, "tracefs") ||
 	    !strcmp(sb->s_type->name, "binder") ||
 	    !strcmp(sb->s_type->name, "bpf") ||
-	    !strcmp(sb->s_type->name, "pstore"))
+	    !strcmp(sb->s_type->name, "pstore") ||
+	    !strcmp(sb->s_type->name, "securityfs"))
 		sbsec->flags |= SE_SBGENFS;
 
 	if (!strcmp(sb->s_type->name, "sysfs") ||
-- 
2.33.0


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

* Re: [PATCH] selinux: enable genfscon labeling for securityfs
  2021-09-15 16:23 [PATCH] selinux: enable genfscon labeling for securityfs Christian Göttsche
@ 2021-09-15 18:27 ` Paul Moore
  2021-09-16 17:41   ` Christian Göttsche
  2021-09-28 15:39 ` [PATCH v2] " Christian Göttsche
  1 sibling, 1 reply; 9+ messages in thread
From: Paul Moore @ 2021-09-15 18:27 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

On Wed, Sep 15, 2021 at 12:24 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Add support for genfscon per-file labeling of securityfs files. This allows
> for separate labels and therby permissions for different files, e.g.
> /sys/kernel/security/integrity/ima/policy.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/hooks.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Hi Christian,

It would be nice if you could add some additional notes on how this
was tested to the description above.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 6517f221d52c..a18626424731 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
>             !strcmp(sb->s_type->name, "tracefs") ||
>             !strcmp(sb->s_type->name, "binder") ||
>             !strcmp(sb->s_type->name, "bpf") ||
> -           !strcmp(sb->s_type->name, "pstore"))
> +           !strcmp(sb->s_type->name, "pstore") ||
> +           !strcmp(sb->s_type->name, "securityfs"))
>                 sbsec->flags |= SE_SBGENFS;
>
>         if (!strcmp(sb->s_type->name, "sysfs") ||
> --
> 2.33.0

-- 
paul moore
www.paul-moore.com

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

* Re: [PATCH] selinux: enable genfscon labeling for securityfs
  2021-09-15 18:27 ` Paul Moore
@ 2021-09-16 17:41   ` Christian Göttsche
  2021-09-17  2:07     ` Paul Moore
  0 siblings, 1 reply; 9+ messages in thread
From: Christian Göttsche @ 2021-09-16 17:41 UTC (permalink / raw)
  To: Paul Moore; +Cc: SElinux list

On Wed, 15 Sept 2021 at 20:28, Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Sep 15, 2021 at 12:24 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Add support for genfscon per-file labeling of securityfs files. This allows
> > for separate labels and therby permissions for different files, e.g.
> > /sys/kernel/security/integrity/ima/policy.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  security/selinux/hooks.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
>
> Hi Christian,
>
> It would be nice if you could add some additional notes on how this
> was tested to the description above.
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 6517f221d52c..a18626424731 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> >             !strcmp(sb->s_type->name, "tracefs") ||
> >             !strcmp(sb->s_type->name, "binder") ||
> >             !strcmp(sb->s_type->name, "bpf") ||
> > -           !strcmp(sb->s_type->name, "pstore"))
> > +           !strcmp(sb->s_type->name, "pstore") ||
> > +           !strcmp(sb->s_type->name, "securityfs"))
> >                 sbsec->flags |= SE_SBGENFS;
> >
> >         if (!strcmp(sb->s_type->name, "sysfs") ||
> > --
> > 2.33.0
>
> --
> paul moore
> www.paul-moore.com

Something like:

    Add support for genfscon per-file labeling of securityfs files. This allows
    for separate labels and thereby access control for different files.
    For example a genfscon statement
        genfscon securityfs /integrity/ima/policy
system_u:object_r:ima_policy_t:s0
    will set a specific label to the IMA policy file and thus allow to
control the ability
    to set the IMA policy.
    Setting labels directly, e.g. via chcon(1) or setfiles(8), is
still not supported.

?

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

* Re: [PATCH] selinux: enable genfscon labeling for securityfs
  2021-09-16 17:41   ` Christian Göttsche
@ 2021-09-17  2:07     ` Paul Moore
  2021-09-17  8:00       ` Christian Göttsche
  2021-09-27 21:56       ` Paul Moore
  0 siblings, 2 replies; 9+ messages in thread
From: Paul Moore @ 2021-09-17  2:07 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: SElinux list

On Thu, Sep 16, 2021 at 1:41 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
> On Wed, 15 Sept 2021 at 20:28, Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Wed, Sep 15, 2021 at 12:24 PM Christian Göttsche
> > <cgzones@googlemail.com> wrote:
> > >
> > > Add support for genfscon per-file labeling of securityfs files. This allows
> > > for separate labels and therby permissions for different files, e.g.
> > > /sys/kernel/security/integrity/ima/policy.
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > ---
> > >  security/selinux/hooks.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Hi Christian,
> >
> > It would be nice if you could add some additional notes on how this
> > was tested to the description above.
> >
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 6517f221d52c..a18626424731 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> > >             !strcmp(sb->s_type->name, "tracefs") ||
> > >             !strcmp(sb->s_type->name, "binder") ||
> > >             !strcmp(sb->s_type->name, "bpf") ||
> > > -           !strcmp(sb->s_type->name, "pstore"))
> > > +           !strcmp(sb->s_type->name, "pstore") ||
> > > +           !strcmp(sb->s_type->name, "securityfs"))
> > >                 sbsec->flags |= SE_SBGENFS;
> > >
> > >         if (!strcmp(sb->s_type->name, "sysfs") ||
> > > --
> > > 2.33.0
> >
> > --
> > paul moore
> > www.paul-moore.com
>
> Something like:
>
>     Add support for genfscon per-file labeling of securityfs files. This allows
>     for separate labels and thereby access control for different files.
>     For example a genfscon statement
>         genfscon securityfs /integrity/ima/policy
> system_u:object_r:ima_policy_t:s0
>     will set a specific label to the IMA policy file and thus allow to
> control the ability
>     to set the IMA policy.
>     Setting labels directly, e.g. via chcon(1) or setfiles(8), is
> still not supported.
>
> ?

That's a much better description of the functionality, especially for
those who may not be very familiar with SELinux, thank you.  However I
was hoping to also hear some confirmation that you have tested this
and it worked without problem?

-- 
paul moore
www.paul-moore.com

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

* Re: [PATCH] selinux: enable genfscon labeling for securityfs
  2021-09-17  2:07     ` Paul Moore
@ 2021-09-17  8:00       ` Christian Göttsche
  2021-09-17 13:27         ` Stephen Smalley
  2021-09-27 21:56       ` Paul Moore
  1 sibling, 1 reply; 9+ messages in thread
From: Christian Göttsche @ 2021-09-17  8:00 UTC (permalink / raw)
  To: Paul Moore; +Cc: SElinux list

On Fri, 17 Sept 2021 at 04:07, Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Sep 16, 2021 at 1:41 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> > On Wed, 15 Sept 2021 at 20:28, Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On Wed, Sep 15, 2021 at 12:24 PM Christian Göttsche
> > > <cgzones@googlemail.com> wrote:
> > > >
> > > > Add support for genfscon per-file labeling of securityfs files. This allows
> > > > for separate labels and therby permissions for different files, e.g.
> > > > /sys/kernel/security/integrity/ima/policy.
> > > >
> > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > ---
> > > >  security/selinux/hooks.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > Hi Christian,
> > >
> > > It would be nice if you could add some additional notes on how this
> > > was tested to the description above.
> > >
> > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > > index 6517f221d52c..a18626424731 100644
> > > > --- a/security/selinux/hooks.c
> > > > +++ b/security/selinux/hooks.c
> > > > @@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> > > >             !strcmp(sb->s_type->name, "tracefs") ||
> > > >             !strcmp(sb->s_type->name, "binder") ||
> > > >             !strcmp(sb->s_type->name, "bpf") ||
> > > > -           !strcmp(sb->s_type->name, "pstore"))
> > > > +           !strcmp(sb->s_type->name, "pstore") ||
> > > > +           !strcmp(sb->s_type->name, "securityfs"))
> > > >                 sbsec->flags |= SE_SBGENFS;
> > > >
> > > >         if (!strcmp(sb->s_type->name, "sysfs") ||
> > > > --
> > > > 2.33.0
> > >
> > > --
> > > paul moore
> > > www.paul-moore.com
> >
> > Something like:
> >
> >     Add support for genfscon per-file labeling of securityfs files. This allows
> >     for separate labels and thereby access control for different files.
> >     For example a genfscon statement
> >         genfscon securityfs /integrity/ima/policy
> > system_u:object_r:ima_policy_t:s0
> >     will set a specific label to the IMA policy file and thus allow to
> > control the ability
> >     to set the IMA policy.
> >     Setting labels directly, e.g. via chcon(1) or setfiles(8), is
> > still not supported.
> >
> > ?
>
> That's a much better description of the functionality, especially for
> those who may not be very familiar with SELinux, thank you.  However I
> was hoping to also hear some confirmation that you have tested this
> and it worked without problem?
>
> --
> paul moore
> www.paul-moore.com

With the following genfscon statements

    genfscon securityfs /  system_u:object_r:securityfs_t:s0
    genfscon securityfs /integrity/ima  system_u:object_r:ima_t:s0
    genfscon securityfs /integrity/ima/policy  system_u:object_r:ima_policy_t:s0
    genfscon securityfs /lockdown  system_u:object_r:lockdown_t:s0

the labels are

    $ ls -laZ /sys/kernel/security/ /sys/kernel/security/integrity/ima/
    /sys/kernel/security/:
    total 0
    drwxr-xr-x.  3 root root system_u:object_r:securityfs_t:s0 0 Sep 17 09:38 .
    drwxr-xr-x. 15 root root system_u:object_r:sysfs_t:s0      0 Sep 17 09:38 ..
    lr--r--r--.  1 root root system_u:object_r:securityfs_t:s0 0 Sep
17 09:38 evm -> integrity/evm/evm
    lr--r--r--.  1 root root system_u:object_r:securityfs_t:s0 0 Sep
17 09:38 ima -> integrity/ima
    drwxr-xr-x.  4 root root system_u:object_r:securityfs_t:s0 0 Sep
17 09:38 integrity
    -rw-r--r--.  1 root root system_u:object_r:lockdown_t:s0   0 Sep
17 09:38 lockdown
    -r--r--r--.  1 root root system_u:object_r:securityfs_t:s0 0 Sep
17 09:38 lsm

    /sys/kernel/security/integrity/ima/:
    total 0
    drwxr-xr-x. 2 root root system_u:object_r:ima_t:s0        0 Sep 17 09:38 .
    rwxr-xr-x. 4 root root system_u:object_r:securityfs_t:s0 0 Sep 17 09:38 ..
    -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
09:38 ascii_runtime_measurements
    -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
09:38 binary_runtime_measurements
    --w-------. 1 root root system_u:object_r:ima_policy_t:s0 0 Sep 17
09:38 policy
    -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
09:38 runtime_measurements_count
    -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
09:38 violations

and audit logs are generated like

    Sep 17 09:38:28 debianBullseye audit[331]: AVC avc:  denied  {
read } for  pid=331 comm="systemd-tmpfile" name="ima" dev="securityfs"
ino=167 scontext=system_u:system_r:systemd_tmpfiles_t:s0
tcontext=system_u:object_r:ima_t:s0 tclass=dir permissive=1
    Sep 17 09:38:28 debianBullseye audit[331]: AVC avc:  denied  {
open } for  pid=331 comm="systemd-tmpfile"
path="/sys/kernel/security/integrity/ima" dev="securityfs" ino=167
scontext=system_u:system_r:systemd_tmpfiles_t:s0
tcontext=system_u:object_r:ima_t:s0 tclass=dir permissive=1
    Sep 17 09:38:28 debianBullseye audit[331]: AVC avc:  denied  {
getattr } for  pid=331 comm="systemd-tmpfile"
path="/sys/kernel/security/integrity/ima/policy" dev="securityfs"
ino=173 scontext=system_u:system_r:systemd_tmpfiles_t:s0
tcontext=system_u:object_r:ima_policy_t:s0 tclass=file permissive=1


chcon(1) fails like

    $ chcon -v -t securityfs_t /sys/kernel/security/integrity/ima/policy
    changing security context of '/sys/kernel/security/integrity/ima/policy'
    chcon: failed to change context of
'/sys/kernel/security/integrity/ima/policy' to
'system_u:object_r:securityfs_t:s0': Operation not supported

A file context definition of

    /sys/kernel/security/integrity/ima/policy --
gen_context(system_u:object_r:securityfs_t,s0)

results in

    $ matchpathcon -V /sys/kernel/security/integrity/ima/policy
    Deprecated, use selabel_lookup
    /sys/kernel/security/integrity/ima/policy has context
system_u:object_r:ima_policy_t:s0, should be
system_u:object_r:securityfs_t:s0
    $ restorecon -vRF /sys/kernel/security/
    # nothing #

Are there any other use cases or edge cases I need to test.

One thing I noticed is that a genfscon statements take affect only
after a reboot, not after remounting the securityfs via

    mount -v -o remount -t securityfs /sys/kernel/security

Is this expected?

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

* Re: [PATCH] selinux: enable genfscon labeling for securityfs
  2021-09-17  8:00       ` Christian Göttsche
@ 2021-09-17 13:27         ` Stephen Smalley
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Smalley @ 2021-09-17 13:27 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: Paul Moore, SElinux list

On Fri, Sep 17, 2021 at 9:07 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On Fri, 17 Sept 2021 at 04:07, Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Thu, Sep 16, 2021 at 1:41 PM Christian Göttsche
> > <cgzones@googlemail.com> wrote:
> > > On Wed, 15 Sept 2021 at 20:28, Paul Moore <paul@paul-moore.com> wrote:
> > > >
> > > > On Wed, Sep 15, 2021 at 12:24 PM Christian Göttsche
> > > > <cgzones@googlemail.com> wrote:
> > > > >
> > > > > Add support for genfscon per-file labeling of securityfs files. This allows
> > > > > for separate labels and therby permissions for different files, e.g.
> > > > > /sys/kernel/security/integrity/ima/policy.
> > > > >
> > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > > ---
> > > > >  security/selinux/hooks.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > Hi Christian,
> > > >
> > > > It would be nice if you could add some additional notes on how this
> > > > was tested to the description above.
> > > >
> > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > > > index 6517f221d52c..a18626424731 100644
> > > > > --- a/security/selinux/hooks.c
> > > > > +++ b/security/selinux/hooks.c
> > > > > @@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> > > > >             !strcmp(sb->s_type->name, "tracefs") ||
> > > > >             !strcmp(sb->s_type->name, "binder") ||
> > > > >             !strcmp(sb->s_type->name, "bpf") ||
> > > > > -           !strcmp(sb->s_type->name, "pstore"))
> > > > > +           !strcmp(sb->s_type->name, "pstore") ||
> > > > > +           !strcmp(sb->s_type->name, "securityfs"))
> > > > >                 sbsec->flags |= SE_SBGENFS;
> > > > >
> > > > >         if (!strcmp(sb->s_type->name, "sysfs") ||
> > > > > --
> > > > > 2.33.0
> > > >
> > > > --
> > > > paul moore
> > > > www.paul-moore.com
> > >
> > > Something like:
> > >
> > >     Add support for genfscon per-file labeling of securityfs files. This allows
> > >     for separate labels and thereby access control for different files.
> > >     For example a genfscon statement
> > >         genfscon securityfs /integrity/ima/policy
> > > system_u:object_r:ima_policy_t:s0
> > >     will set a specific label to the IMA policy file and thus allow to
> > > control the ability
> > >     to set the IMA policy.
> > >     Setting labels directly, e.g. via chcon(1) or setfiles(8), is
> > > still not supported.
> > >
> > > ?
> >
> > That's a much better description of the functionality, especially for
> > those who may not be very familiar with SELinux, thank you.  However I
> > was hoping to also hear some confirmation that you have tested this
> > and it worked without problem?
> >
> > --
> > paul moore
> > www.paul-moore.com
>
> With the following genfscon statements
>
>     genfscon securityfs /  system_u:object_r:securityfs_t:s0
>     genfscon securityfs /integrity/ima  system_u:object_r:ima_t:s0
>     genfscon securityfs /integrity/ima/policy  system_u:object_r:ima_policy_t:s0
>     genfscon securityfs /lockdown  system_u:object_r:lockdown_t:s0
>
> the labels are
>
>     $ ls -laZ /sys/kernel/security/ /sys/kernel/security/integrity/ima/
>     /sys/kernel/security/:
>     total 0
>     drwxr-xr-x.  3 root root system_u:object_r:securityfs_t:s0 0 Sep 17 09:38 .
>     drwxr-xr-x. 15 root root system_u:object_r:sysfs_t:s0      0 Sep 17 09:38 ..
>     lr--r--r--.  1 root root system_u:object_r:securityfs_t:s0 0 Sep
> 17 09:38 evm -> integrity/evm/evm
>     lr--r--r--.  1 root root system_u:object_r:securityfs_t:s0 0 Sep
> 17 09:38 ima -> integrity/ima
>     drwxr-xr-x.  4 root root system_u:object_r:securityfs_t:s0 0 Sep
> 17 09:38 integrity
>     -rw-r--r--.  1 root root system_u:object_r:lockdown_t:s0   0 Sep
> 17 09:38 lockdown
>     -r--r--r--.  1 root root system_u:object_r:securityfs_t:s0 0 Sep
> 17 09:38 lsm
>
>     /sys/kernel/security/integrity/ima/:
>     total 0
>     drwxr-xr-x. 2 root root system_u:object_r:ima_t:s0        0 Sep 17 09:38 .
>     rwxr-xr-x. 4 root root system_u:object_r:securityfs_t:s0 0 Sep 17 09:38 ..
>     -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
> 09:38 ascii_runtime_measurements
>     -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
> 09:38 binary_runtime_measurements
>     --w-------. 1 root root system_u:object_r:ima_policy_t:s0 0 Sep 17
> 09:38 policy
>     -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
> 09:38 runtime_measurements_count
>     -r--r-----. 1 root root system_u:object_r:ima_t:s0        0 Sep 17
> 09:38 violations
>
> and audit logs are generated like
>
>     Sep 17 09:38:28 debianBullseye audit[331]: AVC avc:  denied  {
> read } for  pid=331 comm="systemd-tmpfile" name="ima" dev="securityfs"
> ino=167 scontext=system_u:system_r:systemd_tmpfiles_t:s0
> tcontext=system_u:object_r:ima_t:s0 tclass=dir permissive=1
>     Sep 17 09:38:28 debianBullseye audit[331]: AVC avc:  denied  {
> open } for  pid=331 comm="systemd-tmpfile"
> path="/sys/kernel/security/integrity/ima" dev="securityfs" ino=167
> scontext=system_u:system_r:systemd_tmpfiles_t:s0
> tcontext=system_u:object_r:ima_t:s0 tclass=dir permissive=1
>     Sep 17 09:38:28 debianBullseye audit[331]: AVC avc:  denied  {
> getattr } for  pid=331 comm="systemd-tmpfile"
> path="/sys/kernel/security/integrity/ima/policy" dev="securityfs"
> ino=173 scontext=system_u:system_r:systemd_tmpfiles_t:s0
> tcontext=system_u:object_r:ima_policy_t:s0 tclass=file permissive=1
>
>
> chcon(1) fails like
>
>     $ chcon -v -t securityfs_t /sys/kernel/security/integrity/ima/policy
>     changing security context of '/sys/kernel/security/integrity/ima/policy'
>     chcon: failed to change context of
> '/sys/kernel/security/integrity/ima/policy' to
> 'system_u:object_r:securityfs_t:s0': Operation not supported
>
> A file context definition of
>
>     /sys/kernel/security/integrity/ima/policy --
> gen_context(system_u:object_r:securityfs_t,s0)
>
> results in
>
>     $ matchpathcon -V /sys/kernel/security/integrity/ima/policy
>     Deprecated, use selabel_lookup
>     /sys/kernel/security/integrity/ima/policy has context
> system_u:object_r:ima_policy_t:s0, should be
> system_u:object_r:securityfs_t:s0
>     $ restorecon -vRF /sys/kernel/security/
>     # nothing #
>
> Are there any other use cases or edge cases I need to test.
>
> One thing I noticed is that a genfscon statements take affect only
> after a reboot, not after remounting the securityfs via
>
>     mount -v -o remount -t securityfs /sys/kernel/security
>
> Is this expected?

genfscon is only consulted when a dentry is first instantiated for an
inode, typically on first lookup or creation.
So you'd have to unmount all securityfs mounts to force eviction of
the existing dentries/inodes and get them re-created.
And likely can't be done if the kernel does an internal mount for its
own usage (but doesn't look like that is the case for
securityfs, just selinuxfs).

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

* Re: [PATCH] selinux: enable genfscon labeling for securityfs
  2021-09-17  2:07     ` Paul Moore
  2021-09-17  8:00       ` Christian Göttsche
@ 2021-09-27 21:56       ` Paul Moore
  1 sibling, 0 replies; 9+ messages in thread
From: Paul Moore @ 2021-09-27 21:56 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: SElinux list

On Thu, Sep 16, 2021 at 10:07 PM Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Sep 16, 2021 at 1:41 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> > On Wed, 15 Sept 2021 at 20:28, Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On Wed, Sep 15, 2021 at 12:24 PM Christian Göttsche
> > > <cgzones@googlemail.com> wrote:
> > > >
> > > > Add support for genfscon per-file labeling of securityfs files. This allows
> > > > for separate labels and therby permissions for different files, e.g.
> > > > /sys/kernel/security/integrity/ima/policy.
> > > >
> > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > ---
> > > >  security/selinux/hooks.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > Hi Christian,
> > >
> > > It would be nice if you could add some additional notes on how this
> > > was tested to the description above.
> > >
> > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > > index 6517f221d52c..a18626424731 100644
> > > > --- a/security/selinux/hooks.c
> > > > +++ b/security/selinux/hooks.c
> > > > @@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> > > >             !strcmp(sb->s_type->name, "tracefs") ||
> > > >             !strcmp(sb->s_type->name, "binder") ||
> > > >             !strcmp(sb->s_type->name, "bpf") ||
> > > > -           !strcmp(sb->s_type->name, "pstore"))
> > > > +           !strcmp(sb->s_type->name, "pstore") ||
> > > > +           !strcmp(sb->s_type->name, "securityfs"))
> > > >                 sbsec->flags |= SE_SBGENFS;
> > > >
> > > >         if (!strcmp(sb->s_type->name, "sysfs") ||
> > > > --
> > > > 2.33.0
> > >
> > > --
> > > paul moore
> > > www.paul-moore.com
> >
> > Something like:
> >
> >     Add support for genfscon per-file labeling of securityfs files. This allows
> >     for separate labels and thereby access control for different files.
> >     For example a genfscon statement
> >         genfscon securityfs /integrity/ima/policy
> > system_u:object_r:ima_policy_t:s0
> >     will set a specific label to the IMA policy file and thus allow to
> > control the ability
> >     to set the IMA policy.
> >     Setting labels directly, e.g. via chcon(1) or setfiles(8), is
> > still not supported.
> >
> > ?
>
> That's a much better description of the functionality, especially for
> those who may not be very familiar with SELinux, thank you.  However I
> was hoping to also hear some confirmation that you have tested this
> and it worked without problem?

Hi Christian, my apologies on the delay, I was distracted by a few
other SELinux issues.  Thank you for sending out your testing notes in
the meantime.

Are you okay if I replace the original commit description with your
more verbose version?  If not, could you resend the patch with the
update commit description?

Thanks.

-- 
paul moore
www.paul-moore.com

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

* [PATCH v2] selinux: enable genfscon labeling for securityfs
  2021-09-15 16:23 [PATCH] selinux: enable genfscon labeling for securityfs Christian Göttsche
  2021-09-15 18:27 ` Paul Moore
@ 2021-09-28 15:39 ` Christian Göttsche
  2021-09-28 22:50   ` Paul Moore
  1 sibling, 1 reply; 9+ messages in thread
From: Christian Göttsche @ 2021-09-28 15:39 UTC (permalink / raw)
  To: selinux

Add support for genfscon per-file labeling of securityfs files.  This allows
for separate labels and thereby access control for different files.
For example a genfscon statement

    genfscon securityfs /integrity/ima/policy system_u:object_r:ima_policy_t:s0

will set a private label to the IMA policy file and thus allow to
control the ability to set the IMA policy.
Setting labels directly with setxattr(2), e.g. by chcon(1) or
setfiles(8), is still not supported.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
  - improve commit description


 security/selinux/hooks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 012e8504ed9e..549f631e9832 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	    !strcmp(sb->s_type->name, "tracefs") ||
 	    !strcmp(sb->s_type->name, "binder") ||
 	    !strcmp(sb->s_type->name, "bpf") ||
-	    !strcmp(sb->s_type->name, "pstore"))
+	    !strcmp(sb->s_type->name, "pstore") ||
+	    !strcmp(sb->s_type->name, "securityfs"))
 		sbsec->flags |= SE_SBGENFS;
 
 	if (!strcmp(sb->s_type->name, "sysfs") ||
-- 
2.33.0


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

* Re: [PATCH v2] selinux: enable genfscon labeling for securityfs
  2021-09-28 15:39 ` [PATCH v2] " Christian Göttsche
@ 2021-09-28 22:50   ` Paul Moore
  0 siblings, 0 replies; 9+ messages in thread
From: Paul Moore @ 2021-09-28 22:50 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

On Tue, Sep 28, 2021 at 11:40 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Add support for genfscon per-file labeling of securityfs files.  This allows
> for separate labels and thereby access control for different files.
> For example a genfscon statement
>
>     genfscon securityfs /integrity/ima/policy system_u:object_r:ima_policy_t:s0
>
> will set a private label to the IMA policy file and thus allow to
> control the ability to set the IMA policy.
> Setting labels directly with setxattr(2), e.g. by chcon(1) or
> setfiles(8), is still not supported.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v2:
>   - improve commit description
>
>
>  security/selinux/hooks.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Merged into selinux/next, thanks Christian!

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 012e8504ed9e..549f631e9832 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -760,7 +760,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
>             !strcmp(sb->s_type->name, "tracefs") ||
>             !strcmp(sb->s_type->name, "binder") ||
>             !strcmp(sb->s_type->name, "bpf") ||
> -           !strcmp(sb->s_type->name, "pstore"))
> +           !strcmp(sb->s_type->name, "pstore") ||
> +           !strcmp(sb->s_type->name, "securityfs"))
>                 sbsec->flags |= SE_SBGENFS;
>
>         if (!strcmp(sb->s_type->name, "sysfs") ||
> --
> 2.33.0

-- 
paul moore
www.paul-moore.com

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

end of thread, other threads:[~2021-09-28 22:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 16:23 [PATCH] selinux: enable genfscon labeling for securityfs Christian Göttsche
2021-09-15 18:27 ` Paul Moore
2021-09-16 17:41   ` Christian Göttsche
2021-09-17  2:07     ` Paul Moore
2021-09-17  8:00       ` Christian Göttsche
2021-09-17 13:27         ` Stephen Smalley
2021-09-27 21:56       ` Paul Moore
2021-09-28 15:39 ` [PATCH v2] " Christian Göttsche
2021-09-28 22:50   ` Paul Moore

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.