Re: [PATCH v9 23/25] evm: Make it independent from 'integrity' LSM

On 1/15/2024 10:18 AM, Roberto Sassu wrote:

From: Roberto Sassu roberto.sassu@huawei.com

Define a new structure for EVM-specific metadata, called evm_iint_cache, and embed it in the inode security blob. Introduce evm_iint_inode() to retrieve metadata, and register evm_inode_alloc_security() for the inode_alloc_security LSM hook, to initialize the structure (before splitting metadata, this task was done by iint_init_always()).

Keep the non-NULL checks after calling evm_iint_inode() except in evm_inode_alloc_security(), to take into account inodes for which security_inode_alloc() was not called. When using shared metadata, obtaining a NULL pointer from integrity_iint_find() meant that the file wasn't in the IMA policy. Now, because IMA and EVM use disjoint metadata, the EVM status has to be stored for every inode regardless of the IMA policy.

Given that from now on EVM relies on its own metadata, remove the iint parameter from evm_verifyxattr(). Also, directly retrieve the iint in evm_verify_hmac(), called by both evm_verifyxattr() and evm_verify_current_integrity(), since now there is no performance penalty in retrieving EVM metadata (constant time).

Replicate the management of the IMA_NEW_FILE flag, by introducing evm_post_path_mknod() and evm_file_release() to respectively set and clear the newly introduced flag EVM_NEW_FILE, at the same time IMA does. Like for IMA, select CONFIG_SECURITY_PATH when EVM is enabled, to ensure that files are marked as new.

Unlike ima_post_path_mknod(), evm_post_path_mknod() cannot check if a file must be appraised. Thus, it marks all affected files. Also, it does not clear EVM_NEW_FILE depending on i_version, but that is not a problem because IMA_NEW_FILE is always cleared when set in ima_check_last_writer().

Move the EVM-specific flag EVM_IMMUTABLE_DIGSIG to security/integrity/evm/evm.h, since that definition is now unnecessary in the common integrity layer.

Finally, switch to the LSM reservation mechanism for the EVM xattr, and consequently decrement by one the number of xattrs to allocate in security_inode_init_security().

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com

Reviewed-by: Casey Schaufler casey@schaufler-ca.com

---

include/linux/evm.h | 8 +-- security/integrity/evm/Kconfig | 1 + security/integrity/evm/evm.h | 19 +++++++ security/integrity/evm/evm_crypto.c | 4 +- security/integrity/evm/evm_main.c | 76 ++++++++++++++++++++------- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/integrity.h | 1 - security/security.c | 4 +- 8 files changed, 83 insertions(+), 32 deletions(-)

diff --git a/include/linux/evm.h b/include/linux/evm.h index cb481eccc967..d48d6da32315 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -12,15 +12,12 @@ #include <linux/integrity.h> #include <linux/xattr.h> -struct integrity_iint_cache;

#ifdef CONFIG_EVM extern int evm_set_key(void *key, size_t keylen); extern enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, void *xattr_value,

• 			     size_t xattr_value_len,
  

• 			     struct integrity_iint_cache *iint);
  

• 			     size_t xattr_value_len);
  

int evm_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, struct xattr *xattrs, int *xattr_count); @@ -48,8 +45,7 @@ static inline int evm_set_key(void *key, size_t keylen) static inline enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, void *xattr_value,

• 				    size_t xattr_value_len,
  

• 			struct integrity_iint_cache *iint)
  

• 				    size_t xattr_value_len)
  

{ return INTEGRITY_UNKNOWN; } diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index fba9ee359bc9..861b3bacab82 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -6,6 +6,7 @@ config EVM select CRYPTO_HMAC select CRYPTO_SHA1 select CRYPTO_HASH_INFO

• select SECURITY_PATH default n help EVM protects a file's security extended attributes against

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 53bd7fec93fa..eb1a2c343bd7 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -32,6 +32,25 @@ struct xattr_list { bool enabled; }; +#define EVM_NEW_FILE 0x00000001 +#define EVM_IMMUTABLE_DIGSIG 0x00000002

+/* EVM integrity metadata associated with an inode */ +struct evm_iint_cache {

• unsigned long flags;
• enum integrity_status evm_status:4;

+};

+extern struct lsm_blob_sizes evm_blob_sizes;

