On Tue, 2023-11-07 at 14:39 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
Hi everyone
I kindly ask your support to add the missing reviewed-by/acked-by. I
summarize what is missing below:
- @Mimi: patches 1, 2, 4, 5, 6, 19, 21, 22, 23 (IMA/EVM-specific
patches)
- @Al/@Christian: patches 10-17 (VFS-specific patches)
- @Paul: patches 10-23 (VFS-specific patches/new LSM hooks/new LSMs)
- @David Howells/@Jarkko: patch 18 (new LSM hook in the key subsystem)
- @Chuck Lever: patch 12 (new LSM hook in nfsd/vfs.c)
Paul, as I mentioned I currently based the patch set on lsm/dev-
staging, which include the following dependencies:
8f79e425c140 lsm: don't yet account for IMA in LSM_CONFIG_COUNT calculation
3c91a124f23d lsm: drop LSM_ID_IMA
I know you wanted to wait until at least rc1 to make lsm/dev. I will
help for rebasing my patch set, if needed.
Chuck, Mimi, there were two conflicts during the latest rebase:
d59b3515ab021 - nfsd: Handle EOPENSTALE correctly in the filecache
68279f9c9f59 - treewide: mark stuff as __ro_after_init
The first one required to change from 'goto out_nfserr' to 'goto out'
in the error path of patch 12.
The second one was automatically solved by kdiff3. It was because of
this change:
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -23,7 +23,7 @@
static struct rb_root integrity_iint_tree = RB_ROOT;
static DEFINE_RWLOCK(integrity_iint_lock);
-static struct kmem_cache *iint_cache __read_mostly;
+static struct kmem_cache *iint_cache __ro_after_init;
I'm running the IMA tests for every patch set version. So far, no
issues detected:
https://github.com/robertosassu/ima-evm-utils/actions/runs/6774439916
Please let me know if I can help with the process.
Thanks
Roberto
> IMA and EVM are not effectively LSMs, especially due to the fact that in
> the past they could not provide a security blob while there is another LSM
> active.
>
> That changed in the recent years, the LSM stacking feature now makes it
> possible to stack together multiple LSMs, and allows them to provide a
> security blob for most kernel objects. While the LSM stacking feature has
> some limitations being worked out, it is already suitable to make IMA and
> EVM as LSMs.
>
> In short, while this patch set is big, it does not make any functional
> change to IMA and EVM. IMA and EVM functions are called by the LSM
> infrastructure in the same places as before (except ima_post_path_mknod()),
> rather being hardcoded calls, and the inode metadata pointer is directly
> stored in the inode security blob rather than in a separate rbtree.
>
> To avoid functional changes, it was necessary to keep the 'integrity' LSM
> in addition to the newly introduced 'ima' and 'evm' LSMs, despite there is
> no LSM ID assigned to it. There are two reasons: first, IMA and EVM still
> share the same inode metadata, and thus cannot directly reserve space in
> the security blob for it; second, someone needs to initialize 'ima' and
> 'evm' exactly in this order, as the LSM infrastructure cannot guarantee
> that.
>
> The patch set is organized as follows.
>
> Patches 1-9 make IMA and EVM functions suitable to be registered to the LSM
> infrastructure, by aligning function parameters.
>
> Patches 10-18 add new LSM hooks in the same places where IMA and EVM
> functions are called, if there is no LSM hook already.
>
> Patches 19-22 do the bulk of the work, introduce the new LSMs 'ima' and
> 'evm', and move hardcoded calls to IMA, EVM and integrity functions to
> those LSMs. In addition, they reserve one slot for the 'evm' LSM to supply
> an xattr with the inode_init_security hook.
>
> Finally, patch 23 removes the rbtree used to bind integrity metadata to the
> inodes, and instead reserves a space in the inode security blob to store
> the pointer to that metadata. This also brings performance improvements due
> to retrieving metadata in constant time, as opposed to logarithmic.
>
> The patch set applies on top of lsm/dev-staging, commit ba7ce019d3e9 ("lsm:
> convert security_setselfattr() to use memdup_user()"). No need to merge
> linux-integrity/next-integrity-testing.
>
> Changelog:
>
> v4:
> - Improve short and long description of
> security_inode_post_create_tmpfile(), security_inode_post_set_acl(),
> security_inode_post_remove_acl() and security_file_post_open()
> (suggested by Mimi)
> - Improve commit message of 'ima: Move to LSM infrastructure' (suggested
> by Mimi)
>
> v3:
> - Drop 'ima: Align ima_post_path_mknod() definition with LSM
> infrastructure' and 'ima: Align ima_post_create_tmpfile() definition
> with LSM infrastructure', define the new LSM hooks with the same
> IMA parameters instead (suggested by Mimi)
> - Do IS_PRIVATE() check in security_path_post_mknod() and
> security_inode_post_create_tmpfile() on the new inode rather than the
> parent directory (in the post method it is available)
> - Don't export ima_file_check() (suggested by Stefan)
> - Remove redundant check of file mode in ima_post_path_mknod() (suggested
> by Mimi)
> - Mention that ima_post_path_mknod() is now conditionally invoked when
> CONFIG_SECURITY_PATH=y (suggested by Mimi)
> - Mention when a LSM hook will be introduced in the IMA/EVM alignment
> patches (suggested by Mimi)
> - Simplify the commit messages when introducing a new LSM hook
> - Still keep the 'extern' in the function declaration, until the
> declaration is removed (suggested by Mimi)
> - Improve documentation of security_file_pre_free()
> - Register 'ima' and 'evm' as standalone LSMs (suggested by Paul)
> - Initialize the 'ima' and 'evm' LSMs from 'integrity', to keep the
> original ordering of IMA and EVM functions as when they were hardcoded
> - Return the IMA and EVM LSM IDs to 'integrity' for registration of the
> integrity-specific hooks
> - Reserve an xattr slot from the 'evm' LSM instead of 'integrity'
> - Pass the LSM ID to init_ima_appraise_lsm()
>
> v2:
> - Add description for newly introduced LSM hooks (suggested by Casey)
> - Clarify in the description of security_file_pre_free() that actions can
> be performed while the file is still open
>
> v1:
> - Drop 'evm: Complete description of evm_inode_setattr()', 'fs: Fix
> description of vfs_tmpfile()' and 'security: Introduce LSM_ORDER_LAST',
> they were sent separately (suggested by Christian Brauner)
> - Replace dentry with file descriptor parameter for
> security_inode_post_create_tmpfile()
> - Introduce mode_stripped and pass it as mode argument to
> security_path_mknod() and security_path_post_mknod()
> - Use goto in do_mknodat() and __vfs_removexattr_locked() (suggested by
> Mimi)
> - Replace __lsm_ro_after_init with __ro_after_init
> - Modify short description of security_inode_post_create_tmpfile() and
> security_inode_post_set_acl() (suggested by Stefan)
> - Move security_inode_post_setattr() just after security_inode_setattr()
> (suggested by Mimi)
> - Modify short description of security_key_post_create_or_update()
> (suggested by Mimi)
> - Add back exported functions ima_file_check() and
> evm_inode_init_security() respectively to ima.h and evm.h (reported by
> kernel robot)
> - Remove extern from prototype declarations and fix style issues
> - Remove unnecessary include of linux/lsm_hooks.h in ima_main.c and
> ima_appraise.c
>
> Roberto Sassu (23):
> ima: Align ima_inode_post_setattr() definition with LSM infrastructure
> ima: Align ima_file_mprotect() definition with LSM infrastructure
> ima: Align ima_inode_setxattr() definition with LSM infrastructure
> ima: Align ima_inode_removexattr() definition with LSM infrastructure
> ima: Align ima_post_read_file() definition with LSM infrastructure
> evm: Align evm_inode_post_setattr() definition with LSM infrastructure
> evm: Align evm_inode_setxattr() definition with LSM infrastructure
> evm: Align evm_inode_post_setxattr() definition with LSM
> infrastructure
> security: Align inode_setattr hook definition with EVM
> security: Introduce inode_post_setattr hook
> security: Introduce inode_post_removexattr hook
> security: Introduce file_post_open hook
> security: Introduce file_pre_free_security hook
> security: Introduce path_post_mknod hook
> security: Introduce inode_post_create_tmpfile hook
> security: Introduce inode_post_set_acl hook
> security: Introduce inode_post_remove_acl hook
> security: Introduce key_post_create_or_update hook
> ima: Move to LSM infrastructure
> ima: Move IMA-Appraisal to LSM infrastructure
> evm: Move to LSM infrastructure
> integrity: Move integrity functions to the LSM infrastructure
> integrity: Switch from rbtree to LSM-managed blob for
> integrity_iint_cache
>
> fs/attr.c | 5 +-
> fs/file_table.c | 3 +-
> fs/namei.c | 12 +-
> fs/nfsd/vfs.c | 3 +-
> fs/open.c | 1 -
> fs/posix_acl.c | 5 +-
> fs/xattr.c | 9 +-
> include/linux/evm.h | 103 ----------
> include/linux/ima.h | 142 --------------
> include/linux/integrity.h | 26 ---
> include/linux/lsm_hook_defs.h | 20 +-
> include/linux/security.h | 59 ++++++
> include/uapi/linux/lsm.h | 2 +
> security/integrity/evm/evm_main.c | 138 ++++++++++++--
> security/integrity/iint.c | 113 +++++------
> security/integrity/ima/ima.h | 11 ++
> security/integrity/ima/ima_appraise.c | 37 +++-
> security/integrity/ima/ima_main.c | 96 ++++++++--
> security/integrity/integrity.h | 58 +++++-
> security/keys/key.c | 10 +-
> security/security.c | 261 ++++++++++++++++----------
> security/selinux/hooks.c | 3 +-
> security/smack/smack_lsm.c | 4 +-
> 23 files changed, 614 insertions(+), 507 deletions(-)
>
