diff mbox series

selinux: deprecated fs ocon

Message ID 20230511123213.722912-1-cgzones@googlemail.com (mailing list archive)
State Accepted
Delegated to: Paul Moore
Headers show
Series selinux: deprecated fs ocon | expand

Commit Message

Christian Göttsche May 11, 2023, 12:32 p.m. UTC
The object context type `fs`, not to be confused with the well used
object context type `fscon`, was introduced in the initial git commit
1da177e4c3f4 ("Linux-2.6.12-rc2") but never actually used since.

The paper "A Security Policy Configuration for the Security-Enhanced
Linux" [1] mentions it under `7.2 File System Contexts` but also states:

    Currently, this configuration is unused.

The policy statement defining such object contexts is `fscon`, e.g.:

    fscon 2 3 gen_context(system_u:object_r:conA_t,s0) gen_context(system_u:object_r:conB_t,s0)

It is not documented at selinuxproject.org or in the SELinux notebook
and not supported by the Reference Policy buildsystem - the statement is
not properly sorted - and thus not used in the Reference or Fedora
Policy.

Print a warning message at policy load for each such object context:

    SELinux:  void and deprecated fs ocon 02:03

This topic was initially highlighted by Nicolas Iooss [2].

[1]: https://media.defense.gov/2021/Jul/29/2002815735/-1/-1/0/SELINUX-SECURITY-POLICY-CONFIGURATION-REPORT.PDF
[2]: https://lore.kernel.org/selinux/CAJfZ7=mP2eJaq2BfO3y0VnwUJaY2cS2p=HZMN71z1pKjzaT0Eg@mail.gmail.com/

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 4 ++++
 security/selinux/ss/policydb.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Paul Moore May 18, 2023, 5:56 p.m. UTC | #1
On May 11, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> The object context type `fs`, not to be confused with the well used
> object context type `fscon`, was introduced in the initial git commit
> 1da177e4c3f4 ("Linux-2.6.12-rc2") but never actually used since.
> 
> The paper "A Security Policy Configuration for the Security-Enhanced
> Linux" [1] mentions it under `7.2 File System Contexts` but also states:
> 
>     Currently, this configuration is unused.
> 
> The policy statement defining such object contexts is `fscon`, e.g.:
> 
>     fscon 2 3 gen_context(system_u:object_r:conA_t,s0) gen_context(system_u:object_r:conB_t,s0)
> 
> It is not documented at selinuxproject.org or in the SELinux notebook
> and not supported by the Reference Policy buildsystem - the statement is
> not properly sorted - and thus not used in the Reference or Fedora
> Policy.
> 
> Print a warning message at policy load for each such object context:
> 
>     SELinux:  void and deprecated fs ocon 02:03
> 
> This topic was initially highlighted by Nicolas Iooss [2].
> 
> [1]: https://media.defense.gov/2021/Jul/29/2002815735/-1/-1/0/SELINUX-SECURITY-POLICY-CONFIGURATION-REPORT.PDF
> [2]: https://lore.kernel.org/selinux/CAJfZ7=mP2eJaq2BfO3y0VnwUJaY2cS2p=HZMN71z1pKjzaT0Eg@mail.gmail.com/
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/policydb.c | 4 ++++
>  security/selinux/ss/policydb.h | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)

Thanks, this is a nice catch, although some minor suggestions below ...

> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 97c0074f9312..31b08b34c722 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -2257,6 +2257,10 @@ static int ocontext_read(struct policydb *p, const struct policydb_compat_info *
>  				if (rc)
>  					goto out;
>  
> +				if (i == OCON_FS)
> +					pr_warn("SELinux:  void and deprecated fs ocon %s\n",
> +						c->u.name);

Instead of having to check if 'i == OCON_FS', why not simply put the
pr_warn() call up in the OCON_FS case block on line ~2249 and let it
continue to fallthrough to the OCON_NETIF block?

>  				rc = context_read_and_validate(&c->context[0], p, fp);
>  				if (rc)
>  					goto out;
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index ffc4e7bad205..39cd6222e1a8 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -225,7 +225,7 @@ struct genfs {
>  
>  /* object context array indices */
>  #define OCON_ISID	0 /* initial SIDs */
> -#define OCON_FS		1 /* unlabeled file systems */
> +#define OCON_FS		1 /* unlabeled file systems (deprecated in 6.5) */

Since you are likely re-spinning this (see above), I would just leave
it as "(deprecated)"; those that want to know where it was deprecated
can always check the git log/tags.

>  #define OCON_PORT	2 /* TCP and UDP port numbers */
>  #define OCON_NETIF	3 /* network interfaces */
>  #define OCON_NODE	4 /* nodes */
> -- 
> 2.40.1

--
paul-moore.com
Paul Moore May 18, 2023, 8:18 p.m. UTC | #2
On Thu, May 18, 2023 at 1:56 PM Paul Moore <paul@paul-moore.com> wrote:
> On May 11, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> >
> > The object context type `fs`, not to be confused with the well used
> > object context type `fscon`, was introduced in the initial git commit
> > 1da177e4c3f4 ("Linux-2.6.12-rc2") but never actually used since.
> >
> > The paper "A Security Policy Configuration for the Security-Enhanced
> > Linux" [1] mentions it under `7.2 File System Contexts` but also states:
> >
> >     Currently, this configuration is unused.
> >
> > The policy statement defining such object contexts is `fscon`, e.g.:
> >
> >     fscon 2 3 gen_context(system_u:object_r:conA_t,s0) gen_context(system_u:object_r:conB_t,s0)
> >
> > It is not documented at selinuxproject.org or in the SELinux notebook
> > and not supported by the Reference Policy buildsystem - the statement is
> > not properly sorted - and thus not used in the Reference or Fedora
> > Policy.
> >
> > Print a warning message at policy load for each such object context:
> >
> >     SELinux:  void and deprecated fs ocon 02:03
> >
> > This topic was initially highlighted by Nicolas Iooss [2].
> >
> > [1]: https://media.defense.gov/2021/Jul/29/2002815735/-1/-1/0/SELINUX-SECURITY-POLICY-CONFIGURATION-REPORT.PDF
> > [2]: https://lore.kernel.org/selinux/CAJfZ7=mP2eJaq2BfO3y0VnwUJaY2cS2p=HZMN71z1pKjzaT0Eg@mail.gmail.com/
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  security/selinux/ss/policydb.c | 4 ++++
> >  security/selinux/ss/policydb.h | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
>
> Thanks, this is a nice catch, although some minor suggestions below ...
>
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index 97c0074f9312..31b08b34c722 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -2257,6 +2257,10 @@ static int ocontext_read(struct policydb *p, const struct policydb_compat_info *
> >                               if (rc)
> >                                       goto out;
> >
> > +                             if (i == OCON_FS)
> > +                                     pr_warn("SELinux:  void and deprecated fs ocon %s\n",
> > +                                             c->u.name);
>
> Instead of having to check if 'i == OCON_FS', why not simply put the
> pr_warn() call up in the OCON_FS case block on line ~2249 and let it
> continue to fallthrough to the OCON_NETIF block?

Bah, nevermind, you need to leave it here because of the 'c->u.name'
in the pr_warn().  If you're okay with me adjusting the deprecation
comment (below) during the merge I'll can merge this now ... ?

> >                               rc = context_read_and_validate(&c->context[0], p, fp);
> >                               if (rc)
> >                                       goto out;
> > diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> > index ffc4e7bad205..39cd6222e1a8 100644
> > --- a/security/selinux/ss/policydb.h
> > +++ b/security/selinux/ss/policydb.h
> > @@ -225,7 +225,7 @@ struct genfs {
> >
> >  /* object context array indices */
> >  #define OCON_ISID    0 /* initial SIDs */
> > -#define OCON_FS              1 /* unlabeled file systems */
> > +#define OCON_FS              1 /* unlabeled file systems (deprecated in 6.5) */
>
> Since you are likely re-spinning this (see above), I would just leave
> it as "(deprecated)"; those that want to know where it was deprecated
> can always check the git log/tags.
>
> >  #define OCON_PORT    2 /* TCP and UDP port numbers */
> >  #define OCON_NETIF   3 /* network interfaces */
> >  #define OCON_NODE    4 /* nodes */
> > --
> > 2.40.1
Christian Göttsche May 23, 2023, 6:25 p.m. UTC | #3
On Thu, 18 May 2023 at 22:18, Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, May 18, 2023 at 1:56 PM Paul Moore <paul@paul-moore.com> wrote:
> > On May 11, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> > >
> > > The object context type `fs`, not to be confused with the well used
> > > object context type `fscon`, was introduced in the initial git commit
> > > 1da177e4c3f4 ("Linux-2.6.12-rc2") but never actually used since.
> > >
> > > The paper "A Security Policy Configuration for the Security-Enhanced
> > > Linux" [1] mentions it under `7.2 File System Contexts` but also states:
> > >
> > >     Currently, this configuration is unused.
> > >
> > > The policy statement defining such object contexts is `fscon`, e.g.:
> > >
> > >     fscon 2 3 gen_context(system_u:object_r:conA_t,s0) gen_context(system_u:object_r:conB_t,s0)
> > >
> > > It is not documented at selinuxproject.org or in the SELinux notebook
> > > and not supported by the Reference Policy buildsystem - the statement is
> > > not properly sorted - and thus not used in the Reference or Fedora
> > > Policy.
> > >
> > > Print a warning message at policy load for each such object context:
> > >
> > >     SELinux:  void and deprecated fs ocon 02:03
> > >
> > > This topic was initially highlighted by Nicolas Iooss [2].
> > >
> > > [1]: https://media.defense.gov/2021/Jul/29/2002815735/-1/-1/0/SELINUX-SECURITY-POLICY-CONFIGURATION-REPORT.PDF
> > > [2]: https://lore.kernel.org/selinux/CAJfZ7=mP2eJaq2BfO3y0VnwUJaY2cS2p=HZMN71z1pKjzaT0Eg@mail.gmail.com/
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > ---
> > >  security/selinux/ss/policydb.c | 4 ++++
> > >  security/selinux/ss/policydb.h | 2 +-
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > Thanks, this is a nice catch, although some minor suggestions below ...
> >
> > > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > > index 97c0074f9312..31b08b34c722 100644
> > > --- a/security/selinux/ss/policydb.c
> > > +++ b/security/selinux/ss/policydb.c
> > > @@ -2257,6 +2257,10 @@ static int ocontext_read(struct policydb *p, const struct policydb_compat_info *
> > >                               if (rc)
> > >                                       goto out;
> > >
> > > +                             if (i == OCON_FS)
> > > +                                     pr_warn("SELinux:  void and deprecated fs ocon %s\n",
> > > +                                             c->u.name);
> >
> > Instead of having to check if 'i == OCON_FS', why not simply put the
> > pr_warn() call up in the OCON_FS case block on line ~2249 and let it
> > continue to fallthrough to the OCON_NETIF block?
>
> Bah, nevermind, you need to leave it here because of the 'c->u.name'
> in the pr_warn().  If you're okay with me adjusting the deprecation
> comment (below) during the merge I'll can merge this now ... ?

Yes, please feel free to adjust the inline comment.

>
> > >                               rc = context_read_and_validate(&c->context[0], p, fp);
> > >                               if (rc)
> > >                                       goto out;
> > > diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> > > index ffc4e7bad205..39cd6222e1a8 100644
> > > --- a/security/selinux/ss/policydb.h
> > > +++ b/security/selinux/ss/policydb.h
> > > @@ -225,7 +225,7 @@ struct genfs {
> > >
> > >  /* object context array indices */
> > >  #define OCON_ISID    0 /* initial SIDs */
> > > -#define OCON_FS              1 /* unlabeled file systems */
> > > +#define OCON_FS              1 /* unlabeled file systems (deprecated in 6.5) */
> >
> > Since you are likely re-spinning this (see above), I would just leave
> > it as "(deprecated)"; those that want to know where it was deprecated
> > can always check the git log/tags.
> >
> > >  #define OCON_PORT    2 /* TCP and UDP port numbers */
> > >  #define OCON_NETIF   3 /* network interfaces */
> > >  #define OCON_NODE    4 /* nodes */
> > > --
> > > 2.40.1
>
> --
> paul-moore.com
Paul Moore May 23, 2023, 7:38 p.m. UTC | #4
On Tue, May 23, 2023 at 2:25 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
> On Thu, 18 May 2023 at 22:18, Paul Moore <paul@paul-moore.com> wrote:
> > On Thu, May 18, 2023 at 1:56 PM Paul Moore <paul@paul-moore.com> wrote:
> > > On May 11, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> > > >
> > > > The object context type `fs`, not to be confused with the well used
> > > > object context type `fscon`, was introduced in the initial git commit
> > > > 1da177e4c3f4 ("Linux-2.6.12-rc2") but never actually used since.
> > > >
> > > > The paper "A Security Policy Configuration for the Security-Enhanced
> > > > Linux" [1] mentions it under `7.2 File System Contexts` but also states:
> > > >
> > > >     Currently, this configuration is unused.
> > > >
> > > > The policy statement defining such object contexts is `fscon`, e.g.:
> > > >
> > > >     fscon 2 3 gen_context(system_u:object_r:conA_t,s0) gen_context(system_u:object_r:conB_t,s0)
> > > >
> > > > It is not documented at selinuxproject.org or in the SELinux notebook
> > > > and not supported by the Reference Policy buildsystem - the statement is
> > > > not properly sorted - and thus not used in the Reference or Fedora
> > > > Policy.
> > > >
> > > > Print a warning message at policy load for each such object context:
> > > >
> > > >     SELinux:  void and deprecated fs ocon 02:03
> > > >
> > > > This topic was initially highlighted by Nicolas Iooss [2].
> > > >
> > > > [1]: https://media.defense.gov/2021/Jul/29/2002815735/-1/-1/0/SELINUX-SECURITY-POLICY-CONFIGURATION-REPORT.PDF
> > > > [2]: https://lore.kernel.org/selinux/CAJfZ7=mP2eJaq2BfO3y0VnwUJaY2cS2p=HZMN71z1pKjzaT0Eg@mail.gmail.com/
> > > >
> > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > ---
> > > >  security/selinux/ss/policydb.c | 4 ++++
> > > >  security/selinux/ss/policydb.h | 2 +-
> > > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > Thanks, this is a nice catch, although some minor suggestions below ...
> > >
> > > > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > > > index 97c0074f9312..31b08b34c722 100644
> > > > --- a/security/selinux/ss/policydb.c
> > > > +++ b/security/selinux/ss/policydb.c
> > > > @@ -2257,6 +2257,10 @@ static int ocontext_read(struct policydb *p, const struct policydb_compat_info *
> > > >                               if (rc)
> > > >                                       goto out;
> > > >
> > > > +                             if (i == OCON_FS)
> > > > +                                     pr_warn("SELinux:  void and deprecated fs ocon %s\n",
> > > > +                                             c->u.name);
> > >
> > > Instead of having to check if 'i == OCON_FS', why not simply put the
> > > pr_warn() call up in the OCON_FS case block on line ~2249 and let it
> > > continue to fallthrough to the OCON_NETIF block?
> >
> > Bah, nevermind, you need to leave it here because of the 'c->u.name'
> > in the pr_warn().  If you're okay with me adjusting the deprecation
> > comment (below) during the merge I'll can merge this now ... ?
>
> Yes, please feel free to adjust the inline comment.

Okay, done and merged into selinux/next, thanks.
diff mbox series

Patch

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 97c0074f9312..31b08b34c722 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -2257,6 +2257,10 @@  static int ocontext_read(struct policydb *p, const struct policydb_compat_info *
 				if (rc)
 					goto out;
 
+				if (i == OCON_FS)
+					pr_warn("SELinux:  void and deprecated fs ocon %s\n",
+						c->u.name);
+
 				rc = context_read_and_validate(&c->context[0], p, fp);
 				if (rc)
 					goto out;
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index ffc4e7bad205..39cd6222e1a8 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -225,7 +225,7 @@  struct genfs {
 
 /* object context array indices */
 #define OCON_ISID	0 /* initial SIDs */
-#define OCON_FS		1 /* unlabeled file systems */
+#define OCON_FS		1 /* unlabeled file systems (deprecated in 6.5) */
 #define OCON_PORT	2 /* TCP and UDP port numbers */
 #define OCON_NETIF	3 /* network interfaces */
 #define OCON_NODE	4 /* nodes */