All of lore.kernel.org
 help / color / mirror / Atom feed
* [TESTSUITE PATCH] policy: drop usage of files_list_pids()
@ 2023-01-16 21:46 Christian Göttsche
  2023-01-17 10:00 ` Ondrej Mosnacek
  2023-01-17 17:09 ` [TESTSUITE PATCH v2] policy: handle files_list_pids() renaming in Refpolicy Christian Göttsche
  0 siblings, 2 replies; 6+ messages in thread
From: Christian Göttsche @ 2023-01-16 21:46 UTC (permalink / raw)
  To: selinux

files_list_pids() has been superseded and marked deprecated in the
Reference Policy since Jun 2020[1].  In the latest release it has been
completely removed[2].

Grant the necessary permissions via raw rules to support recent
Refpolicy versions as well as old ones without the replacement
interface files_list_runtime().

[1]: https://github.com/SELinuxProject/refpolicy/commit/be04bb3e7e63671ed8a3c501a2ee76e11c3b92bb
[2]: https://github.com/SELinuxProject/refpolicy/commit/3ca0cd59d7a9b531dd3620a02940396343fe2ed5

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 policy/test_global.te | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/policy/test_global.te b/policy/test_global.te
index e95102a..4bf30f8 100644
--- a/policy/test_global.te
+++ b/policy/test_global.te
@@ -121,7 +121,6 @@ allow testsuite_domain proc_t:file { getattr read open };
 files_list_var(testsuite_domain)
 files_list_home(testsuite_domain)
 dev_read_rand(testsuite_domain)
