From be84f32bb2c981ca670922e047cdde1488b233de Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 22 Mar 2024 10:03:12 -0400 Subject: ima: Fix use-after-free on a dentry's dname.name ->d_name.name can change on rename and the earlier value can be freed; there are conditions sufficient to stabilize it (->d_lock on dentry, ->d_lock on its parent, ->i_rwsem exclusive on the parent's inode, rename_lock), but none of those are met at any of the sites. Take a stable snapshot of the name instead. Link: https://lore.kernel.org/all/20240202182732.GE2087318@ZenIV/ Signed-off-by: Al Viro Signed-off-by: Stefan Berger Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 16 ++++++++++++---- security/integrity/ima/ima_template_lib.c | 17 ++++++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index b37d043d5748..1856981e33df 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -245,8 +245,8 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file, const char *audit_cause = "failed"; struct inode *inode = file_inode(file); struct inode *real_inode = d_real_inode(file_dentry(file)); - const char *filename = file->f_path.dentry->d_name.name; struct ima_max_digest_data hash; + struct name_snapshot filename; struct kstat stat; int result = 0; int length; @@ -317,9 +317,13 @@ out: if (file->f_flags & O_DIRECT) audit_cause = "failed(directio)"; + take_dentry_name_snapshot(&filename, file->f_path.dentry); + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, - filename, "collect_data", audit_cause, - result, 0); + filename.name.name, "collect_data", + audit_cause, result, 0); + + release_dentry_name_snapshot(&filename); } return result; } @@ -432,6 +436,7 @@ out: */ const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf) { + struct name_snapshot filename; char *pathname = NULL; *pathbuf = __getname(); @@ -445,7 +450,10 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *namebuf) } if (!pathname) { - strscpy(namebuf, path->dentry->d_name.name, NAME_MAX); + take_dentry_name_snapshot(&filename, path->dentry); + strscpy(namebuf, filename.name.name, NAME_MAX); + release_dentry_name_snapshot(&filename); + pathname = namebuf; } diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 6cd0add524cd..3b2cb8f1002e 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -483,7 +483,10 @@ static int ima_eventname_init_common(struct ima_event_data *event_data, bool size_limit) { const char *cur_filename = NULL; + struct name_snapshot filename; u32 cur_filename_len = 0; + bool snapshot = false; + int ret; BUG_ON(event_data->filename == NULL && event_data->file == NULL); @@ -496,7 +499,10 @@ static int ima_eventname_init_common(struct ima_event_data *event_data, } if (event_data->file) { - cur_filename = event_data->file->f_path.dentry->d_name.name; + take_dentry_name_snapshot(&filename, + event_data->file->f_path.dentry); + snapshot = true; + cur_filename = filename.name.name; cur_filename_len = strlen(cur_filename); } else /* @@ -505,8 +511,13 @@ static int ima_eventname_init_common(struct ima_event_data *event_data, */ cur_filename_len = IMA_EVENT_NAME_LEN_MAX; out: - return ima_write_template_field_data(cur_filename, cur_filename_len, - DATA_FMT_STRING, field_data); + ret = ima_write_template_field_data(cur_filename, cur_filename_len, + DATA_FMT_STRING, field_data); + + if (snapshot) + release_dentry_name_snapshot(&filename); + + return ret; } /* -- cgit v1.2.3-70-g09d2 From cc293c8466625bf8d238fc41d26db169a126e21b Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 22 Mar 2024 14:50:36 -0400 Subject: ima: define an init_module critical data record The init_module syscall loads an ELF image into kernel space without measuring the buffer containing the ELF image. To close this kernel module integrity gap, define a new critical-data record which includes the hash of the ELF image. Instead of including the buffer data in the IMA measurement list, include the hash of the buffer data to avoid large IMA measurement list records. The buffer data hash would be the same value as the finit_module syscall file hash. To enable measuring the init_module buffer and other critical data from boot, define "ima_policy=critical_data" on the boot command line. Since builtin policies are not persistent, a custom IMA policy must include the rule as well: measure func=CRITICAL_DATA label=modules To verify the template data hash value, first convert the buffer data hash to binary: grep "init_module" \ /sys/kernel/security/integrity/ima/ascii_runtime_measurements | \ tail -1 | cut -d' ' -f 6 | xxd -r -p | sha256sum Reported-by: Ken Goldman Reviewed-by: Jarkko Sakkinen Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index c84e8c55333d..4b4348d681a6 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -902,6 +902,13 @@ static int ima_post_load_data(char *buf, loff_t size, return 0; } + /* + * Measure the init_module syscall buffer containing the ELF image. + */ + if (load_id == LOADING_MODULE) + ima_measure_critical_data("modules", "init_module", + buf, size, true, NULL, 0); + return 0; } -- cgit v1.2.3-70-g09d2 From 38aa3f5ac6d2de6b471ecb6e1cd878957ae7e8de Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 4 Apr 2024 09:00:48 -0600 Subject: integrity: Avoid -Wflex-array-member-not-at-end warnings -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally. There is currently an object (`hdr)` in `struct ima_max_digest_data` that contains a flexible structure (`struct ima_digest_data`): struct ima_max_digest_data { struct ima_digest_data hdr; u8 digest[HASH_MAX_DIGESTSIZE]; } __packed; So, in order to avoid ending up with a flexible-array member in the middle of a struct, we use the `__struct_group()` helper to separate the flexible array from the rest of the members in the flexible structure: struct ima_digest_data { __struct_group(ima_digest_data_hdr, hdr, __packed, ... the rest of the members ); u8 digest[]; } __packed; And similarly for `struct evm_ima_xattr_data`. With the change described above, we can now declare an object of the type of the tagged `struct ima_digest_data_hdr`, without embedding the flexible array in the middle of another struct: struct ima_max_digest_data { struct ima_digest_data_hdr hdr; u8 digest[HASH_MAX_DIGESTSIZE]; } __packed; And similarly for `struct evm_digest` and `struct evm_xattr`. We also use `container_of()` whenever we need to retrieve a pointer to the flexible structure. So, with these changes, fix the following warnings: security/integrity/evm/evm.h:64:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/evm/../integrity.h:40:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/evm/../integrity.h:68:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/ima/../integrity.h:40:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/ima/../integrity.h:68:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/integrity.h:40:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/integrity.h:68:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/platform_certs/../integrity.h:40:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] security/integrity/platform_certs/../integrity.h:68:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] Link: https://github.com/KSPP/linux/issues/202 Signed-off-by: Gustavo A. R. Silva Signed-off-by: Mimi Zohar --- security/integrity/evm/evm.h | 2 +- security/integrity/ima/ima_api.c | 6 ++++-- security/integrity/ima/ima_appraise.c | 4 +++- security/integrity/ima/ima_init.c | 6 ++++-- security/integrity/ima/ima_main.c | 6 ++++-- security/integrity/ima/ima_template_lib.c | 10 ++++++---- security/integrity/integrity.h | 12 +++++++++--- 7 files changed, 31 insertions(+), 15 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index eb1a2c343bd7..72e3341ae6f7 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -61,7 +61,7 @@ extern int evm_hmac_attrs; extern struct list_head evm_config_xattrnames; struct evm_digest { - struct ima_digest_data hdr; + struct ima_digest_data_hdr hdr; char digest[IMA_MAX_DIGEST_SIZE]; } __packed; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 1856981e33df..3d286de231e1 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -246,6 +246,8 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file, struct inode *inode = file_inode(file); struct inode *real_inode = d_real_inode(file_dentry(file)); struct ima_max_digest_data hash; + struct ima_digest_data *hash_hdr = container_of(&hash.hdr, + struct ima_digest_data, hdr); struct name_snapshot filename; struct kstat stat; int result = 0; @@ -286,9 +288,9 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file, result = -ENODATA; } } else if (buf) { - result = ima_calc_buffer_hash(buf, size, &hash.hdr); + result = ima_calc_buffer_hash(buf, size, hash_hdr); } else { - result = ima_calc_file_hash(file, &hash.hdr); + result = ima_calc_file_hash(file, hash_hdr); } if (result && result != -EBADF && result != -EINVAL) diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 3497741caea9..656c709b974f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -378,7 +378,9 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, } rc = calc_file_id_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo, - iint->ima_hash->digest, &hash.hdr); + iint->ima_hash->digest, + container_of(&hash.hdr, + struct ima_digest_data, hdr)); if (rc) { *cause = "sigv3-hashing-error"; *status = INTEGRITY_FAIL; diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 393f5c7912d5..4e208239a40e 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -48,12 +48,14 @@ static int __init ima_add_boot_aggregate(void) struct ima_event_data event_data = { .iint = iint, .filename = boot_aggregate_name }; struct ima_max_digest_data hash; + struct ima_digest_data *hash_hdr = container_of(&hash.hdr, + struct ima_digest_data, hdr); int result = -ENOMEM; int violation = 0; memset(iint, 0, sizeof(*iint)); memset(&hash, 0, sizeof(hash)); - iint->ima_hash = &hash.hdr; + iint->ima_hash = hash_hdr; iint->ima_hash->algo = ima_hash_algo; iint->ima_hash->length = hash_digest_size[ima_hash_algo]; @@ -70,7 +72,7 @@ static int __init ima_add_boot_aggregate(void) * is not found. */ if (ima_tpm_chip) { - result = ima_calc_boot_aggregate(&hash.hdr); + result = ima_calc_boot_aggregate(hash_hdr); if (result < 0) { audit_cause = "hashing_error"; goto err_out; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 4b4348d681a6..fff155f230a5 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -948,6 +948,8 @@ int process_buffer_measurement(struct mnt_idmap *idmap, .buf_len = size}; struct ima_template_desc *template; struct ima_max_digest_data hash; + struct ima_digest_data *hash_hdr = container_of(&hash.hdr, + struct ima_digest_data, hdr); char digest_hash[IMA_MAX_DIGEST_SIZE]; int digest_hash_len = hash_digest_size[ima_hash_algo]; int violation = 0; @@ -986,7 +988,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap, if (!pcr) pcr = CONFIG_IMA_MEASURE_PCR_IDX; - iint.ima_hash = &hash.hdr; + iint.ima_hash = hash_hdr; iint.ima_hash->algo = ima_hash_algo; iint.ima_hash->length = hash_digest_size[ima_hash_algo]; @@ -997,7 +999,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap, } if (buf_hash) { - memcpy(digest_hash, hash.hdr.digest, digest_hash_len); + memcpy(digest_hash, hash_hdr->digest, digest_hash_len); ret = ima_calc_buffer_hash(digest_hash, digest_hash_len, iint.ima_hash); diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 3b2cb8f1002e..4183956c53af 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -339,6 +339,8 @@ int ima_eventdigest_init(struct ima_event_data *event_data, struct ima_field_data *field_data) { struct ima_max_digest_data hash; + struct ima_digest_data *hash_hdr = container_of(&hash.hdr, + struct ima_digest_data, hdr); u8 *cur_digest = NULL; u32 cur_digestsize = 0; struct inode *inode; @@ -358,7 +360,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data, if ((const char *)event_data->filename == boot_aggregate_name) { if (ima_tpm_chip) { hash.hdr.algo = HASH_ALGO_SHA1; - result = ima_calc_boot_aggregate(&hash.hdr); + result = ima_calc_boot_aggregate(hash_hdr); /* algo can change depending on available PCR banks */ if (!result && hash.hdr.algo != HASH_ALGO_SHA1) @@ -368,7 +370,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data, memset(&hash, 0, sizeof(hash)); } - cur_digest = hash.hdr.digest; + cur_digest = hash_hdr->digest; cur_digestsize = hash_digest_size[HASH_ALGO_SHA1]; goto out; } @@ -379,14 +381,14 @@ int ima_eventdigest_init(struct ima_event_data *event_data, inode = file_inode(event_data->file); hash.hdr.algo = ima_template_hash_algo_allowed(ima_hash_algo) ? ima_hash_algo : HASH_ALGO_SHA1; - result = ima_calc_file_hash(event_data->file, &hash.hdr); + result = ima_calc_file_hash(event_data->file, hash_hdr); if (result) { integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, event_data->filename, "collect_data", "failed", result, 0); return result; } - cur_digest = hash.hdr.digest; + cur_digest = hash_hdr->digest; cur_digestsize = hash.hdr.length; out: return ima_eventdigest_init_common(cur_digest, cur_digestsize, diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 50d6f798e613..660f76cb69d3 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -31,19 +31,24 @@ enum evm_ima_xattr_type { }; struct evm_ima_xattr_data { - u8 type; + /* New members must be added within the __struct_group() macro below. */ + __struct_group(evm_ima_xattr_data_hdr, hdr, __packed, + u8 type; + ); u8 data[]; } __packed; /* Only used in the EVM HMAC code. */ struct evm_xattr { - struct evm_ima_xattr_data data; + struct evm_ima_xattr_data_hdr data; u8 digest[SHA1_DIGEST_SIZE]; } __packed; #define IMA_MAX_DIGEST_SIZE HASH_MAX_DIGESTSIZE struct ima_digest_data { + /* New members must be added within the __struct_group() macro below. */ + __struct_group(ima_digest_data_hdr, hdr, __packed, u8 algo; u8 length; union { @@ -57,6 +62,7 @@ struct ima_digest_data { } ng; u8 data[2]; } xattr; + ); u8 digest[]; } __packed; @@ -65,7 +71,7 @@ struct ima_digest_data { * with the maximum hash size, define ima_max_digest_data struct. */ struct ima_max_digest_data { - struct ima_digest_data hdr; + struct ima_digest_data_hdr hdr; u8 digest[HASH_MAX_DIGESTSIZE]; } __packed; -- cgit v1.2.3-70-g09d2 From c21632b66895eb23c05e4eeedb68128fb243d168 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:04 -0500 Subject: ima: Rename backing_inode to real_inode Rename the backing_inode variable to real_inode since it gets its value from real_inode(). Suggested-by: Amir Goldstein Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Acked-by: Amir Goldstein Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index fff155f230a5..eebf629f192e 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -208,7 +208,7 @@ static int process_measurement(struct file *file, const struct cred *cred, u32 secid, char *buf, loff_t size, int mask, enum ima_hooks func) { - struct inode *backing_inode, *inode = file_inode(file); + struct inode *real_inode, *inode = file_inode(file); struct ima_iint_cache *iint = NULL; struct ima_template_desc *template_desc = NULL; char *pathbuf = NULL; @@ -285,14 +285,16 @@ static int process_measurement(struct file *file, const struct cred *cred, iint->measured_pcrs = 0; } - /* Detect and re-evaluate changes made to the backing file. */ - backing_inode = d_real_inode(file_dentry(file)); - if (backing_inode != inode && + /* + * On stacked filesystems, detect and re-evaluate file data changes. + */ + real_inode = d_real_inode(file_dentry(file)); + if (real_inode != inode && (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) { - if (!IS_I_VERSION(backing_inode) || - backing_inode->i_sb->s_dev != iint->real_dev || - backing_inode->i_ino != iint->real_ino || - !inode_eq_iversion(backing_inode, iint->version)) { + 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)) { iint->flags &= ~IMA_DONE_MASK; iint->measured_pcrs = 0; } -- cgit v1.2.3-70-g09d2 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 --- fs/overlayfs/copy_up.c | 2 +- include/linux/lsm_hook_defs.h | 3 ++- include/linux/security.h | 4 ++-- security/integrity/evm/evm_main.c | 2 +- security/security.c | 5 +++-- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 2 +- 7 files changed, 11 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 0762575a1e70..a5ef2005a2cc 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -114,7 +114,7 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *oldpath, struct de if (ovl_is_private_xattr(sb, name)) continue; - error = security_inode_copy_up_xattr(name); + error = security_inode_copy_up_xattr(old, name); if (error < 0 && error != -EOPNOTSUPP) break; if (error == 1) { 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; } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 81dbade5b9b3..b0a862bfd74a 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -946,7 +946,7 @@ static void evm_inode_post_setattr(struct mnt_idmap *idmap, evm_update_evmxattr(dentry, NULL, NULL, 0); } -static int evm_inode_copy_up_xattr(const char *name) +static int evm_inode_copy_up_xattr(struct dentry *src, const char *name) { if (strcmp(name, XATTR_NAME_EVM) == 0) return 1; /* Discard */ diff --git a/security/security.c b/security/security.c index 0a9a0ac3f266..e5da848c50b9 100644 --- a/security/security.c +++ b/security/security.c @@ -2628,6 +2628,7 @@ EXPORT_SYMBOL(security_inode_copy_up); /** * security_inode_copy_up_xattr() - Filter xattrs in an overlayfs copy-up op + * @src: union dentry of copy-up file * @name: xattr name * * Filter the xattrs being copied up when a unioned file is copied up from a @@ -2638,7 +2639,7 @@ EXPORT_SYMBOL(security_inode_copy_up); * if the security module does not know about attribute, or a negative * error code to abort the copy up. */ -int security_inode_copy_up_xattr(const char *name) +int security_inode_copy_up_xattr(struct dentry *src, const char *name) { int rc; @@ -2647,7 +2648,7 @@ int security_inode_copy_up_xattr(const char *name) * xattr), -EOPNOTSUPP if it does not know anything about the xattr or * any other error code in case of an error. */ - rc = call_int_hook(inode_copy_up_xattr, name); + rc = call_int_hook(inode_copy_up_xattr, src, name); if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr)) return rc; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 3448454c82d0..9f018f51e47a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3530,7 +3530,7 @@ static int selinux_inode_copy_up(struct dentry *src, struct cred **new) return 0; } -static int selinux_inode_copy_up_xattr(const char *name) +static int selinux_inode_copy_up_xattr(struct dentry *dentry, const char *name) { /* The copy_up hook above sets the initial context on an inode, but we * don't then want to overwrite it by blindly copying all the lower diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 146667937811..79d8dc633b9a 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4885,7 +4885,7 @@ static int smack_inode_copy_up(struct dentry *dentry, struct cred **new) return 0; } -static int smack_inode_copy_up_xattr(const char *name) +static int smack_inode_copy_up_xattr(struct dentry *src, const char *name) { /* * Return 1 if this is the smack access Smack attribute. -- cgit v1.2.3-70-g09d2 From f2b3fc42f6ce19524d8ecaf9f878456ed8c50914 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:06 -0500 Subject: evm: Implement per signature type decision in security_inode_copy_up_xattr To support "portable and immutable signatures" on otherwise unsupported filesystems, determine the EVM signature type by the content of a file's xattr. If the file has the appropriate signature type then allow it to be copied up. All other signature types are discarded as before. "Portable and immutable" EVM signatures can be copied up by stacked file- system since the metadata their signature covers does not include file- system-specific data such as a file's inode number, generation, and UUID. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b0a862bfd74a..c658d2f1494b 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -948,9 +948,34 @@ static void evm_inode_post_setattr(struct mnt_idmap *idmap, static int evm_inode_copy_up_xattr(struct dentry *src, const char *name) { - if (strcmp(name, XATTR_NAME_EVM) == 0) - return 1; /* Discard */ - return -EOPNOTSUPP; + struct evm_ima_xattr_data *xattr_data = NULL; + int rc; + + if (strcmp(name, XATTR_NAME_EVM) != 0) + return -EOPNOTSUPP; + + /* first need to know the sig type */ + rc = vfs_getxattr_alloc(&nop_mnt_idmap, src, XATTR_NAME_EVM, + (char **)&xattr_data, 0, GFP_NOFS); + if (rc <= 0) + return -EPERM; + + if (rc < offsetof(struct evm_ima_xattr_data, type) + + sizeof(xattr_data->type)) + return -EPERM; + + switch (xattr_data->type) { + case EVM_XATTR_PORTABLE_DIGSIG: + rc = 0; /* allow copy-up */ + break; + case EVM_XATTR_HMAC: + case EVM_IMA_XATTR_DIGSIG: + default: + rc = 1; /* discard */ + } + + kfree(xattr_data); + return rc; } /* -- cgit v1.2.3-70-g09d2 From faf994811e0548df854e5fd946f3094725fcd9b5 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:07 -0500 Subject: evm: Use the metadata inode to calculate metadata hash Changes to file attributes (mode bits, uid, gid) on the lower layer are not taken into account when d_backing_inode() is used when a file is accessed on the overlay layer and this file has not yet been copied up. This is because d_backing_inode() does not return the real inode of the lower layer but instead returns the backing inode which in this case holds wrong file attributes. Further, when CONFIG_OVERLAY_FS_METACOPY is enabled and a copy-up is triggered due to file metadata changes, then the metadata are held by the backing inode while the data are still held by the real inode. Therefore, use d_inode(d_real(dentry, D_REAL_METADATA)) to get to the file's metadata inode and use it to calculate the metadata hash with. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Acked-by: Amir Goldstein Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 7552d49d0725..35416f55391c 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -223,7 +223,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, size_t req_xattr_value_len, uint8_t type, struct evm_digest *data) { - struct inode *inode = d_backing_inode(dentry); + struct inode *inode = d_inode(d_real(dentry, D_REAL_METADATA)); struct xattr_list *xattr; struct shash_desc *desc; size_t xattr_size = 0; -- 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 'security') 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 'security') 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 cd9b909a117210bfd77a89bb06a3154c1fc51b51 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:10 -0500 Subject: ima: re-evaluate file integrity on file metadata change Force a file's integrity to be re-evaluated on file metadata change by resetting both the IMA and EVM status flags. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 4b215d85c14b..f04f43af651c 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "ima.h" @@ -211,6 +212,7 @@ static int process_measurement(struct file *file, const struct cred *cred, struct inode *real_inode, *inode = file_inode(file); struct ima_iint_cache *iint = NULL; struct ima_template_desc *template_desc = NULL; + struct inode *metadata_inode; char *pathbuf = NULL; char filename[NAME_MAX]; const char *pathname = NULL; @@ -286,7 +288,8 @@ static int process_measurement(struct file *file, const struct cred *cred, } /* - * On stacked filesystems, detect and re-evaluate file data changes. + * On stacked filesystems, detect and re-evaluate file data and + * metadata changes. */ real_inode = d_real_inode(file_dentry(file)); if (real_inode != inode && @@ -297,6 +300,15 @@ static int process_measurement(struct file *file, const struct cred *cred, iint->flags &= ~IMA_DONE_MASK; iint->measured_pcrs = 0; } + + /* + * Reset the EVM status when metadata changed. + */ + metadata_inode = d_inode(d_real(file_dentry(file), + D_REAL_METADATA)); + if (evm_metadata_changed(inode, metadata_inode)) + iint->flags &= ~(IMA_APPRAISED | + IMA_APPRAISED_SUBMASK); } /* Determine if already appraised/measured based on bitmask -- cgit v1.2.3-70-g09d2 From 47add87ad181473e5ef2438918669540ba5016a6 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:11 -0500 Subject: evm: Enforce signatures on unsupported filesystem for EVM_INIT_X509 Unsupported filesystems currently do not enforce any signatures. Add support for signature enforcement of the "original" and "portable & immutable" signatures when EVM_INIT_X509 is enabled. The "original" signature type contains filesystem specific metadata. Thus it cannot be copied up and verified. However with EVM_INIT_X509 and EVM_ALLOW_METADATA_WRITES enabled, the "original" file signature may be written. When EVM_ALLOW_METADATA_WRITES is not set or once it is removed from /sys/kernel/security/evm by setting EVM_INIT_HMAC for example, it is not possible to write or remove xattrs on the overlay filesystem. This change still prevents EVM from writing HMAC signatures on unsupported filesystem when EVM_INIT_HMAC is enabled. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index c1ca0894cd8a..cfb4f9809369 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -192,7 +192,11 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, iint->evm_status == INTEGRITY_PASS_IMMUTABLE)) return iint->evm_status; - if (is_unsupported_fs(dentry)) + /* + * On unsupported filesystems without EVM_INIT_X509 enabled, skip + * signature verification. + */ + if (!(evm_initialized & EVM_INIT_X509) && is_unsupported_fs(dentry)) return INTEGRITY_UNKNOWN; /* if status is not PASS, try to check again - against -ENOMEM */ @@ -261,7 +265,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, evm_status = INTEGRITY_PASS_IMMUTABLE; } else if (!IS_RDONLY(inode) && !(inode->i_sb->s_readonly_remount) && - !IS_IMMUTABLE(inode)) { + !IS_IMMUTABLE(inode) && + !is_unsupported_fs(dentry)) { evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); @@ -419,9 +424,6 @@ enum integrity_status evm_verifyxattr(struct dentry *dentry, if (!evm_key_loaded() || !evm_protected_xattr(xattr_name)) return INTEGRITY_UNKNOWN; - if (is_unsupported_fs(dentry)) - return INTEGRITY_UNKNOWN; - return evm_verify_hmac(dentry, xattr_name, xattr_value, xattr_value_len); } -- 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 'security') 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 From 5e2e4d0ea5c2c886c4a082890be6a1c2ee064605 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Fri, 23 Feb 2024 12:25:13 -0500 Subject: evm: Rename is_unsupported_fs to is_unsupported_hmac_fs Rename is_unsupported_fs to is_unsupported_hmac_fs since now only HMAC is unsupported. Co-developed-by: Mimi Zohar Signed-off-by: Stefan Berger Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index c4a6081ce596..62fe66dd53ce 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -151,7 +151,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry) return count; } -static int is_unsupported_fs(struct dentry *dentry) +static int is_unsupported_hmac_fs(struct dentry *dentry) { struct inode *inode = d_backing_inode(dentry); @@ -196,7 +196,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, * On unsupported filesystems without EVM_INIT_X509 enabled, skip * signature verification. */ - if (!(evm_initialized & EVM_INIT_X509) && is_unsupported_fs(dentry)) + if (!(evm_initialized & EVM_INIT_X509) && + is_unsupported_hmac_fs(dentry)) return INTEGRITY_UNKNOWN; /* if status is not PASS, try to check again - against -ENOMEM */ @@ -266,7 +267,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, } else if (!IS_RDONLY(inode) && !(inode->i_sb->s_readonly_remount) && !IS_IMMUTABLE(inode) && - !is_unsupported_fs(dentry)) { + !is_unsupported_hmac_fs(dentry)) { evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); @@ -502,12 +503,12 @@ static int evm_protect_xattr(struct mnt_idmap *idmap, if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) { if (!capable(CAP_SYS_ADMIN)) return -EPERM; - if (is_unsupported_fs(dentry)) + if (is_unsupported_hmac_fs(dentry)) return -EPERM; } else if (!evm_protected_xattr(xattr_name)) { if (!posix_xattr_acl(xattr_name)) return 0; - if (is_unsupported_fs(dentry)) + if (is_unsupported_hmac_fs(dentry)) return 0; evm_status = evm_verify_current_integrity(dentry); @@ -515,7 +516,7 @@ static int evm_protect_xattr(struct mnt_idmap *idmap, (evm_status == INTEGRITY_NOXATTRS)) return 0; goto out; - } else if (is_unsupported_fs(dentry)) + } else if (is_unsupported_hmac_fs(dentry)) return 0; evm_status = evm_verify_current_integrity(dentry); @@ -817,7 +818,7 @@ static void evm_inode_post_setxattr(struct dentry *dentry, if (!(evm_initialized & EVM_INIT_HMAC)) return; - if (is_unsupported_fs(dentry)) + if (is_unsupported_hmac_fs(dentry)) return; evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); @@ -916,7 +917,7 @@ static int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry, if (evm_initialized & EVM_ALLOW_METADATA_WRITES) return 0; - if (is_unsupported_fs(dentry)) + if (is_unsupported_hmac_fs(dentry)) return 0; if (!(ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))) @@ -967,7 +968,7 @@ static void evm_inode_post_setattr(struct mnt_idmap *idmap, if (!(evm_initialized & EVM_INIT_HMAC)) return; - if (is_unsupported_fs(dentry)) + if (is_unsupported_hmac_fs(dentry)) return; if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) -- cgit v1.2.3-70-g09d2 From 9fa8e76250082a45d0d3dad525419ab98bd01658 Mon Sep 17 00:00:00 2001 From: Enrico Bravi Date: Fri, 12 Apr 2024 11:09:53 +0200 Subject: ima: add crypto agility support for template-hash algorithm The template hash showed by the ascii_runtime_measurements and binary_runtime_measurements is the one calculated using sha1 and there is no possibility to change this value, despite the fact that the template hash is calculated using the hash algorithms corresponding to all the PCR banks configured in the TPM. Add the support to retrieve the ima log with the template data hash calculated with a specific hash algorithm. Add a new file in the securityfs ima directory for each hash algo configured in a PCR bank of the TPM. Each new file has the name with the following structure: {binary, ascii}_runtime_measurements_ Legacy files are kept, to avoid breaking existing applications, but as symbolic links which point to {binary, ascii}_runtime_measurements_sha1 files. These two files are created even if a TPM chip is not detected or the sha1 bank is not configured in the TPM. As example, in the case a TPM chip is present and sha256 is the only configured PCR bank, the listing of the securityfs ima directory is the following: lr--r--r-- [...] ascii_runtime_measurements -> ascii_runtime_measurements_sha1 -r--r----- [...] ascii_runtime_measurements_sha1 -r--r----- [...] ascii_runtime_measurements_sha256 lr--r--r-- [...] binary_runtime_measurements -> binary_runtime_measurements_sha1 -r--r----- [...] binary_runtime_measurements_sha1 -r--r----- [...] binary_runtime_measurements_sha256 --w------- [...] policy -r--r----- [...] runtime_measurements_count -r--r----- [...] violations Signed-off-by: Enrico Bravi Signed-off-by: Silvia Sisinni Reviewed-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 8 +++ security/integrity/ima/ima_crypto.c | 7 +- security/integrity/ima/ima_fs.c | 134 ++++++++++++++++++++++++++++++++---- security/integrity/ima/ima_kexec.c | 1 + 4 files changed, 132 insertions(+), 18 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 9151b5369cdc..3e568126cd48 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -49,11 +49,19 @@ extern int ima_policy_flag; /* bitset of digests algorithms allowed in the setxattr hook */ extern atomic_t ima_setxattr_allowed_hash_algorithms; +/* IMA hash algorithm description */ +struct ima_algo_desc { + struct crypto_shash *tfm; + enum hash_algo algo; +}; + /* set during initialization */ extern int ima_hash_algo __ro_after_init; extern int ima_sha1_idx __ro_after_init; extern int ima_hash_algo_idx __ro_after_init; extern int ima_extra_slots __ro_after_init; +extern struct ima_algo_desc *ima_algo_array __ro_after_init; + extern int ima_appraise; extern struct tpm_chip *ima_tpm_chip; extern const char boot_aggregate_name[]; diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index f3738b2c8bcd..6f5696d999d0 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -57,11 +57,6 @@ MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size"); static struct crypto_shash *ima_shash_tfm; static struct crypto_ahash *ima_ahash_tfm; -struct ima_algo_desc { - struct crypto_shash *tfm; - enum hash_algo algo; -}; - int ima_sha1_idx __ro_after_init; int ima_hash_algo_idx __ro_after_init; /* @@ -70,7 +65,7 @@ int ima_hash_algo_idx __ro_after_init; */ int ima_extra_slots __ro_after_init; -static struct ima_algo_desc *ima_algo_array; +struct ima_algo_desc *ima_algo_array __ro_after_init; static int __init ima_init_ima_crypto(void) { diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index cd1683dad3bf..abdd22007ed8 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -116,9 +116,31 @@ void ima_putc(struct seq_file *m, void *data, int datalen) seq_putc(m, *(char *)data++); } +static struct dentry **ascii_securityfs_measurement_lists __ro_after_init; +static struct dentry **binary_securityfs_measurement_lists __ro_after_init; +static int securityfs_measurement_list_count __ro_after_init; + +static void lookup_template_data_hash_algo(int *algo_idx, enum hash_algo *algo, + struct seq_file *m, + struct dentry **lists) +{ + struct dentry *dentry; + int i; + + dentry = file_dentry(m->file); + + for (i = 0; i < securityfs_measurement_list_count; i++) { + if (dentry == lists[i]) { + *algo_idx = i; + *algo = ima_algo_array[i].algo; + break; + } + } +} + /* print format: * 32bit-le=pcr# - * char[20]=template digest + * char[n]=template digest * 32bit-le=template name size * char[n]=template name * [eventdata length] @@ -132,7 +154,15 @@ int ima_measurements_show(struct seq_file *m, void *v) char *template_name; u32 pcr, namelen, template_data_len; /* temporary fields */ bool is_ima_template = false; - int i; + enum hash_algo algo; + int i, algo_idx; + + algo_idx = ima_sha1_idx; + algo = HASH_ALGO_SHA1; + + if (m->file != NULL) + lookup_template_data_hash_algo(&algo_idx, &algo, m, + binary_securityfs_measurement_lists); /* get entry */ e = qe->entry; @@ -151,7 +181,7 @@ int ima_measurements_show(struct seq_file *m, void *v) ima_putc(m, &pcr, sizeof(e->pcr)); /* 2nd: template digest */ - ima_putc(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); + ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]); /* 3rd: template name size */ namelen = !ima_canonical_fmt ? strlen(template_name) : @@ -220,7 +250,15 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) struct ima_queue_entry *qe = v; struct ima_template_entry *e; char *template_name; - int i; + enum hash_algo algo; + int i, algo_idx; + + algo_idx = ima_sha1_idx; + algo = HASH_ALGO_SHA1; + + if (m->file != NULL) + lookup_template_data_hash_algo(&algo_idx, &algo, m, + ascii_securityfs_measurement_lists); /* get entry */ e = qe->entry; @@ -233,8 +271,8 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) /* 1st: PCR used (config option) */ seq_printf(m, "%2d ", e->pcr); - /* 2nd: SHA1 template hash */ - ima_print_digest(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); + /* 2nd: template hash */ + ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]); /* 3th: template name */ seq_printf(m, " %s", template_name); @@ -379,6 +417,71 @@ static const struct seq_operations ima_policy_seqops = { }; #endif +static void __init remove_securityfs_measurement_lists(struct dentry **lists) +{ + int i; + + if (lists) { + for (i = 0; i < securityfs_measurement_list_count; i++) + securityfs_remove(lists[i]); + + kfree(lists); + } + + securityfs_measurement_list_count = 0; +} + +static int __init create_securityfs_measurement_lists(void) +{ + char file_name[NAME_MAX + 1]; + struct dentry *dentry; + u16 algo; + int i; + + securityfs_measurement_list_count = NR_BANKS(ima_tpm_chip); + + if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip)) + securityfs_measurement_list_count++; + + ascii_securityfs_measurement_lists = + kcalloc(securityfs_measurement_list_count, sizeof(struct dentry *), + GFP_KERNEL); + if (!ascii_securityfs_measurement_lists) + return -ENOMEM; + + binary_securityfs_measurement_lists = + kcalloc(securityfs_measurement_list_count, sizeof(struct dentry *), + GFP_KERNEL); + if (!binary_securityfs_measurement_lists) + return -ENOMEM; + + for (i = 0; i < securityfs_measurement_list_count; i++) { + algo = ima_algo_array[i].algo; + + sprintf(file_name, "ascii_runtime_measurements_%s", + hash_algo_name[algo]); + dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP, + ima_dir, NULL, + &ima_ascii_measurements_ops); + if (IS_ERR(dentry)) + return PTR_ERR(dentry); + + ascii_securityfs_measurement_lists[i] = dentry; + + sprintf(file_name, "binary_runtime_measurements_%s", + hash_algo_name[algo]); + dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP, + ima_dir, NULL, + &ima_measurements_ops); + if (IS_ERR(dentry)) + return PTR_ERR(dentry); + + binary_securityfs_measurement_lists[i] = dentry; + } + + return 0; +} + /* * ima_open_policy: sequentialize access to the policy file */ @@ -454,6 +557,9 @@ int __init ima_fs_init(void) { int ret; + ascii_securityfs_measurement_lists = NULL; + binary_securityfs_measurement_lists = NULL; + ima_dir = securityfs_create_dir("ima", integrity_dir); if (IS_ERR(ima_dir)) return PTR_ERR(ima_dir); @@ -465,19 +571,21 @@ int __init ima_fs_init(void) goto out; } + ret = create_securityfs_measurement_lists(); + if (ret != 0) + goto out; + binary_runtime_measurements = - securityfs_create_file("binary_runtime_measurements", - S_IRUSR | S_IRGRP, ima_dir, NULL, - &ima_measurements_ops); + securityfs_create_symlink("binary_runtime_measurements", ima_dir, + "binary_runtime_measurements_sha1", NULL); if (IS_ERR(binary_runtime_measurements)) { ret = PTR_ERR(binary_runtime_measurements); goto out; } ascii_runtime_measurements = - securityfs_create_file("ascii_runtime_measurements", - S_IRUSR | S_IRGRP, ima_dir, NULL, - &ima_ascii_measurements_ops); + securityfs_create_symlink("ascii_runtime_measurements", ima_dir, + "ascii_runtime_measurements_sha1", NULL); if (IS_ERR(ascii_runtime_measurements)) { ret = PTR_ERR(ascii_runtime_measurements); goto out; @@ -515,6 +623,8 @@ out: securityfs_remove(runtime_measurements_count); securityfs_remove(ascii_runtime_measurements); securityfs_remove(binary_runtime_measurements); + remove_securityfs_measurement_lists(ascii_securityfs_measurement_lists); + remove_securityfs_measurement_lists(binary_securityfs_measurement_lists); securityfs_remove(ima_symlink); securityfs_remove(ima_dir); diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index dadc1d138118..52e00332defe 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -30,6 +30,7 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer, goto out; } + file.file = NULL; file.size = segment_size; file.read_pos = 0; file.count = sizeof(khdr); /* reserved space */ -- cgit v1.2.3-70-g09d2