| Message ID | 20230228141247.626736-2-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Ondrej Mosnáček |
| Headers | show |
| Series | Infiniband test fixes/improvements | expand |
On Tue, Feb 28, 2023 at 9:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > The ibv_create_cq() operation requires the caller to be able to lock > enough memory (RLIMIT_MEMLOCK). In some environments (such as RHEL-8) > the default resource limits may not be enough, requiring CAP_IPC_LOCK to > go above the limit. To make sure the test works also under stricter > resource limits, grant CAP_IPC_LOCK to test_ibpkey_access_t. > > Reported-by: Mimi Zohar <zohar@linux.ibm.com> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > policy/test_ibpkey.te | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/policy/test_ibpkey.te b/policy/test_ibpkey.te > index 863ff16..97f0c3c 100644 > --- a/policy/test_ibpkey.te > +++ b/policy/test_ibpkey.te > @@ -10,6 +10,8 @@ type test_ibpkey_access_t; > testsuite_domain_type(test_ibpkey_access_t) > typeattribute test_ibpkey_access_t ibpkeydomain; > > +allow test_ibpkey_access_t self:capability ipc_lock; FWIW, I brought this up back in 2019 and have been carrying a local selinux-testsuite patch for this ever since (it's the only way to get a clean run of the IB tests). While it can be fixed in the selinux-testsuite policy, I believe this is a more general problem and should probably be fixed in refpol. https://lore.kernel.org/selinux/CAHC9VhTuYi+W0RukEV4WNrP5X_AFeouaWMsdbgxSL1v04mouWw@mail.gmail.com/ > dev_rw_infiniband_dev(test_ibpkey_access_t) > dev_rw_sysfs(test_ibpkey_access_t) > > -- > 2.39.2
On Tue, 2023-02-28 at 11:51 -0500, Paul Moore wrote: > On Tue, Feb 28, 2023 at 9:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > The ibv_create_cq() operation requires the caller to be able to lock > > enough memory (RLIMIT_MEMLOCK). In some environments (such as RHEL-8) > > the default resource limits may not be enough, requiring CAP_IPC_LOCK to > > go above the limit. To make sure the test works also under stricter > > resource limits, grant CAP_IPC_LOCK to test_ibpkey_access_t. > > > > Reported-by: Mimi Zohar <zohar@linux.ibm.com> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > --- > > policy/test_ibpkey.te | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/policy/test_ibpkey.te b/policy/test_ibpkey.te > > index 863ff16..97f0c3c 100644 > > --- a/policy/test_ibpkey.te > > +++ b/policy/test_ibpkey.te > > @@ -10,6 +10,8 @@ type test_ibpkey_access_t; > > testsuite_domain_type(test_ibpkey_access_t) > > typeattribute test_ibpkey_access_t ibpkeydomain; > > > > +allow test_ibpkey_access_t self:capability ipc_lock; > > FWIW, I brought this up back in 2019 and have been carrying a local > selinux-testsuite patch for this ever since (it's the only way to get > a clean run of the IB tests). Confirmed, with this change the SELinux infiniband tests are now working on stable linux-4.19.y. > While it can be fixed in the > selinux-testsuite policy, I believe this is a more general problem and > should probably be fixed in refpol. > > https://lore.kernel.org/selinux/CAHC9VhTuYi+W0RukEV4WNrP5X_AFeouaWMsdbgxSL1v04mouWw@mail.gmail.com/ > > > dev_rw_infiniband_dev(test_ibpkey_access_t) > > dev_rw_sysfs(test_ibpkey_access_t)
On Tue, Feb 28, 2023 at 5:51 PM Paul Moore <paul@paul-moore.com> wrote: > On Tue, Feb 28, 2023 at 9:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > The ibv_create_cq() operation requires the caller to be able to lock > > enough memory (RLIMIT_MEMLOCK). In some environments (such as RHEL-8) > > the default resource limits may not be enough, requiring CAP_IPC_LOCK to > > go above the limit. To make sure the test works also under stricter > > resource limits, grant CAP_IPC_LOCK to test_ibpkey_access_t. > > > > Reported-by: Mimi Zohar <zohar@linux.ibm.com> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > --- > > policy/test_ibpkey.te | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/policy/test_ibpkey.te b/policy/test_ibpkey.te > > index 863ff16..97f0c3c 100644 > > --- a/policy/test_ibpkey.te > > +++ b/policy/test_ibpkey.te > > @@ -10,6 +10,8 @@ type test_ibpkey_access_t; > > testsuite_domain_type(test_ibpkey_access_t) > > typeattribute test_ibpkey_access_t ibpkeydomain; > > > > +allow test_ibpkey_access_t self:capability ipc_lock; > > FWIW, I brought this up back in 2019 and have been carrying a local > selinux-testsuite patch for this ever since (it's the only way to get > a clean run of the IB tests). While it can be fixed in the > selinux-testsuite policy, I believe this is a more general problem and > should probably be fixed in refpol. > > https://lore.kernel.org/selinux/CAHC9VhTuYi+W0RukEV4WNrP5X_AFeouaWMsdbgxSL1v04mouWw@mail.gmail.com/ I don't understand how you'd like this to be fixed in the system policy... I don't think there is any policy interface that would semantically match "any users of the SELinux IB hooks" or "callers of ibv_create_cq()" that we could stick the capability rule into. At least the testsuite policy doesn't use any such interface. Closest to it would be dev_rw_infiniband_dev(), but that doesn't seem like the right place. Not to mention that the fact whether the capability is required or not depends on the resource limits imposed on the process. If its RLIMIT_MEMLOCK limit is sufficient, a process is perfectly able to create the cq without CAP_IPC_LOCK. Automatically granting it to all domains that use InfiniBand in some way "just in case" would potentially grant it also to domains that don't actually need it, violating the principle of least privilege. -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
On Wed, Mar 1, 2023 at 10:21 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Tue, Feb 28, 2023 at 5:51 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Feb 28, 2023 at 9:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > The ibv_create_cq() operation requires the caller to be able to lock > > > enough memory (RLIMIT_MEMLOCK). In some environments (such as RHEL-8) > > > the default resource limits may not be enough, requiring CAP_IPC_LOCK to > > > go above the limit. To make sure the test works also under stricter > > > resource limits, grant CAP_IPC_LOCK to test_ibpkey_access_t. > > > > > > Reported-by: Mimi Zohar <zohar@linux.ibm.com> > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > --- > > > policy/test_ibpkey.te | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/policy/test_ibpkey.te b/policy/test_ibpkey.te > > > index 863ff16..97f0c3c 100644 > > > --- a/policy/test_ibpkey.te > > > +++ b/policy/test_ibpkey.te > > > @@ -10,6 +10,8 @@ type test_ibpkey_access_t; > > > testsuite_domain_type(test_ibpkey_access_t) > > > typeattribute test_ibpkey_access_t ibpkeydomain; > > > > > > +allow test_ibpkey_access_t self:capability ipc_lock; > > > > FWIW, I brought this up back in 2019 and have been carrying a local > > selinux-testsuite patch for this ever since (it's the only way to get > > a clean run of the IB tests). While it can be fixed in the > > selinux-testsuite policy, I believe this is a more general problem and > > should probably be fixed in refpol. > > > > https://lore.kernel.org/selinux/CAHC9VhTuYi+W0RukEV4WNrP5X_AFeouaWMsdbgxSL1v04mouWw@mail.gmail.com/ > > I don't understand how you'd like this to be fixed in the system > policy... I don't think there is any policy interface that would > semantically match "any users of the SELinux IB hooks" or "callers of > ibv_create_cq()" that we could stick the capability rule into. At > least the testsuite policy doesn't use any such interface. Closest to > it would be dev_rw_infiniband_dev(), but that doesn't seem like the > right place. Look at it this way, the selinux-testsuite is not doing anything particularly unusual with respect to talking over IB; if the tests need that permission it seems reasonable that normal IB users would also need these permissions. > Not to mention that the fact whether the capability is required or not > depends on the resource limits imposed on the process. If its > RLIMIT_MEMLOCK limit is sufficient, a process is perfectly able to > create the cq without CAP_IPC_LOCK. Automatically granting it to all > domains that use InfiniBand in some way "just in case" would > potentially grant it also to domains that don't actually need it, > violating the principle of least privilege. Once again, the selinux-testsuite is not doing anything particularly unusual so if we are hitting this it seems reasonable that other users are hitting this as well. If you're concerned about granting CAP_IPC_LOCK you could always put it in a dedicated IB/RDMA refpol interface as I believe this is just an issue with the IB/RDMA verb interface involving CQs/QPs and not the underlying IB protocol layer. Say something like "dev_rw_infiniband_rdma()"* which would call "dev_rw_infiniband()"* and add the CAP_IPC_LOCK permission. It would be good to hear Chris' take on this. * Upstream refpol appears to have shortened the interface to "dev_rw_infiniband()".
On Wed, Mar 1, 2023 at 7:49 PM Paul Moore <paul@paul-moore.com> wrote: > On Wed, Mar 1, 2023 at 10:21 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Tue, Feb 28, 2023 at 5:51 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Tue, Feb 28, 2023 at 9:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > The ibv_create_cq() operation requires the caller to be able to lock > > > > enough memory (RLIMIT_MEMLOCK). In some environments (such as RHEL-8) > > > > the default resource limits may not be enough, requiring CAP_IPC_LOCK to > > > > go above the limit. To make sure the test works also under stricter > > > > resource limits, grant CAP_IPC_LOCK to test_ibpkey_access_t. > > > > > > > > Reported-by: Mimi Zohar <zohar@linux.ibm.com> > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > --- > > > > policy/test_ibpkey.te | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/policy/test_ibpkey.te b/policy/test_ibpkey.te > > > > index 863ff16..97f0c3c 100644 > > > > --- a/policy/test_ibpkey.te > > > > +++ b/policy/test_ibpkey.te > > > > @@ -10,6 +10,8 @@ type test_ibpkey_access_t; > > > > testsuite_domain_type(test_ibpkey_access_t) > > > > typeattribute test_ibpkey_access_t ibpkeydomain; > > > > > > > > +allow test_ibpkey_access_t self:capability ipc_lock; > > > > > > FWIW, I brought this up back in 2019 and have been carrying a local > > > selinux-testsuite patch for this ever since (it's the only way to get > > > a clean run of the IB tests). While it can be fixed in the > > > selinux-testsuite policy, I believe this is a more general problem and > > > should probably be fixed in refpol. > > > > > > https://lore.kernel.org/selinux/CAHC9VhTuYi+W0RukEV4WNrP5X_AFeouaWMsdbgxSL1v04mouWw@mail.gmail.com/ > > > > I don't understand how you'd like this to be fixed in the system > > policy... I don't think there is any policy interface that would > > semantically match "any users of the SELinux IB hooks" or "callers of > > ibv_create_cq()" that we could stick the capability rule into. At > > least the testsuite policy doesn't use any such interface. Closest to > > it would be dev_rw_infiniband_dev(), but that doesn't seem like the > > right place. > > Look at it this way, the selinux-testsuite is not doing anything > particularly unusual with respect to talking over IB; if the tests > need that permission it seems reasonable that normal IB users would > also need these permissions. > > > Not to mention that the fact whether the capability is required or not > > depends on the resource limits imposed on the process. If its > > RLIMIT_MEMLOCK limit is sufficient, a process is perfectly able to > > create the cq without CAP_IPC_LOCK. Automatically granting it to all > > domains that use InfiniBand in some way "just in case" would > > potentially grant it also to domains that don't actually need it, > > violating the principle of least privilege. > > Once again, the selinux-testsuite is not doing anything particularly > unusual so if we are hitting this it seems reasonable that other users > are hitting this as well. If you're concerned about granting > CAP_IPC_LOCK you could always put it in a dedicated IB/RDMA refpol > interface as I believe this is just an issue with the IB/RDMA verb > interface involving CQs/QPs and not the underlying IB protocol layer. > Say something like "dev_rw_infiniband_rdma()"* which would call > "dev_rw_infiniband()"* and add the CAP_IPC_LOCK permission. > > It would be good to hear Chris' take on this. Okay, so I guess you addressed your comments more towards refpolicy maintainer/contributors than to me as the submitter/testsuite maintainer and I didn't have to react so defensively... I agree that having better semantic interfaces for RDMA users in refpolicy and Fedora policy would be nice, but I also wouldn't block having a working testsuite on that. I'll be happy to switch any new appropriate interfaces (and replicate them in Fedora policy) once they are available. > > * Upstream refpol appears to have shortened the interface to > "dev_rw_infiniband()". > > -- > paul-moore.com > -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
On Fri, Mar 3, 2023 at 7:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Wed, Mar 1, 2023 at 7:49 PM Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Mar 1, 2023 at 10:21 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > On Tue, Feb 28, 2023 at 5:51 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Tue, Feb 28, 2023 at 9:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > > > The ibv_create_cq() operation requires the caller to be able to lock > > > > > enough memory (RLIMIT_MEMLOCK). In some environments (such as RHEL-8) > > > > > the default resource limits may not be enough, requiring CAP_IPC_LOCK to > > > > > go above the limit. To make sure the test works also under stricter > > > > > resource limits, grant CAP_IPC_LOCK to test_ibpkey_access_t. > > > > > > > > > > Reported-by: Mimi Zohar <zohar@linux.ibm.com> > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > > --- > > > > > policy/test_ibpkey.te | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/policy/test_ibpkey.te b/policy/test_ibpkey.te > > > > > index 863ff16..97f0c3c 100644 > > > > > --- a/policy/test_ibpkey.te > > > > > +++ b/policy/test_ibpkey.te > > > > > @@ -10,6 +10,8 @@ type test_ibpkey_access_t; > > > > > testsuite_domain_type(test_ibpkey_access_t) > > > > > typeattribute test_ibpkey_access_t ibpkeydomain; > > > > > > > > > > +allow test_ibpkey_access_t self:capability ipc_lock; > > > > > > > > FWIW, I brought this up back in 2019 and have been carrying a local > > > > selinux-testsuite patch for this ever since (it's the only way to get > > > > a clean run of the IB tests). While it can be fixed in the > > > > selinux-testsuite policy, I believe this is a more general problem and > > > > should probably be fixed in refpol. > > > > > > > > https://lore.kernel.org/selinux/CAHC9VhTuYi+W0RukEV4WNrP5X_AFeouaWMsdbgxSL1v04mouWw@mail.gmail.com/ > > > > > > I don't understand how you'd like this to be fixed in the system > > > policy... I don't think there is any policy interface that would > > > semantically match "any users of the SELinux IB hooks" or "callers of > > > ibv_create_cq()" that we could stick the capability rule into. At > > > least the testsuite policy doesn't use any such interface. Closest to > > > it would be dev_rw_infiniband_dev(), but that doesn't seem like the > > > right place. > > > > Look at it this way, the selinux-testsuite is not doing anything > > particularly unusual with respect to talking over IB; if the tests > > need that permission it seems reasonable that normal IB users would > > also need these permissions. > > > > > Not to mention that the fact whether the capability is required or not > > > depends on the resource limits imposed on the process. If its > > > RLIMIT_MEMLOCK limit is sufficient, a process is perfectly able to > > > create the cq without CAP_IPC_LOCK. Automatically granting it to all > > > domains that use InfiniBand in some way "just in case" would > > > potentially grant it also to domains that don't actually need it, > > > violating the principle of least privilege. > > > > Once again, the selinux-testsuite is not doing anything particularly > > unusual so if we are hitting this it seems reasonable that other users > > are hitting this as well. If you're concerned about granting > > CAP_IPC_LOCK you could always put it in a dedicated IB/RDMA refpol > > interface as I believe this is just an issue with the IB/RDMA verb > > interface involving CQs/QPs and not the underlying IB protocol layer. > > Say something like "dev_rw_infiniband_rdma()"* which would call > > "dev_rw_infiniband()"* and add the CAP_IPC_LOCK permission. > > > > It would be good to hear Chris' take on this. > > Okay, so I guess you addressed your comments more towards refpolicy > maintainer/contributors than to me as the submitter/testsuite > maintainer and I didn't have to react so defensively... > > I agree that having better semantic interfaces for RDMA users in > refpolicy and Fedora policy would be nice, but I also wouldn't block > having a working testsuite on that. I'll be happy to switch any new > appropriate interfaces (and replicate them in Fedora policy) once they > are available. Yes, the test suite needs to be functional even if the reference policy is missing important permissions, I was hoping to see some comment from Chris or perhaps some of the other policy folks about this but it looks like that isn't going to happen in a timely fashion.
diff --git a/policy/test_ibpkey.te b/policy/test_ibpkey.te index 863ff16..97f0c3c 100644 --- a/policy/test_ibpkey.te +++ b/policy/test_ibpkey.te @@ -10,6 +10,8 @@ type test_ibpkey_access_t; testsuite_domain_type(test_ibpkey_access_t) typeattribute test_ibpkey_access_t ibpkeydomain; +allow test_ibpkey_access_t self:capability ipc_lock; + dev_rw_infiniband_dev(test_ibpkey_access_t) dev_rw_sysfs(test_ibpkey_access_t)
The ibv_create_cq() operation requires the caller to be able to lock enough memory (RLIMIT_MEMLOCK). In some environments (such as RHEL-8) the default resource limits may not be enough, requiring CAP_IPC_LOCK to go above the limit. To make sure the test works also under stricter resource limits, grant CAP_IPC_LOCK to test_ibpkey_access_t. Reported-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- policy/test_ibpkey.te | 2 ++ 1 file changed, 2 insertions(+)