Keyrings Archive mirror
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.ibm.com>
To: Roberto Sassu <roberto.sassu@huaweicloud.com>,
	viro@zeniv.linux.org.uk, brauner@kernel.org,
	chuck.lever@oracle.com, jlayton@kernel.org, neilb@suse.de,
	kolga@netapp.com, Dai.Ngo@oracle.com, tom@talpey.com,
	paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
	dmitry.kasatkin@gmail.com, dhowells@redhat.com,
	jarkko@kernel.org, stephen.smalley.work@gmail.com,
	eparis@parisplace.org, casey@schaufler-ca.com, shuah@kernel.org,
	mic@digikod.net
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-nfs@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-integrity@vger.kernel.org, keyrings@vger.kernel.org,
	selinux@vger.kernel.org, linux-kselftest@vger.kernel.org,
	Roberto Sassu <roberto.sassu@huawei.com>
Subject: Re: [PATCH v8 21/24] evm: Move to LSM infrastructure
Date: Tue, 02 Jan 2024 12:44:16 -0500	[thread overview]
Message-ID: <08e90ff7754aad45785ab05576f308a7aaae3438.camel@linux.ibm.com> (raw)
In-Reply-To: <42911719-547d-443a-b2f2-07b0cfb11f7a@huaweicloud.com>

On Tue, 2024-01-02 at 12:56 +0100, Roberto Sassu wrote:
> On 12/26/2023 11:13 PM, Mimi Zohar wrote:
> > On Thu, 2023-12-14 at 18:08 +0100, Roberto Sassu wrote:
> >> From: Roberto Sassu <roberto.sassu@huawei.com>
> >>
> >> As for IMA, move hardcoded EVM function calls from various places in the
> >> kernel to the LSM infrastructure, by introducing a new LSM named 'evm'
> >> (last and always enabled like 'ima'). The order in the Makefile ensures
> >> that 'evm' hooks are executed after 'ima' ones.
> >>
> >> Make EVM functions as static (except for evm_inode_init_security(), which
> >> is exported), and register them as hook implementations in init_evm_lsm().
> >>
> >> Unlike before (see commit to move IMA to the LSM infrastructure),
> >> evm_inode_post_setattr(), evm_inode_post_set_acl(),
> >> evm_inode_post_remove_acl(), and evm_inode_post_removexattr() are not
> >> executed for private inodes.
> >>
> > 
> > Missing is a comment on moving the inline function definitions -
> > evm_inode_remove_acl(), evm_inode_post_remove_acl(), and
> > evm_inode_post_set_acl() - to evm_main.c.
> 
> Ok.
> 
> >> Finally, add the LSM_ID_EVM case in lsm_list_modules_test.c
> >>
> >> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> >> ---
> > 
> > [...]
> >> @@ -2307,9 +2299,7 @@ int security_inode_setxattr(struct mnt_idmap *idmap,
> >>   
> >>   	if (ret == 1)
> >>   		ret = cap_inode_setxattr(dentry, name, value, size, flags);
> >> -	if (ret)
> >> -		return ret;
> >> -	return evm_inode_setxattr(idmap, dentry, name, value, size, flags);
> >> +	return ret;
> >>   }
> > 
> > Even though capability will be called after EVM, it doesn't make a
> > difference in this instance.
> > 
> > [...]
> > 
> >>   /**
> >> @@ -2493,9 +2472,7 @@ int security_inode_removexattr(struct mnt_idmap *idmap,
> >>   	ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name);
> >>   	if (ret == 1)
> >>   		ret = cap_inode_removexattr(idmap, dentry, name);
> >> -	if (ret)
> >> -		return ret;
> >> -	return evm_inode_removexattr(idmap, dentry, name);
> >> +	return ret;
> >>   }
> > 
> > 'security.capability' is one of the EVM protected xattrs.  As
> > capability isn't an LSM, it will now be called after EVM, which is a
> > problem.
> 
> Uhm, according to this comment in security_inode_removexattr() and 
> security_inode_setxattr():
> 
> 	/*
> 	 * SELinux and Smack integrate the cap call,
> 	 * so assume that all LSMs supplying this call do so.
> 	 */
> 
> We can add the call to IMA and EVM as well, to be compliant.