+static inline struct evm_iint_cache *evm_iint_inode(const struct inode *inode) +{

• if (unlikely(!inode->i_security))

• return NULL;
  

• return inode->i_security + evm_blob_sizes.lbs_inode;

+}

extern int evm_initialized; #define EVM_ATTR_FSUUID 0x0001 diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index b1ffd4cc0b44..7552d49d0725 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -322,10 +322,10 @@ int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, static int evm_is_immutable(struct dentry *dentry, struct inode *inode) { const struct evm_ima_xattr_data *xattr_data = NULL;

• struct integrity_iint_cache *iint;

• struct evm_iint_cache *iint; int rc = 0;

• iint = integrity_iint_find(inode);

• iint = evm_iint_inode(inode); if (iint && (iint->flags & EVM_IMMUTABLE_DIGSIG)) return 1;

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index f65dbf3e9256..14c8f38e61d6 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -178,14 +178,14 @@ static int is_unsupported_fs(struct dentry *dentry) static enum integrity_status evm_verify_hmac(struct dentry *dentry, const char *xattr_name, char *xattr_value,

• 			     size_t xattr_value_len,
  

• 			     struct integrity_iint_cache *iint)
  

• 			     size_t xattr_value_len)
  

{ struct evm_ima_xattr_data *xattr_data = NULL; struct signature_v2_hdr *hdr; enum integrity_status evm_status = INTEGRITY_PASS; struct evm_digest digest;

• struct inode *inode;

• struct inode *inode = d_backing_inode(dentry);
• struct evm_iint_cache *iint = evm_iint_inode(inode); int rc, xattr_len, evm_immutable = 0;

if (iint && (iint->evm_status == INTEGRITY_PASS || @@ -254,8 +254,6 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, (const char *)xattr_data, xattr_len, digest.digest, digest.hdr.length); if (!rc) {

• 	inode = d_backing_inode(dentry);
  

• if (xattr_data->type == EVM_XATTR_PORTABLE_DIGSIG) {
  	if (iint)
  		iint->flags |= EVM_IMMUTABLE_DIGSIG;
  

@@ -403,7 +401,6 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,

• @xattr_name: requested xattr
• @xattr_value: requested xattr value
• @xattr_value_len: requested xattr value length

• • @iint: inode integrity metadata
  • Calculate the HMAC for the given dentry and verify it against the stored
  • security.evm xattr. For performance, use the xattr value and length

@@ -416,8 +413,7 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, */ enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name,

• 		      void *xattr_value, size_t xattr_value_len,
  

• 		      struct integrity_iint_cache *iint)
  

• 		      void *xattr_value, size_t xattr_value_len)
  

{ if (!evm_key_loaded() || !evm_protected_xattr(xattr_name)) return INTEGRITY_UNKNOWN; @@ -425,13 +421,8 @@ enum integrity_status evm_verifyxattr(struct dentry *dentry, if (is_unsupported_fs(dentry)) return INTEGRITY_UNKNOWN;

• if (!iint) {

• iint = integrity_iint_find(d_backing_inode(dentry));
  

• if (!iint)
  

• 	return INTEGRITY_UNKNOWN;
  

• } return evm_verify_hmac(dentry, xattr_name, xattr_value,

• 		 xattr_value_len, iint);
  

• 		 xattr_value_len);
  

} EXPORT_SYMBOL_GPL(evm_verifyxattr); @@ -448,7 +439,7 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry) if (!evm_key_loaded() || !S_ISREG(inode->i_mode) || evm_fixmode) return INTEGRITY_PASS;

• return evm_verify_hmac(dentry, NULL, NULL, 0, NULL);

• return evm_verify_hmac(dentry, NULL, NULL, 0);

} /* @@ -526,14 +517,14 @@ static int evm_protect_xattr(struct mnt_idmap *idmap, evm_status = evm_verify_current_integrity(dentry); if (evm_status == INTEGRITY_NOXATTRS) {

• struct integrity_iint_cache *iint;
  

• struct evm_iint_cache *iint;
  

/* Exception if the HMAC is not going to be calculated. */ if (evm_hmac_disabled()) return 0;

• iint = integrity_iint_find(d_backing_inode(dentry));
  

• if (iint && (iint->flags & IMA_NEW_FILE))
  

• iint = evm_iint_inode(d_backing_inode(dentry));
  

• if (iint && (iint->flags & EVM_NEW_FILE))
  return 0;
  

