Message-ID: <7c751783b28766412f158e5ca074748ed18070bd.camel@linux.ibm.com>
Subject: Re: [RFC 17/20] ima: Use integrity_admin_ns_capable() to check
 corresponding capability
From:   James Bottomley <jejb@linux.ibm.com>
Reply-To: jejb@linux.ibm.com
To:     Stefan Berger <stefanb@linux.ibm.com>,
        linux-integrity@vger.kernel.org
Cc:     zohar@linux.ibm.com, serge@hallyn.com,
        christian.brauner@ubuntu.com, containers@lists.linux.dev,
        dmitry.kasatkin@gmail.com, ebiederm@xmission.com,
        krzysztof.struczynski@huawei.com, roberto.sassu@huawei.com,
        mpeters@redhat.com, lhinds@redhat.com, lsturman@redhat.com,
        puiterwi@redhat.com, jamjoom@us.ibm.com,
        linux-kernel@vger.kernel.org, paul@paul-moore.com, rgb@redhat.com,
        linux-security-module@vger.kernel.org, jmorris@namei.org,
        Denis Semakin <denis.semakin@huawei.com>
Date:   Wed, 01 Dec 2021 11:58:09 -0500
In-Reply-To: <20211130160654.1418231-18-stefanb@linux.ibm.com>
References: <20211130160654.1418231-1-stefanb@linux.ibm.com>
         <20211130160654.1418231-18-stefanb@linux.ibm.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Precedence: bulk

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