SELinux and Smack are the only current LSMs that register the
security_inode_removexattr hook.  Both enforce mandatory access
control,
so their calling capabilities to enforce DAC kind of makes sense.  I'm
not sure it makes sense for IMA and EVM to call capability directly,
just because of the comment.

> However, I'm missing why the two cases are different. It seems 
> cap_inode_set/removexattr() are doing just checks.

Both IMA and EVM require CAP_SYS_ADMIN to write/remove security.ima and
security.evm respectively.  In addition, EVM must recalculate
security.evm if any protected security xattrs are set or
removed.   However, security.evm is updated on
security_inode_post_setxattr, not security_inode_setxattr.

Mimi


  reply	other threads:[~2024-01-02 17:45 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-14 17:08 [PATCH v8 00/24] security: Move IMA and EVM to the LSM infrastructure Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 01/24] ima: Align ima_inode_post_setattr() definition with " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 02/24] ima: Align ima_file_mprotect() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 03/24] ima: Align ima_inode_setxattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 04/24] ima: Align ima_inode_removexattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 05/24] ima: Align ima_post_read_file() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 06/24] evm: Align evm_inode_post_setattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 07/24] evm: Align evm_inode_setxattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 08/24] evm: Align evm_inode_post_setxattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 09/24] security: Align inode_setattr hook definition with EVM Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 10/24] security: Introduce inode_post_setattr hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 11/24] security: Introduce inode_post_removexattr hook Roberto Sassu
2023-12-15 21:05   ` Casey Schaufler
2023-12-14 17:08 ` [PATCH v8 12/24] security: Introduce file_post_open hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 13/24] security: Introduce file_release hook Roberto Sassu
2023-12-15 21:41   ` Casey Schaufler
2023-12-14 17:08 ` [PATCH v8 14/24] security: Introduce path_post_mknod hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 15/24] security: Introduce inode_post_create_tmpfile hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 16/24] security: Introduce inode_post_set_acl hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 17/24] security: Introduce inode_post_remove_acl hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 18/24] security: Introduce key_post_create_or_update hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 19/24] ima: Move to LSM infrastructure Roberto Sassu
2023-12-26 18:14   ` Mimi Zohar
2023-12-26 20:14     ` Casey Schaufler
2023-12-27 19:52       ` Mimi Zohar
2023-12-27 20:20         ` Casey Schaufler
2023-12-14 17:08 ` [PATCH v8 20/24] ima: Move IMA-Appraisal " Roberto Sassu
2023-12-26 22:15   ` Mimi Zohar
2023-12-14 17:08 ` [PATCH v8 21/24] evm: Move " Roberto Sassu
2023-12-15 21:54   ` Casey Schaufler
2023-12-26 22:13   ` Mimi Zohar
2024-01-02 11:56     ` Roberto Sassu
2024-01-02 17:44       ` Mimi Zohar [this message]
2023-12-14 17:08 ` [PATCH v8 22/24] evm: Make it independent from 'integrity' LSM Roberto Sassu
2023-12-27  3:12   ` Mimi Zohar
2023-12-14 17:08 ` [PATCH v8 23/24] ima: " Roberto Sassu
2023-12-18  8:21   ` Roberto Sassu
2023-12-27 13:22   ` Mimi Zohar
2023-12-27 16:39     ` Roberto Sassu
2023-12-27 19:21       ` Mimi Zohar
2024-01-02 10:53         ` Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 24/24] integrity: Remove LSM Roberto Sassu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=08e90ff7754aad45785ab05576f308a7aaae3438.camel@linux.ibm.com \
    --to=zohar@linux.ibm.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=brauner@kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=chuck.lever@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=eparis@parisplace.org \
    --cc=jarkko@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=kolga@netapp.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@digikod.net \
    --cc=neilb@suse.de \
    --cc=paul@paul-moore.com \
    --cc=roberto.sassu@huawei.com \
    --cc=roberto.sassu@huaweicloud.com \
    --cc=selinux@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tom@talpey.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).