/* exception for pseudo filesystems */ @@ -735,9 +726,9 @@ static int evm_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry, static void evm_reset_status(struct inode *inode) {

• struct integrity_iint_cache *iint;

• struct evm_iint_cache *iint;

• iint = integrity_iint_find(inode);

• iint = evm_iint_inode(inode); if (iint) iint->evm_status = INTEGRITY_UNKNOWN;

} @@ -1019,6 +1010,42 @@ int evm_inode_init_security(struct inode *inode, struct inode *dir, } EXPORT_SYMBOL_GPL(evm_inode_init_security); +static int evm_inode_alloc_security(struct inode *inode) +{

• struct evm_iint_cache *iint = evm_iint_inode(inode);
• /* Called by security_inode_alloc(), it cannot be NULL. */
• iint->flags = 0UL;
• iint->evm_status = INTEGRITY_UNKNOWN;
• return 0;

+}

+static void evm_file_release(struct file *file) +{

• struct inode *inode = file_inode(file);
• struct evm_iint_cache *iint = evm_iint_inode(inode);
• fmode_t mode = file->f_mode;
• if (!S_ISREG(inode->i_mode) || !(mode & FMODE_WRITE))

• return;
  

• if (iint && atomic_read(&inode->i_writecount) == 1)

• iint->flags &= ~EVM_NEW_FILE;
  

+}

+static void evm_post_path_mknod(struct mnt_idmap *idmap, struct dentry *dentry) +{

• struct inode *inode = d_backing_inode(dentry);
• struct evm_iint_cache *iint = evm_iint_inode(inode);
• if (!S_ISREG(inode->i_mode))

• return;
  

• if (iint)

• iint->flags |= EVM_NEW_FILE;
  

+}

#ifdef CONFIG_EVM_LOAD_X509 void __init evm_load_x509(void) { @@ -1071,6 +1098,9 @@ static struct security_hook_list evm_hooks[] __ro_after_init = { LSM_HOOK_INIT(inode_removexattr, evm_inode_removexattr), LSM_HOOK_INIT(inode_post_removexattr, evm_inode_post_removexattr), LSM_HOOK_INIT(inode_init_security, evm_inode_init_security),

• LSM_HOOK_INIT(inode_alloc_security, evm_inode_alloc_security),
• LSM_HOOK_INIT(file_release, evm_file_release),
• LSM_HOOK_INIT(path_post_mknod, evm_post_path_mknod),

}; static const struct lsm_id evm_lsmid = { @@ -1084,10 +1114,16 @@ static int __init init_evm_lsm(void) return 0; } +struct lsm_blob_sizes evm_blob_sizes __ro_after_init = {

• .lbs_inode = sizeof(struct evm_iint_cache),
• .lbs_xattr_count = 1,

+};

DEFINE_LSM(evm) = { .name = "evm", .init = init_evm_lsm, .order = LSM_ORDER_LAST,

• .blobs = &evm_blob_sizes,

}; late_initcall(init_evm); diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 076451109637..1dd6ee72a20a 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -520,7 +520,7 @@ int ima_appraise_measurement(enum ima_hooks func, } status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value,

• 		 rc < 0 ? 0 : rc, iint);
  

• 		 rc < 0 ? 0 : rc);
  

  switch (status) { case INTEGRITY_PASS: case INTEGRITY_PASS_IMMUTABLE:

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 59eaddd84434..7a97c269a072 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -37,7 +37,6 @@ #define IMA_DIGSIG_REQUIRED 0x01000000 #define IMA_PERMIT_DIRECTIO 0x02000000 #define IMA_NEW_FILE 0x04000000 -#define EVM_IMMUTABLE_DIGSIG 0x08000000 #define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000 #define IMA_MODSIG_ALLOWED 0x20000000 #define IMA_CHECK_BLACKLIST 0x40000000 diff --git a/security/security.c b/security/security.c index a44740640a9a..f811cc376a7a 100644 --- a/security/security.c +++ b/security/security.c @@ -1716,8 +1716,8 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, return 0; if (initxattrs) {

• /* Allocate +1 for EVM and +1 as terminator. */
  

• new_xattrs = kcalloc(blob_sizes.lbs_xattr_count + 2,
  

• /* Allocate +1 as terminator. */
  

• new_xattrs = kcalloc(blob_sizes.lbs_xattr_count + 1,
  	     sizeof(*new_xattrs), GFP_NOFS);
  

  if (!new_xattrs) return -ENOMEM;