| Message ID | 20230116214626.28955-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Ondrej Mosnáček |
| Headers | show |
| Series | [TESTSUITE] policy: drop usage of files_list_pids() | expand |
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 >
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. >
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,
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;
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(-)