Message-ID: <34085058-ff5f-c28e-c716-6f4fa71747a3@linux.ibm.com>
Date:   Wed, 1 Dec 2021 12:35:52 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [RFC 17/20] ima: Use integrity_admin_ns_capable() to check
 corresponding capability
Content-Language: en-US
To:     jejb@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>
References: <20211130160654.1418231-1-stefanb@linux.ibm.com>
 <20211130160654.1418231-18-stefanb@linux.ibm.com>
 <7c751783b28766412f158e5ca074748ed18070bd.camel@linux.ibm.com>
From:   Stefan Berger <stefanb@linux.ibm.com>
In-Reply-To: <7c751783b28766412f158e5ca074748ed18070bd.camel@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
Precedence: bulk


On 12/1/21 11:58, James Bottomley wrote:
> 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

Casey suggested using CAP_MAC_ADMIN, which I think would also work.

     CAP_MAC_ADMIN (since Linux 2.6.25)
               Allow MAC configuration or state changes. Implemented for
               the Smack Linux Security Module (LSM).


Down the road I think we should cover setting file extended attributes 
with the same capability as well for when a user signs files or installs 
packages with file signatures.  A container runtime could hold 
CAP_SYS_ADMIN while setting up a container and mounting filesystems and 
drop it for the first process started there. Since we are using the user 
namespace to spawn an IMA namespace, we would then require 
CAP_SYSTEM_ADMIN to be left available so that the user can do IMA 
related stuff in the container (set or append to the policy, write file 
signatures). I am not sure whether that should be the case or rather 
give the user something finer grained, such as CAP_MAC_ADMIN. So, it's 
about granularity...


>
> 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.

At least docker containers drop CAP_SYS_ADMIN. I am not sure what the 
decision was based on but probably they don't want to give the user what 
is not absolutely necessary, but usage of user namespaces (with IMA 
namespaces) would kind of force it to be available then to do 
IMA-related stuff ...

Following this man page here 
https://man7.org/linux/man-pages/man7/user_namespaces.7.html

CAP_SYS_ADMIN in a user namespace is about

- bind-mounting filesystems

- mounting /proc filesystems

- creating nested user namespaces

- configuring UTS namespace

- configuring whether setgroups() can be used

- usage of setns()


Do we want to add '- only way of *setting up* IMA related stuff' to this 
list?

   Stefan


>
> James
>
>