-files_list_pids(testsuite_domain)
 require {
 	type root_t;
 	type etc_t;
@@ -136,8 +135,12 @@ require {
 	type init_t;
 	type initrc_t;
 	type console_device_t;
+	type var_t;
+	type var_run_t;
 }
-allow testsuite_domain { root_t etc_t bin_t sbin_t lib_t usr_t devpts_t }:dir list_dir_perms;
+allow testsuite_domain { root_t etc_t bin_t sbin_t lib_t usr_t devpts_t var_run_t }:dir list_dir_perms;
+allow testsuite_domain var_t:dir search_dir_perms;
+allow testsuite_domain { var_t var_run_t }:lnk_file read_lnk_file_perms;
 allow testsuite_domain lib_t:file read_file_perms;
 allow testsuite_domain lib_t:lnk_file read;
 allow testsuite_domain etc_t:file read_file_perms;
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [TESTSUITE PATCH] policy: drop usage of files_list_pids()
  2023-01-16 21:46 [TESTSUITE PATCH] policy: drop usage of files_list_pids() Christian Göttsche
@ 2023-01-17 10:00 ` Ondrej Mosnacek
  2023-01-17 16:10   ` Christian Göttsche
  2023-01-17 17:09 ` [TESTSUITE PATCH v2] policy: handle files_list_pids() renaming in Refpolicy Christian Göttsche
  1 sibling, 1 reply; 6+ messages in thread
From: Ondrej Mosnacek @ 2023-01-17 10:00 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

On Mon, Jan 16, 2023 at 10:48 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
> files_list_pids() has been superseded and marked deprecated in the
> Reference Policy since Jun 2020[1].  In the latest release it has been
> completely removed[2].
>
> Grant the necessary permissions via raw rules to support recent
> Refpolicy versions as well as old ones without the replacement
> interface files_list_runtime().

It seems the permissions aren't actually needed, at least on current
Fedoras. Simply removing the call passes the CI:
https://github.com/WOnder93/selinux-testsuite/commit/d0883a56d2583800a1fa79490097e73b842cec17

Do you have an environment with refpolicy where you can test it? It
would be better to just remove the interface call if it's not needed.

>
> [1]: https://github.com/SELinuxProject/refpolicy/commit/be04bb3e7e63671ed8a3c501a2ee76e11c3b92bb
> [2]: https://github.com/SELinuxProject/refpolicy/commit/3ca0cd59d7a9b531dd3620a02940396343fe2ed5
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  policy/test_global.te | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/policy/test_global.te b/policy/test_global.te
> index e95102a..4bf30f8 100644
> --- a/policy/test_global.te
> +++ b/policy/test_global.te
> @@ -121,7 +121,6 @@ allow testsuite_domain proc_t:file { getattr read open };
>  files_list_var(testsuite_domain)
>  files_list_home(testsuite_domain)
>  dev_read_rand(testsuite_domain)
> -files_list_pids(testsuite_domain)
>  require {
>         type root_t;
>         type etc_t;
> @@ -136,8 +135,12 @@ require {
>         type init_t;
>         type initrc_t;
>         type console_device_t;
> +       type var_t;
> +       type var_run_t;
>  }
> -allow testsuite_domain { root_t etc_t bin_t sbin_t lib_t usr_t devpts_t }:dir list_dir_perms;
> +allow testsuite_domain { root_t etc_t bin_t sbin_t lib_t usr_t devpts_t var_run_t }:dir list_dir_perms;
> +allow testsuite_domain var_t:dir search_dir_perms;
> +allow testsuite_domain { var_t var_run_t }:lnk_file read_lnk_file_perms;
>  allow testsuite_domain lib_t:file read_file_perms;
>  allow testsuite_domain lib_t:lnk_file read;
>  allow testsuite_domain etc_t:file read_file_perms;
> --
> 2.39.0
>

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [TESTSUITE PATCH] policy: drop usage of files_list_pids()
  2023-01-17 10:00 ` Ondrej Mosnacek
@ 2023-01-17 16:10   ` Christian Göttsche
  2023-01-17 16:43     ` Ondrej Mosnacek
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Göttsche @ 2023-01-17 16:10 UTC (permalink / raw)
  To: Ondrej Mosnacek; +Cc: selinux

On Tue, 17 Jan 2023 at 11:00, Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Mon, Jan 16, 2023 at 10:48 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> > files_list_pids() has been superseded and marked deprecated in the
> > Reference Policy since Jun 2020[1].  In the latest release it has been
> > completely removed[2].
> >
> > Grant the necessary permissions via raw rules to support recent
> > Refpolicy versions as well as old ones without the replacement
> > interface files_list_runtime().
>
> It seems the permissions aren't actually needed, at least on current
> Fedoras. Simply removing the call passes the CI:
> https://github.com/WOnder93/selinux-testsuite/commit/d0883a56d2583800a1fa79490097e73b842cec17

On Fedora the call of `auth_read_passwd(testsuite_domain)`[1] leads to
a call of `sssd_stream_connect()`[2], which includes
`files_search_pids()`[3].

There is no indirect call in the Debian version of Refpolicy though:

    type=PROCTITLE msg=audit(17/01/23 16:41:13.404:577) :
proctitle=keys/keyctl_relabel system_u:object_r:test_newcon_key_t:s0
    type=PATH msg=audit(17/01/23 16:41:13.404:577) : item=0
name=/var/run/setrans/.setrans-unix nametype=UNKNOWN cap_fp=none
cap_fi=none cap_fe=0 cap_fver=0 cap_frootid=0
    type=CWD msg=audit(17/01/23 16:41:13.404:577) :
cwd=/root/workspace/selinux/selinux-testsuite/tests
    type=SYSCALL msg=audit(17/01/23 16:41:13.404:577) : arch=x86_64
syscall=access success=no exit=EACCES(Permission denied)
a0=0x7ea1b2255068 a1=F_OK a2=0x7ffd39131fb0 a3=0xa9ab59f33f82d0d9
items=1 ppid=4569 pid=4593 auid=root uid=root gid=root euid=root
suid=root fsuid=ro
ot egid=root sgid=root fsgid=root tty=pts1 ses=1 comm=keyctl_relabel
exe=/root/workspace/selinux/selinux-testsuite/tests/keys/keyctl_relabel
subj=unconfined_u:unconfined_r:test_key_t:s0-s0:c0.c1023 key=(null)
    type=AVC msg=audit(17/01/23 16:41:13.404:577) : avc:  denied  {
read } for  pid=4593 comm=keyctl_relabel name=run dev="vda1"
ino=390346 scontext=unconfined_u:unconfined_r:test_key_t:s0-s0:c0.c1023
tcontext=system_u:object_r:var_run_t:s0 tclass=lnk_file permissive=0

The tessuite passes nevertheless, so one could ignore or explicitly
dontaudit them.

An alternative would be to call the interfaces conditionally:

    ifdef(`files_list_pids', `
        files_list_pids(testsuite_domain)
    ')
    ifdef(`files_list_runtime', `
        files_list_runtime(testsuite_domain)
    ')

[1]: https://github.com/SELinuxProject/selinux-testsuite/blob/bbab270f66c3fc33b4fdc7cec8beb0003afbb4ee/policy/test_global.te#L159
[2]: https://github.com/fedora-selinux/selinux-policy/blob/33883ed0612f156e12dc9a0268848908052e3085/policy/modules/system/authlogin.if#L2271
[3]: https://github.com/fedora-selinux/selinux-policy/blob/25bdcfdf5821ddba2c47fc4306bc43debc4c0f75/policy/modules/contrib/sssd.if#L407

>
> Do you have an environment with refpolicy where you can test it? It
> would be better to just remove the interface call if it's not needed.
>
> >
> > [1]: https://github.com/SELinuxProject/refpolicy/commit/be04bb3e7e63671ed8a3c501a2ee76e11c3b92bb
> > [2]: https://github.com/SELinuxProject/refpolicy/commit/3ca0cd59d7a9b531dd3620a02940396343fe2ed5
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  policy/test_global.te | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/policy/test_global.te b/policy/test_global.te
> > index e95102a..4bf30f8 100644
> > --- a/policy/test_global.te
> > +++ b/policy/test_global.te
> > @@ -121,7 +121,6 @@ allow testsuite_domain proc_t:file { getattr read open };
> >  files_list_var(testsuite_domain)

The explicit rules targeting var_t can be dropped, cause I overlooked
the `files_list_var(testsuite_domain)` call.

> >  files_list_home(testsuite_domain)
> >  dev_read_rand(testsuite_domain)
> > -files_list_pids(testsuite_domain)
> >  require {
> >         type root_t;
> >         type etc_t;
> > @@ -136,8 +135,12 @@ require {
> >         type init_t;
> >         type initrc_t;
> >         type console_device_t;
> > +       type var_t;
> > +       type var_run_t;
> >  }
> > -allow testsuite_domain { root_t etc_t bin_t sbin_t lib_t usr_t devpts_t }:dir list_dir_perms;
> > +allow testsuite_domain { root_t etc_t bin_t sbin_t lib_t usr_t devpts_t var_run_t }:dir list_dir_perms;
> > +allow testsuite_domain var_t:dir search_dir_perms;
> > +allow testsuite_domain { var_t var_run_t }:lnk_file read_lnk_file_perms;
> >  allow testsuite_domain lib_t:file read_file_perms;
> >  allow testsuite_domain lib_t:lnk_file read;
> >  allow testsuite_domain etc_t:file read_file_perms;
> > --
> > 2.39.0
> >
>
> --
> Ondrej Mosnacek
> Senior Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [TESTSUITE PATCH] policy: drop usage of files_list_pids()
  2023-01-17 16:10   ` Christian Göttsche
@ 2023-01-17 16:43     ` Ondrej Mosnacek
  0 siblings, 0 replies; 6+ messages in thread
From: Ondrej Mosnacek @ 2023-01-17 16:43 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

On Tue, Jan 17, 2023 at 5:13 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
> On Tue, 17 Jan 2023 at 11:00, Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > On Mon, Jan 16, 2023 at 10:48 PM Christian Göttsche
> > <cgzones@googlemail.com> wrote:
> > > files_list_pids() has been superseded and marked deprecated in the
> > > Reference Policy since Jun 2020[1].  In the latest release it has been
> > > completely removed[2].
> > >
> > > Grant the necessary permissions via raw rules to support recent
> > > Refpolicy versions as well as old ones without the replacement
> > > interface files_list_runtime().
> >
> > It seems the permissions aren't actually needed, at least on current
> > Fedoras. Simply removing the call passes the CI:
> > https://github.com/WOnder93/selinux-testsuite/commit/d0883a56d2583800a1fa79490097e73b842cec17
>
> On Fedora the call of `auth_read_passwd(testsuite_domain)`[1] leads to
> a call of `sssd_stream_connect()`[2], which includes
> `files_search_pids()`[3].
>
> There is no indirect call in the Debian version of Refpolicy though:

Ok, so let's keep the rules then.

>
>     type=PROCTITLE msg=audit(17/01/23 16:41:13.404:577) :
> proctitle=keys/keyctl_relabel system_u:object_r:test_newcon_key_t:s0
>     type=PATH msg=audit(17/01/23 16:41:13.404:577) : item=0
> name=/var/run/setrans/.setrans-unix nametype=UNKNOWN cap_fp=none
> cap_fi=none cap_fe=0 cap_fver=0 cap_frootid=0
>     type=CWD msg=audit(17/01/23 16:41:13.404:577) :
> cwd=/root/workspace/selinux/selinux-testsuite/tests
>     type=SYSCALL msg=audit(17/01/23 16:41:13.404:577) : arch=x86_64
> syscall=access success=no exit=EACCES(Permission denied)
> a0=0x7ea1b2255068 a1=F_OK a2=0x7ffd39131fb0 a3=0xa9ab59f33f82d0d9
> items=1 ppid=4569 pid=4593 auid=root uid=root gid=root euid=root
> suid=root fsuid=ro
> ot egid=root sgid=root fsgid=root tty=pts1 ses=1 comm=keyctl_relabel
> exe=/root/workspace/selinux/selinux-testsuite/tests/keys/keyctl_relabel
> subj=unconfined_u:unconfined_r:test_key_t:s0-s0:c0.c1023 key=(null)
>     type=AVC msg=audit(17/01/23 16:41:13.404:577) : avc:  denied  {
> read } for  pid=4593 comm=keyctl_relabel name=run dev="vda1"
> ino=390346 scontext=unconfined_u:unconfined_r:test_key_t:s0-s0:c0.c1023
> tcontext=system_u:object_r:var_run_t:s0 tclass=lnk_file permissive=0
>
> The tessuite passes nevertheless, so one could ignore or explicitly
> dontaudit them.
>
> An alternative would be to call the interfaces conditionally:
>
>     ifdef(`files_list_pids', `
>         files_list_pids(testsuite_domain)
>     ')
>     ifdef(`files_list_runtime', `
>         files_list_runtime(testsuite_domain)
>     ')

Yeah, I'd prefer the conditional calls, with a comment explaining that
Refpolicy has renamed the interface.

Thanks,

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [TESTSUITE PATCH v2] policy: handle files_list_pids() renaming in Refpolicy
  2023-01-16 21:46 [TESTSUITE PATCH] policy: drop usage of files_list_pids() Christian Göttsche
  2023-01-17 10:00 ` Ondrej Mosnacek
@ 2023-01-17 17:09 ` Christian Göttsche
  2023-01-20  9:38   ` Ondrej Mosnacek
  1 sibling, 1 reply; 6+ messages in thread
From: Christian Göttsche @ 2023-01-17 17:09 UTC (permalink / raw)
  To: selinux

files_list_pids() has been superseded and marked deprecated in the
Reference Policy since Jun 2020[1].  In the latest release it has been
completely removed[2].

Call both the old and replacement interface conditionally to support
recent Refpolicy versions as well as old ones.

[1]: https://github.com/SELinuxProject/refpolicy/commit/be04bb3e7e63671ed8a3c501a2ee76e11c3b92bb
[2]: https://github.com/SELinuxProject/refpolicy/commit/3ca0cd59d7a9b531dd3620a02940396343fe2ed5

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
   call both interfaces conditionally
---
 policy/test_global.te | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/policy/test_global.te b/policy/test_global.te
index e95102a..052c7dd 100644
--- a/policy/test_global.te
+++ b/policy/test_global.te
@@ -121,7 +121,6 @@ allow testsuite_domain proc_t:file { getattr read open };
 files_list_var(testsuite_domain)
 files_list_home(testsuite_domain)
 dev_read_rand(testsuite_domain)
-files_list_pids(testsuite_domain)
 require {
 	type root_t;
 	type etc_t;
@@ -154,6 +153,14 @@ selinux_compute_create_context(testsuite_domain)
 selinux_compute_relabel_context(testsuite_domain)
 selinux_compute_user_contexts(testsuite_domain)
 
+# Reference policy renamed files_list_pids() to files_list_runtime()
+ifdef(`files_list_pids', `
+    files_list_pids(testsuite_domain)
+')
+ifdef(`files_list_runtime', `
+    files_list_runtime(testsuite_domain)
+')
+
 ifdef(`distro_redhat', `
     ifdef(`auth_read_passwd', `
         auth_read_passwd(testsuite_domain)
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [TESTSUITE PATCH v2] policy: handle files_list_pids() renaming in Refpolicy
  2023-01-17 17:09 ` [TESTSUITE PATCH v2] policy: handle files_list_pids() renaming in Refpolicy Christian Göttsche
@ 2023-01-20  9:38   ` Ondrej Mosnacek
  0 siblings, 0 replies; 6+ messages in thread
From: Ondrej Mosnacek @ 2023-01-20  9:38 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

On Tue, Jan 17, 2023 at 6:14 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
> files_list_pids() has been superseded and marked deprecated in the
> Reference Policy since Jun 2020[1].  In the latest release it has been
> completely removed[2].
>
> Call both the old and replacement interface conditionally to support
> recent Refpolicy versions as well as old ones.
>
> [1]: https://github.com/SELinuxProject/refpolicy/commit/be04bb3e7e63671ed8a3c501a2ee76e11c3b92bb
> [2]: https://github.com/SELinuxProject/refpolicy/commit/3ca0cd59d7a9b531dd3620a02940396343fe2ed5
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v2:
>    call both interfaces conditionally
> ---
>  policy/test_global.te | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/policy/test_global.te b/policy/test_global.te
> index e95102a..052c7dd 100644
> --- a/policy/test_global.te
> +++ b/policy/test_global.te
> @@ -121,7 +121,6 @@ allow testsuite_domain proc_t:file { getattr read open };
>  files_list_var(testsuite_domain)
>  files_list_home(testsuite_domain)
>  dev_read_rand(testsuite_domain)
> -files_list_pids(testsuite_domain)
>  require {
>         type root_t;
>         type etc_t;
> @@ -154,6 +153,14 @@ selinux_compute_create_context(testsuite_domain)
>  selinux_compute_relabel_context(testsuite_domain)
>  selinux_compute_user_contexts(testsuite_domain)
>
> +# Reference policy renamed files_list_pids() to files_list_runtime()
> +ifdef(`files_list_pids', `
> +    files_list_pids(testsuite_domain)
> +')
> +ifdef(`files_list_runtime', `
> +    files_list_runtime(testsuite_domain)
> +')
> +
>  ifdef(`distro_redhat', `
>      ifdef(`auth_read_passwd', `
>          auth_read_passwd(testsuite_domain)
> --
> 2.39.0

Thanks! Applied:
https://github.com/SELinuxProject/selinux-testsuite/commit/ebda879d7b6a7369fafc980b0bc223792d7bd5ab

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-20  9:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-16 21:46 [TESTSUITE PATCH] policy: drop usage of files_list_pids() Christian Göttsche
2023-01-17 10:00 ` Ondrej Mosnacek
2023-01-17 16:10   ` Christian Göttsche
2023-01-17 16:43     ` Ondrej Mosnacek
2023-01-17 17:09 ` [TESTSUITE PATCH v2] policy: handle files_list_pids() renaming in Refpolicy Christian Göttsche
2023-01-20  9:38   ` Ondrej Mosnacek

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.