From 3253804773c0613a1bad5bfea2edf172b760d8b6 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:05 -0500 Subject: security: allow finer granularity in permitting copy-up of security xattrs Copying up xattrs is solely based on the security xattr name. For finer granularity add a dentry parameter to the security_inode_copy_up_xattr hook definition, allowing decisions to be based on the xattr content as well. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Acked-by: Amir Goldstein Acked-by: Paul Moore (LSM,SELinux) Signed-off-by: Mimi Zohar --- include/linux/lsm_hook_defs.h | 3 ++- include/linux/security.h | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 334e00efbde4..f804b76cde44 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -176,7 +176,8 @@ LSM_HOOK(int, 0, inode_listsecurity, struct inode *inode, char *buffer, size_t buffer_size) LSM_HOOK(void, LSM_RET_VOID, inode_getsecid, struct inode *inode, u32 *secid) LSM_HOOK(int, 0, inode_copy_up, struct dentry *src, struct cred **new) -LSM_HOOK(int, -EOPNOTSUPP, inode_copy_up_xattr, const char *name) +LSM_HOOK(int, -EOPNOTSUPP, inode_copy_up_xattr, struct dentry *src, + const char *name) LSM_HOOK(int, 0, kernfs_init_security, struct kernfs_node *kn_dir, struct kernfs_node *kn) LSM_HOOK(int, 0, file_permission, struct file *file, int mask) diff --git a/include/linux/security.h b/include/linux/security.h index 41a8f667bdfa..21cf70346b33 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -398,7 +398,7 @@ int security_inode_setsecurity(struct inode *inode, const char *name, const void int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size); void security_inode_getsecid(struct inode *inode, u32 *secid); int security_inode_copy_up(struct dentry *src, struct cred **new); -int security_inode_copy_up_xattr(const char *name); +int security_inode_copy_up_xattr(struct dentry *src, const char *name); int security_kernfs_init_security(struct kernfs_node *kn_dir, struct kernfs_node *kn); int security_file_permission(struct file *file, int mask); @@ -1016,7 +1016,7 @@ static inline int security_kernfs_init_security(struct kernfs_node *kn_dir, return 0; } -static inline int security_inode_copy_up_xattr(const char *name) +static inline int security_inode_copy_up_xattr(struct dentry *src, const char *name) { return -EOPNOTSUPP; } -- cgit v1.2.3-70-g09d2 From 309e2b775da8b2c28fccc4ac2621801f06920ce0 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:08 -0500 Subject: ima: Move file-change detection variables into new structure Move all the variables used for file change detection into a structure that can be used by IMA and EVM. Implement an inline function for storing the identification of an inode and one for detecting changes to an inode based on this new structure. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Signed-off-by: Mimi Zohar --- include/linux/integrity.h | 34 ++++++++++++++++++++++++++++++++++ security/integrity/ima/ima.h | 4 +--- security/integrity/ima/ima_api.c | 10 +++++----- security/integrity/ima/ima_iint.c | 2 +- security/integrity/ima/ima_main.c | 7 +++---- 5 files changed, 44 insertions(+), 13 deletions(-) (limited to 'include') diff --git a/include/linux/integrity.h b/include/linux/integrity.h index 459b79683783..f5842372359b 100644 --- a/include/linux/integrity.h +++ b/include/linux/integrity.h @@ -8,6 +8,7 @@ #define _LINUX_INTEGRITY_H #include +#include enum integrity_status { INTEGRITY_PASS = 0, @@ -28,4 +29,37 @@ static inline void integrity_load_keys(void) } #endif /* CONFIG_INTEGRITY */ +/* An inode's attributes for detection of changes */ +struct integrity_inode_attributes { + u64 version; /* track inode changes */ + unsigned long ino; + dev_t dev; +}; + +/* + * On stacked filesystems the i_version alone is not enough to detect file data + * or metadata change. Additional metadata is required. + */ +static inline void +integrity_inode_attrs_store(struct integrity_inode_attributes *attrs, + u64 i_version, const struct inode *inode) +{ + attrs->version = i_version; + attrs->dev = inode->i_sb->s_dev; + attrs->ino = inode->i_ino; +} + +/* + * On stacked filesystems detect whether the inode or its content has changed. + */ +static inline bool +integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs, + const struct inode *inode) +{ + return (inode->i_sb->s_dev != attrs->dev || + inode->i_ino != attrs->ino || + !inode_eq_iversion(inode, attrs->version)); +} + + #endif /* _LINUX_INTEGRITY_H */ diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 11d7c0332207..9151b5369cdc 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -175,12 +175,10 @@ struct ima_kexec_hdr { /* IMA integrity metadata associated with an inode */ struct ima_iint_cache { struct mutex mutex; /* protects: version, flags, digest */ - u64 version; /* track inode changes */ + struct integrity_inode_attributes real_inode; unsigned long flags; unsigned long measured_pcrs; unsigned long atomic_flags; - unsigned long real_ino; - dev_t real_dev; enum integrity_status ima_file_status:4; enum integrity_status ima_mmap_status:4; enum integrity_status ima_bprm_status:4; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 3d286de231e1..984e861f6e33 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -305,11 +305,11 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file, iint->ima_hash = tmpbuf; memcpy(iint->ima_hash, &hash, length); - iint->version = i_version; - if (real_inode != inode) { - iint->real_ino = real_inode->i_ino; - iint->real_dev = real_inode->i_sb->s_dev; - } + if (real_inode == inode) + iint->real_inode.version = i_version; + else + integrity_inode_attrs_store(&iint->real_inode, i_version, + real_inode); /* Possibly temporary failure due to type of read (eg. O_DIRECT) */ if (!result) diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c index e7c9c216c1c6..e23412a2c56b 100644 --- a/security/integrity/ima/ima_iint.c +++ b/security/integrity/ima/ima_iint.c @@ -59,7 +59,7 @@ static void ima_iint_init_always(struct ima_iint_cache *iint, struct inode *inode) { iint->ima_hash = NULL; - iint->version = 0; + iint->real_inode.version = 0; iint->flags = 0UL; iint->atomic_flags = 0UL; iint->ima_file_status = INTEGRITY_UNKNOWN; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index eebf629f192e..4b215d85c14b 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -173,7 +173,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint, STATX_CHANGE_COOKIE, AT_STATX_SYNC_AS_STAT) || !(stat.result_mask & STATX_CHANGE_COOKIE) || - stat.change_cookie != iint->version) { + stat.change_cookie != iint->real_inode.version) { iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE); iint->measured_pcrs = 0; if (update) @@ -292,9 +292,8 @@ static int process_measurement(struct file *file, const struct cred *cred, if (real_inode != inode && (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) { if (!IS_I_VERSION(real_inode) || - real_inode->i_sb->s_dev != iint->real_dev || - real_inode->i_ino != iint->real_ino || - !inode_eq_iversion(real_inode, iint->version)) { + integrity_inode_attrs_changed(&iint->real_inode, + real_inode)) { iint->flags &= ~IMA_DONE_MASK; iint->measured_pcrs = 0; } -- cgit v1.2.3-70-g09d2 From a652aa59068bd78d96a1ab6ea7c0c4d3c79fb5e8 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:09 -0500 Subject: evm: Store and detect metadata inode attributes changes On stacked filesystem the metadata inode may be different than the one file data inode and therefore changes to it need to be detected independently. Therefore, store the i_version, device number, and inode number associated with the file metadata inode. Implement a function to detect changes to the inode and if a change is detected reset the evm_status. This function will be called by IMA when IMA detects that the metadata inode is different from the file's inode. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Signed-off-by: Mimi Zohar --- include/linux/evm.h | 8 ++++++++ security/integrity/evm/evm.h | 6 ++++-- security/integrity/evm/evm_crypto.c | 23 +++++++++++++++++------ security/integrity/evm/evm_main.c | 30 ++++++++++++++++++++++++++++-- 4 files changed, 57 insertions(+), 10 deletions(-) (limited to 'include') diff --git a/include/linux/evm.h b/include/linux/evm.h index d48d6da32315..ddece4a6b25d 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -26,6 +26,8 @@ extern int evm_protected_xattr_if_enabled(const char *req_xattr_name); extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, int buffer_size, char type, bool canonical_fmt); +extern bool evm_metadata_changed(struct inode *inode, + struct inode *metadata_inode); #ifdef CONFIG_FS_POSIX_ACL extern int posix_xattr_acl(const char *xattrname); #else @@ -76,5 +78,11 @@ static inline int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, return -EOPNOTSUPP; } +static inline bool evm_metadata_changed(struct inode *inode, + struct inode *metadata_inode) +{ + return false; +} + #endif /* CONFIG_EVM */ #endif /* LINUX_EVM_H */ diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 72e3341ae6f7..51aba5a54275 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -39,6 +39,7 @@ struct xattr_list { struct evm_iint_cache { unsigned long flags; enum integrity_status evm_status:4; + struct integrity_inode_attributes metadata_inode; }; extern struct lsm_blob_sizes evm_blob_sizes; @@ -74,11 +75,12 @@ int evm_update_evmxattr(struct dentry *dentry, size_t req_xattr_value_len); int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, const char *req_xattr_value, - size_t req_xattr_value_len, struct evm_digest *data); + size_t req_xattr_value_len, struct evm_digest *data, + struct evm_iint_cache *iint); int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, const char *req_xattr_value, size_t req_xattr_value_len, char type, - struct evm_digest *data); + struct evm_digest *data, struct evm_iint_cache *iint); int evm_init_hmac(struct inode *inode, const struct xattr *xattrs, char *hmac_val); int evm_init_secfs(void); diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 35416f55391c..7c06ffd633d2 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -221,7 +221,8 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, const char *req_xattr_name, const char *req_xattr_value, size_t req_xattr_value_len, - uint8_t type, struct evm_digest *data) + uint8_t type, struct evm_digest *data, + struct evm_iint_cache *iint) { struct inode *inode = d_inode(d_real(dentry, D_REAL_METADATA)); struct xattr_list *xattr; @@ -231,6 +232,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, int error; int size, user_space_size; bool ima_present = false; + u64 i_version = 0; if (!(inode->i_opflags & IOP_XATTR) || inode->i_sb->s_user_ns != &init_user_ns) @@ -294,6 +296,13 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, } hmac_add_misc(desc, inode, type, data->digest); + if (inode != d_backing_inode(dentry) && iint) { + if (IS_I_VERSION(inode)) + i_version = inode_query_iversion(inode); + integrity_inode_attrs_store(&iint->metadata_inode, i_version, + inode); + } + /* Portable EVM signatures must include an IMA hash */ if (type == EVM_XATTR_PORTABLE_DIGSIG && !ima_present) error = -EPERM; @@ -305,18 +314,19 @@ out: int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, const char *req_xattr_value, size_t req_xattr_value_len, - struct evm_digest *data) + struct evm_digest *data, struct evm_iint_cache *iint) { return evm_calc_hmac_or_hash(dentry, req_xattr_name, req_xattr_value, - req_xattr_value_len, EVM_XATTR_HMAC, data); + req_xattr_value_len, EVM_XATTR_HMAC, data, + iint); } int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, const char *req_xattr_value, size_t req_xattr_value_len, - char type, struct evm_digest *data) + char type, struct evm_digest *data, struct evm_iint_cache *iint) { return evm_calc_hmac_or_hash(dentry, req_xattr_name, req_xattr_value, - req_xattr_value_len, type, data); + req_xattr_value_len, type, data, iint); } static int evm_is_immutable(struct dentry *dentry, struct inode *inode) @@ -357,6 +367,7 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name, const char *xattr_value, size_t xattr_value_len) { struct inode *inode = d_backing_inode(dentry); + struct evm_iint_cache *iint = evm_iint_inode(inode); struct evm_digest data; int rc = 0; @@ -372,7 +383,7 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name, data.hdr.algo = HASH_ALGO_SHA1; rc = evm_calc_hmac(dentry, xattr_name, xattr_value, - xattr_value_len, &data); + xattr_value_len, &data, iint); if (rc == 0) { data.hdr.xattr.sha1.type = EVM_XATTR_HMAC; rc = __vfs_setxattr_noperm(&nop_mnt_idmap, dentry, diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index c658d2f1494b..c1ca0894cd8a 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -226,7 +226,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, digest.hdr.algo = HASH_ALGO_SHA1; rc = evm_calc_hmac(dentry, xattr_name, xattr_value, - xattr_value_len, &digest); + xattr_value_len, &digest, iint); if (rc) break; rc = crypto_memneq(xattr_data->data, digest.digest, @@ -247,7 +247,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, hdr = (struct signature_v2_hdr *)xattr_data; digest.hdr.algo = hdr->hash_algo; rc = evm_calc_hash(dentry, xattr_name, xattr_value, - xattr_value_len, xattr_data->type, &digest); + xattr_value_len, xattr_data->type, &digest, + iint); if (rc) break; rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM, @@ -733,6 +734,31 @@ static void evm_reset_status(struct inode *inode) iint->evm_status = INTEGRITY_UNKNOWN; } +/** + * evm_metadata_changed: Detect changes to the metadata + * @inode: a file's inode + * @metadata_inode: metadata inode + * + * On a stacked filesystem detect whether the metadata has changed. If this is + * the case reset the evm_status associated with the inode that represents the + * file. + */ +bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode) +{ + struct evm_iint_cache *iint = evm_iint_inode(inode); + bool ret = false; + + if (iint) { + ret = (!IS_I_VERSION(metadata_inode) || + integrity_inode_attrs_changed(&iint->metadata_inode, + metadata_inode)); + if (ret) + iint->evm_status = INTEGRITY_UNKNOWN; + } + + return ret; +} + /** * evm_revalidate_status - report whether EVM status re-validation is necessary * @xattr_name: pointer to the affected extended attribute name -- cgit v1.2.3-70-g09d2 From 1f65e57dc5417b166843438bef31c70b9a5208fe Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:12 -0500 Subject: fs: Rename SB_I_EVM_UNSUPPORTED to SB_I_EVM_HMAC_UNSUPPORTED Now that EVM supports RSA signatures for previously completely unsupported filesystems rename the flag SB_I_EVM_UNSUPPORTED to SB_I_EVM_HMAC_UNSUPPORTED to reflect that only HMAC is not supported. Suggested-by: Amir Goldstein Suggested-by: Mimi Zohar Signed-off-by: Stefan Berger Acked-by: Amir Goldstein Signed-off-by: Mimi Zohar --- fs/overlayfs/super.c | 2 +- include/linux/fs.h | 2 +- security/integrity/evm/evm_main.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index a40fc7e05525..06a231970cb5 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1460,7 +1460,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc) * lead to unexpected results. */ sb->s_iflags |= SB_I_NOUMASK; - sb->s_iflags |= SB_I_EVM_UNSUPPORTED; + sb->s_iflags |= SB_I_EVM_HMAC_UNSUPPORTED; err = -ENOMEM; root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe); diff --git a/include/linux/fs.h b/include/linux/fs.h index 8dfd53b52744..7bbd5e2d2a20 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1177,7 +1177,7 @@ extern int send_sigurg(struct fown_struct *fown); #define SB_I_USERNS_VISIBLE 0x00000010 /* fstype already mounted */ #define SB_I_IMA_UNVERIFIABLE_SIGNATURE 0x00000020 #define SB_I_UNTRUSTED_MOUNTER 0x00000040 -#define SB_I_EVM_UNSUPPORTED 0x00000080 +#define SB_I_EVM_HMAC_UNSUPPORTED 0x00000080 #define SB_I_SKIP_SYNC 0x00000100 /* Skip superblock at global sync */ #define SB_I_PERSB_BDI 0x00000200 /* has a per-sb bdi */ diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index cfb4f9809369..c4a6081ce596 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -155,7 +155,7 @@ static int is_unsupported_fs(struct dentry *dentry) { struct inode *inode = d_backing_inode(dentry); - if (inode->i_sb->s_iflags & SB_I_EVM_UNSUPPORTED) { + if (inode->i_sb->s_iflags & SB_I_EVM_HMAC_UNSUPPORTED) { pr_info_once("%s not supported\n", inode->i_sb->s_type->name); return 1; } -- cgit v1.2.3-70-g09d2