Re: [RFC 17/20] ima: Use integrity_admin_ns_capable() to check corresponding capability

James Bottomley
2021-12-01T16:58:42Z

On Tue, 2021-11-30 at 11:06 -0500, Stefan Berger wrote:
> From: Denis Semakin <denis.semakin@huawei.com>
>
> Use integrity_admin_ns_capable() to check corresponding capability to
> allow read/write IMA policy without CAP_SYS_ADMIN but with
> CAP_INTEGRITY_ADMIN.
>
> Signed-off-by: Denis Semakin <denis.semakin@huawei.com>
> ---
> security/integrity/ima/ima_fs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/ima_fs.c
> b/security/integrity/ima/ima_fs.c
> index fd2798f2d224..6766bb8262f2 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -393,7 +393,7 @@ static int ima_open_policy(struct inode *inode,
> struct file *filp)
> #else
> if ((filp->f_flags & O_ACCMODE) != O_RDONLY)
> return -EACCES;
> - if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
> + if (!integrity_admin_ns_capable(ns->user_ns))

so this one is basically replacing what you did in RFC 16/20, which
seems a little redundant.

The question I'd like to ask is: is there still a reason for needing
CAP_INTEGRITY_ADMIN? My thinking is that now IMA is pretty much tied
to requiring a user (and a mount, because of securityfs_ns) namespace,
there might not be a pressing need for an admin capability separated
from CAP_SYS_ADMIN because the owner of the user namespace passes the
ns_capable(..., CAP_SYS_ADMIN) check. The rationale in

https://kernsec.org/wiki/index.php/IMA_Namespacing_design_considerations

Is effectively "because CAP_SYS_ADMIN is too powerful" but that's no
longer true of the user namespace owner. It only passes the ns_capable
() check not the capable() one, so while it does get CAP_SYS_ADMIN, it
can only use it in a few situations which represent quite a power
reduction already.

James