From b44a7dfc6fa16e01f2497c9fa62c3926f94be174 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 28 Dec 2015 16:02:29 -0500 Subject: vfs: define a generic function to read a file from the kernel For a while it was looked down upon to directly read files from Linux. These days there exists a few mechanisms in the kernel that do just this though to load a file into a local buffer. There are minor but important checks differences on each. This patch set is the first attempt at resolving some of these differences. This patch introduces a common function for reading files from the kernel with the corresponding security post-read hook and function. Changelog v4+: - export security_kernel_post_read_file() - Fengguang Wu v3: - additional bounds checking - Luis v2: - To simplify patch review, re-ordered patches Signed-off-by: Mimi Zohar Reviewed-by: Luis R. Rodriguez Acked-by: Kees Cook Cc: Al Viro --- security/security.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'security/security.c') diff --git a/security/security.c b/security/security.c index e8ffd92ae2eb..c98dd6bf4ebd 100644 --- a/security/security.c +++ b/security/security.c @@ -910,6 +910,12 @@ int security_kernel_module_from_file(struct file *file) return ima_module_check(file); } +int security_kernel_post_read_file(struct file *file, char *buf, loff_t size) +{ + return call_int_hook(kernel_post_read_file, 0, file, buf, size); +} +EXPORT_SYMBOL_GPL(security_kernel_post_read_file); + int security_task_fix_setuid(struct cred *new, const struct cred *old, int flags) { @@ -1697,6 +1703,8 @@ struct security_hook_heads security_hook_heads = { LIST_HEAD_INIT(security_hook_heads.kernel_module_request), .kernel_module_from_file = LIST_HEAD_INIT(security_hook_heads.kernel_module_from_file), + .kernel_post_read_file = + LIST_HEAD_INIT(security_hook_heads.kernel_post_read_file), .task_fix_setuid = LIST_HEAD_INIT(security_hook_heads.task_fix_setuid), .task_setpgid = LIST_HEAD_INIT(security_hook_heads.task_setpgid), -- cgit v1.2.3-70-g09d2 From bc8ca5b92d54f6f005fa73ad546f02fca26ddd85 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Sun, 24 Jan 2016 10:07:32 -0500 Subject: vfs: define kernel_read_file_id enumeration To differentiate between the kernel_read_file() callers, this patch defines a new enumeration named kernel_read_file_id and includes the caller identifier as an argument. Subsequent patches define READING_KEXEC_IMAGE, READING_KEXEC_INITRAMFS, READING_FIRMWARE, READING_MODULE, and READING_POLICY. Changelog v3: - Replace the IMA specific enumeration with a generic one. Signed-off-by: Mimi Zohar Acked-by: Kees Cook Acked-by: Luis R. Rodriguez Cc: Al Viro --- fs/exec.c | 4 ++-- include/linux/fs.h | 7 ++++++- include/linux/lsm_hooks.h | 4 +++- include/linux/security.h | 7 +++++-- security/security.c | 5 +++-- 5 files changed, 19 insertions(+), 8 deletions(-) (limited to 'security/security.c') diff --git a/fs/exec.c b/fs/exec.c index 6b6668baa44a..1138dc502c77 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -833,7 +833,7 @@ int kernel_read(struct file *file, loff_t offset, EXPORT_SYMBOL(kernel_read); int kernel_read_file(struct file *file, void **buf, loff_t *size, - loff_t max_size) + loff_t max_size, enum kernel_read_file_id id) { loff_t i_size, pos; ssize_t bytes = 0; @@ -871,7 +871,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, goto out; } - ret = security_kernel_post_read_file(file, *buf, i_size); + ret = security_kernel_post_read_file(file, *buf, i_size, id); if (!ret) *size = pos; diff --git a/include/linux/fs.h b/include/linux/fs.h index 9a83d82b61ac..aa84bcb9c368 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2576,8 +2576,13 @@ static inline void i_readcount_inc(struct inode *inode) #endif extern int do_pipe_flags(int *, int); +enum kernel_read_file_id { + READING_MAX_ID +}; + extern int kernel_read(struct file *, loff_t, char *, unsigned long); -extern int kernel_read_file(struct file *, void **, loff_t *, loff_t); +extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, + enum kernel_read_file_id); extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t); extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *); extern struct file * open_exec(const char *); diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index f82631cc7248..2337f33913c1 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -567,6 +567,7 @@ * by the kernel. * @buf pointer to buffer containing the file contents. * @size length of the file contents. + * @id kernel read file identifier * Return 0 if permission is granted. * @task_fix_setuid: * Update the module's state after setting one or more of the user @@ -1464,7 +1465,8 @@ union security_list_options { int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size); int (*kernel_module_request)(char *kmod_name); int (*kernel_module_from_file)(struct file *file); - int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size); + int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size, + enum kernel_read_file_id id); int (*task_fix_setuid)(struct cred *new, const struct cred *old, int flags); int (*task_setpgid)(struct task_struct *p, pid_t pgid); diff --git a/include/linux/security.h b/include/linux/security.h index f30f5647d1e1..b68ce94e4e00 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -28,6 +28,7 @@ #include #include #include +#include struct linux_binprm; struct cred; @@ -301,7 +302,8 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode); int security_kernel_fw_from_file(struct file *file, char *buf, size_t size); int security_kernel_module_request(char *kmod_name); int security_kernel_module_from_file(struct file *file); -int security_kernel_post_read_file(struct file *file, char *buf, loff_t size); +int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, + enum kernel_read_file_id id); int security_task_fix_setuid(struct cred *new, const struct cred *old, int flags); int security_task_setpgid(struct task_struct *p, pid_t pgid); @@ -868,7 +870,8 @@ static inline int security_kernel_module_from_file(struct file *file) } static inline int security_kernel_post_read_file(struct file *file, - char *buf, loff_t size) + char *buf, loff_t size, + enum kernel_read_file_id id) { return 0; } diff --git a/security/security.c b/security/security.c index c98dd6bf4ebd..5b96eabaafd4 100644 --- a/security/security.c +++ b/security/security.c @@ -910,9 +910,10 @@ int security_kernel_module_from_file(struct file *file) return ima_module_check(file); } -int security_kernel_post_read_file(struct file *file, char *buf, loff_t size) +int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, + enum kernel_read_file_id id) { - return call_int_hook(kernel_post_read_file, 0, file, buf, size); + return call_int_hook(kernel_post_read_file, 0, file, buf, size, id); } EXPORT_SYMBOL_GPL(security_kernel_post_read_file); -- cgit v1.2.3-70-g09d2 From cf2222178645e545e96717b2825601321ce4745c Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Thu, 14 Jan 2016 17:57:47 -0500 Subject: ima: define a new hook to measure and appraise a file already in memory This patch defines a new IMA hook ima_post_read_file() for measuring and appraising files read by the kernel. The caller loads the file into memory before calling this function, which calculates the hash followed by the normal IMA policy based processing. Changelog v5: - fail ima_post_read_file() if either file or buf is NULL v3: - rename ima_hash_and_process_file() to ima_post_read_file() v1: - split patch Signed-off-by: Mimi Zohar Acked-by: Dmitry Kasatkin --- include/linux/ima.h | 8 +++++++ include/linux/security.h | 1 + security/integrity/ima/ima.h | 4 +++- security/integrity/ima/ima_api.c | 6 +++-- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/ima/ima_main.c | 45 ++++++++++++++++++++++++++++------- security/integrity/ima/ima_policy.c | 1 + security/integrity/integrity.h | 7 ++++-- security/security.c | 7 +++++- 9 files changed, 66 insertions(+), 15 deletions(-) (limited to 'security/security.c') diff --git a/include/linux/ima.h b/include/linux/ima.h index 120ccc53fcb7..d29a6a23fc19 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -20,6 +20,8 @@ extern void ima_file_free(struct file *file); extern int ima_file_mmap(struct file *file, unsigned long prot); extern int ima_module_check(struct file *file); extern int ima_fw_from_file(struct file *file, char *buf, size_t size); +extern int ima_post_read_file(struct file *file, void *buf, loff_t size, + enum kernel_read_file_id id); #else static inline int ima_bprm_check(struct linux_binprm *bprm) @@ -52,6 +54,12 @@ static inline int ima_fw_from_file(struct file *file, char *buf, size_t size) return 0; } +static inline int ima_post_read_file(struct file *file, void *buf, loff_t size, + enum kernel_read_file_id id) +{ + return 0; +} + #endif /* CONFIG_IMA */ #ifdef CONFIG_IMA_APPRAISE diff --git a/include/linux/security.h b/include/linux/security.h index b68ce94e4e00..d920718dc845 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 2c5262f2823f..0b7134c04165 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -152,7 +153,8 @@ enum ima_hooks { int ima_get_action(struct inode *inode, int mask, enum ima_hooks func); int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func); int ima_collect_measurement(struct integrity_iint_cache *iint, - struct file *file, enum hash_algo algo); + struct file *file, void *buf, loff_t size, + enum hash_algo algo); void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 8750254506a9..370e42dfc5c5 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -188,7 +188,8 @@ int ima_get_action(struct inode *inode, int mask, enum ima_hooks func) * Return 0 on success, error code otherwise */ int ima_collect_measurement(struct integrity_iint_cache *iint, - struct file *file, enum hash_algo algo) + struct file *file, void *buf, loff_t size, + enum hash_algo algo) { const char *audit_cause = "failed"; struct inode *inode = file_inode(file); @@ -210,7 +211,8 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, hash.hdr.algo = algo; - result = ima_calc_file_hash(file, &hash.hdr); + result = (!buf) ? ima_calc_file_hash(file, &hash.hdr) : + ima_calc_buffer_hash(buf, size, &hash.hdr); if (!result) { int length = sizeof(hash.hdr) + hash.hdr.length; void *tmpbuf = krealloc(iint->ima_hash, length, diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 288844908788..cb0d0ff1137b 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -300,7 +300,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file) if (iint->flags & IMA_DIGSIG) return; - rc = ima_collect_measurement(iint, file, ima_hash_algo); + rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo); if (rc < 0) return; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 1be99a27a7f3..757765354158 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -153,8 +153,8 @@ void ima_file_free(struct file *file) ima_check_last_writer(iint, inode, file); } -static int process_measurement(struct file *file, int mask, - enum ima_hooks func, int opened) +static int process_measurement(struct file *file, char *buf, loff_t size, + int mask, enum ima_hooks func, int opened) { struct inode *inode = file_inode(file); struct integrity_iint_cache *iint = NULL; @@ -226,7 +226,7 @@ static int process_measurement(struct file *file, int mask, hash_algo = ima_get_hash_algo(xattr_value, xattr_len); - rc = ima_collect_measurement(iint, file, hash_algo); + rc = ima_collect_measurement(iint, file, buf, size, hash_algo); if (rc != 0) { if (file->f_flags & O_DIRECT) rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES; @@ -273,7 +273,8 @@ out: int ima_file_mmap(struct file *file, unsigned long prot) { if (file && (prot & PROT_EXEC)) - return process_measurement(file, MAY_EXEC, MMAP_CHECK, 0); + return process_measurement(file, NULL, 0, MAY_EXEC, + MMAP_CHECK, 0); return 0; } @@ -292,7 +293,8 @@ int ima_file_mmap(struct file *file, unsigned long prot) */ int ima_bprm_check(struct linux_binprm *bprm) { - return process_measurement(bprm->file, MAY_EXEC, BPRM_CHECK, 0); + return process_measurement(bprm->file, NULL, 0, MAY_EXEC, + BPRM_CHECK, 0); } /** @@ -307,7 +309,7 @@ int ima_bprm_check(struct linux_binprm *bprm) */ int ima_file_check(struct file *file, int mask, int opened) { - return process_measurement(file, + return process_measurement(file, NULL, 0, mask & (MAY_READ | MAY_WRITE | MAY_EXEC), FILE_CHECK, opened); } @@ -332,7 +334,7 @@ int ima_module_check(struct file *file) #endif return 0; /* We rely on module signature checking */ } - return process_measurement(file, MAY_EXEC, MODULE_CHECK, 0); + return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0); } int ima_fw_from_file(struct file *file, char *buf, size_t size) @@ -343,7 +345,34 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size) return -EACCES; /* INTEGRITY_UNKNOWN */ return 0; } - return process_measurement(file, MAY_EXEC, FIRMWARE_CHECK, 0); + return process_measurement(file, NULL, 0, MAY_EXEC, FIRMWARE_CHECK, 0); +} + +/** + * ima_post_read_file - in memory collect/appraise/audit measurement + * @file: pointer to the file to be measured/appraised/audit + * @buf: pointer to in memory file contents + * @size: size of in memory file contents + * @read_id: caller identifier + * + * Measure/appraise/audit in memory file based on policy. Policy rules + * are written in terms of a policy identifier. + * + * On success return 0. On integrity appraisal error, assuming the file + * is in policy and IMA-appraisal is in enforcing mode, return -EACCES. + */ +int ima_post_read_file(struct file *file, void *buf, loff_t size, + enum kernel_read_file_id read_id) +{ + enum ima_hooks func = FILE_CHECK; + + if (!file || !buf || size == 0) { /* should never happen */ + if (ima_appraise & IMA_APPRAISE_ENFORCE) + return -EACCES; + return 0; + } + + return process_measurement(file, buf, size, MAY_READ, func, 0); } static int __init init_ima(void) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index b089ebef6648..cfbe86f476d0 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -12,6 +12,7 @@ */ #include #include +#include #include #include #include diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 5efe2ecc538d..9a0ea4c4e3dd 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -49,12 +49,14 @@ #define IMA_MODULE_APPRAISED 0x00008000 #define IMA_FIRMWARE_APPRAISE 0x00010000 #define IMA_FIRMWARE_APPRAISED 0x00020000 +#define IMA_READ_APPRAISE 0x00040000 +#define IMA_READ_APPRAISED 0x00080000 #define IMA_APPRAISE_SUBMASK (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \ IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE | \ - IMA_FIRMWARE_APPRAISE) + IMA_FIRMWARE_APPRAISE | IMA_READ_APPRAISE) #define IMA_APPRAISED_SUBMASK (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \ IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED | \ - IMA_FIRMWARE_APPRAISED) + IMA_FIRMWARE_APPRAISED | IMA_READ_APPRAISED) enum evm_ima_xattr_type { IMA_XATTR_DIGEST = 0x01, @@ -111,6 +113,7 @@ struct integrity_iint_cache { enum integrity_status ima_bprm_status:4; enum integrity_status ima_module_status:4; enum integrity_status ima_firmware_status:4; + enum integrity_status ima_read_status:4; enum integrity_status evm_status:4; struct ima_digest_data *ima_hash; }; diff --git a/security/security.c b/security/security.c index 5b96eabaafd4..ef4c65a9fd17 100644 --- a/security/security.c +++ b/security/security.c @@ -913,7 +913,12 @@ int security_kernel_module_from_file(struct file *file) int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id) { - return call_int_hook(kernel_post_read_file, 0, file, buf, size, id); + int ret; + + ret = call_int_hook(kernel_post_read_file, 0, file, buf, size, id); + if (ret) + return ret; + return ima_post_read_file(file, buf, size, id); } EXPORT_SYMBOL_GPL(security_kernel_post_read_file); -- cgit v1.2.3-70-g09d2 From e40ba6d56b41754b37b995dbc8035b2b3a6afd8a Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Thu, 19 Nov 2015 12:39:22 -0500 Subject: firmware: replace call to fw_read_file_contents() with kernel version Replace the fw_read_file_contents with kernel_file_read_from_path(). Although none of the upstreamed LSMs define a kernel_fw_from_file hook, IMA is called by the security function to prevent unsigned firmware from being loaded and to measure/appraise signed firmware, based on policy. Instead of reading the firmware twice, once for measuring/appraising the firmware and again for reading the firmware contents into memory, the kernel_post_read_file() security hook calculates the file hash based on the in memory file buffer. The firmware is read once. This patch removes the LSM kernel_fw_from_file() hook and security call. Changelog v4+: - revert dropped buf->size assignment - reported by Sergey Senozhatsky v3: - remove kernel_fw_from_file hook - use kernel_file_read_from_path() - requested by Luis v2: - reordered and squashed firmware patches - fix MAX firmware size (Kees Cook) Signed-off-by: Mimi Zohar Acked-by: Kees Cook Acked-by: Luis R. Rodriguez --- drivers/base/firmware_class.c | 52 ++++++++------------------------------- include/linux/fs.h | 1 + include/linux/ima.h | 6 ----- include/linux/lsm_hooks.h | 11 --------- include/linux/security.h | 7 ------ security/integrity/ima/ima_main.c | 21 ++++++++-------- security/security.c | 13 ---------- 7 files changed, 21 insertions(+), 90 deletions(-) (limited to 'security/security.c') diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index c743a2f18c33..a414008ea64c 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -291,37 +292,6 @@ static const char * const fw_path[] = { module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644); MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path"); -static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf) -{ - int size; - char *buf; - int rc; - - if (!S_ISREG(file_inode(file)->i_mode)) - return -EINVAL; - size = i_size_read(file_inode(file)); - if (size <= 0) - return -EINVAL; - buf = vmalloc(size); - if (!buf) - return -ENOMEM; - rc = kernel_read(file, 0, buf, size); - if (rc != size) { - if (rc > 0) - rc = -EIO; - goto fail; - } - rc = security_kernel_fw_from_file(file, buf, size); - if (rc) - goto fail; - fw_buf->data = buf; - fw_buf->size = size; - return 0; -fail: - vfree(buf); - return rc; -} - static void fw_finish_direct_load(struct device *device, struct firmware_buf *buf) { @@ -334,6 +304,7 @@ static void fw_finish_direct_load(struct device *device, static int fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf) { + loff_t size; int i, len; int rc = -ENOENT; char *path; @@ -343,8 +314,6 @@ static int fw_get_filesystem_firmware(struct device *device, return -ENOMEM; for (i = 0; i < ARRAY_SIZE(fw_path); i++) { - struct file *file; - /* skip the unset customized path */ if (!fw_path[i][0]) continue; @@ -356,18 +325,16 @@ static int fw_get_filesystem_firmware(struct device *device, break; } - file = filp_open(path, O_RDONLY, 0); - if (IS_ERR(file)) - continue; - rc = fw_read_file_contents(file, buf); - fput(file); + buf->size = 0; + rc = kernel_read_file_from_path(path, &buf->data, &size, + INT_MAX, READING_FIRMWARE); if (rc) { dev_warn(device, "loading %s failed with error %d\n", path, rc); continue; } - dev_dbg(device, "direct-loading %s\n", - buf->fw_id); + dev_dbg(device, "direct-loading %s\n", buf->fw_id); + buf->size = size; fw_finish_direct_load(device, buf); break; } @@ -689,8 +656,9 @@ static ssize_t firmware_loading_store(struct device *dev, dev_err(dev, "%s: map pages failed\n", __func__); else - rc = security_kernel_fw_from_file(NULL, - fw_buf->data, fw_buf->size); + rc = security_kernel_post_read_file(NULL, + fw_buf->data, fw_buf->size, + READING_FIRMWARE); /* * Same logic as fw_load_abort, only the DONE bit diff --git a/include/linux/fs.h b/include/linux/fs.h index 00fa5c45fd63..c8bc4d8c843f 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2577,6 +2577,7 @@ static inline void i_readcount_inc(struct inode *inode) extern int do_pipe_flags(int *, int); enum kernel_read_file_id { + READING_FIRMWARE = 1, READING_MAX_ID }; diff --git a/include/linux/ima.h b/include/linux/ima.h index d29a6a23fc19..7aea4863c244 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -19,7 +19,6 @@ extern int ima_file_check(struct file *file, int mask, int opened); extern void ima_file_free(struct file *file); extern int ima_file_mmap(struct file *file, unsigned long prot); extern int ima_module_check(struct file *file); -extern int ima_fw_from_file(struct file *file, char *buf, size_t size); extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); @@ -49,11 +48,6 @@ static inline int ima_module_check(struct file *file) return 0; } -static inline int ima_fw_from_file(struct file *file, char *buf, size_t size) -{ - return 0; -} - static inline int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id) { diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 2337f33913c1..7d04a1220223 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -541,15 +541,6 @@ * @inode points to the inode to use as a reference. * The current task must be the one that nominated @inode. * Return 0 if successful. - * @kernel_fw_from_file: - * Load firmware from userspace (not called for built-in firmware). - * @file contains the file structure pointing to the file containing - * the firmware to load. This argument will be NULL if the firmware - * was loaded via the uevent-triggered blob-based interface exposed - * by CONFIG_FW_LOADER_USER_HELPER. - * @buf pointer to buffer containing firmware contents. - * @size length of the firmware contents. - * Return 0 if permission is granted. * @kernel_module_request: * Ability to trigger the kernel to automatically upcall to userspace for * userspace to load a kernel module with the given name. @@ -1462,7 +1453,6 @@ union security_list_options { void (*cred_transfer)(struct cred *new, const struct cred *old); int (*kernel_act_as)(struct cred *new, u32 secid); int (*kernel_create_files_as)(struct cred *new, struct inode *inode); - int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size); int (*kernel_module_request)(char *kmod_name); int (*kernel_module_from_file)(struct file *file); int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size, @@ -1725,7 +1715,6 @@ struct security_hook_heads { struct list_head cred_transfer; struct list_head kernel_act_as; struct list_head kernel_create_files_as; - struct list_head kernel_fw_from_file; struct list_head kernel_post_read_file; struct list_head kernel_module_request; struct list_head kernel_module_from_file; diff --git a/include/linux/security.h b/include/linux/security.h index d920718dc845..cee1349e1155 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -300,7 +300,6 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp); void security_transfer_creds(struct cred *new, const struct cred *old); int security_kernel_act_as(struct cred *new, u32 secid); int security_kernel_create_files_as(struct cred *new, struct inode *inode); -int security_kernel_fw_from_file(struct file *file, char *buf, size_t size); int security_kernel_module_request(char *kmod_name); int security_kernel_module_from_file(struct file *file); int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, @@ -854,12 +853,6 @@ static inline int security_kernel_create_files_as(struct cred *cred, return 0; } -static inline int security_kernel_fw_from_file(struct file *file, - char *buf, size_t size) -{ - return 0; -} - static inline int security_kernel_module_request(char *kmod_name) { return 0; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 757765354158..e9651be17b72 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -337,17 +337,6 @@ int ima_module_check(struct file *file) return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0); } -int ima_fw_from_file(struct file *file, char *buf, size_t size) -{ - if (!file) { - if ((ima_appraise & IMA_APPRAISE_FIRMWARE) && - (ima_appraise & IMA_APPRAISE_ENFORCE)) - return -EACCES; /* INTEGRITY_UNKNOWN */ - return 0; - } - return process_measurement(file, NULL, 0, MAY_EXEC, FIRMWARE_CHECK, 0); -} - /** * ima_post_read_file - in memory collect/appraise/audit measurement * @file: pointer to the file to be measured/appraised/audit @@ -366,12 +355,22 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, { enum ima_hooks func = FILE_CHECK; + if (!file && read_id == READING_FIRMWARE) { + if ((ima_appraise & IMA_APPRAISE_FIRMWARE) && + (ima_appraise & IMA_APPRAISE_ENFORCE)) + return -EACCES; /* INTEGRITY_UNKNOWN */ + return 0; + } + if (!file || !buf || size == 0) { /* should never happen */ if (ima_appraise & IMA_APPRAISE_ENFORCE) return -EACCES; return 0; } + if (read_id == READING_FIRMWARE) + func = FIRMWARE_CHECK; + return process_measurement(file, buf, size, MAY_READ, func, 0); } diff --git a/security/security.c b/security/security.c index ef4c65a9fd17..cd85be61c416 100644 --- a/security/security.c +++ b/security/security.c @@ -884,17 +884,6 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode) return call_int_hook(kernel_create_files_as, 0, new, inode); } -int security_kernel_fw_from_file(struct file *file, char *buf, size_t size) -{ - int ret; - - ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size); - if (ret) - return ret; - return ima_fw_from_file(file, buf, size); -} -EXPORT_SYMBOL_GPL(security_kernel_fw_from_file); - int security_kernel_module_request(char *kmod_name) { return call_int_hook(kernel_module_request, 0, kmod_name); @@ -1703,8 +1692,6 @@ struct security_hook_heads security_hook_heads = { LIST_HEAD_INIT(security_hook_heads.kernel_act_as), .kernel_create_files_as = LIST_HEAD_INIT(security_hook_heads.kernel_create_files_as), - .kernel_fw_from_file = - LIST_HEAD_INIT(security_hook_heads.kernel_fw_from_file), .kernel_module_request = LIST_HEAD_INIT(security_hook_heads.kernel_module_request), .kernel_module_from_file = -- cgit v1.2.3-70-g09d2 From 39eeb4fb97f60dbdfc823c1a673a8844b9226b60 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Sat, 30 Jan 2016 22:23:26 -0500 Subject: security: define kernel_read_file hook The kernel_read_file security hook is called prior to reading the file into memory. Changelog v4+: - export security_kernel_read_file() Signed-off-by: Mimi Zohar Acked-by: Kees Cook Acked-by: Luis R. Rodriguez Acked-by: Casey Schaufler --- fs/exec.c | 4 ++++ include/linux/ima.h | 6 ++++++ include/linux/lsm_hooks.h | 8 ++++++++ include/linux/security.h | 7 +++++++ security/integrity/ima/ima_main.c | 16 ++++++++++++++++ security/security.c | 13 +++++++++++++ 6 files changed, 54 insertions(+) (limited to 'security/security.c') diff --git a/fs/exec.c b/fs/exec.c index 64cb3bc788c1..8aaa38666119 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -842,6 +842,10 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0) return -EINVAL; + ret = security_kernel_read_file(file, id); + if (ret) + return ret; + i_size = i_size_read(file_inode(file)); if (max_size > 0 && i_size > max_size) return -EFBIG; diff --git a/include/linux/ima.h b/include/linux/ima.h index 7aea4863c244..6adcaea8101c 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -19,6 +19,7 @@ extern int ima_file_check(struct file *file, int mask, int opened); extern void ima_file_free(struct file *file); extern int ima_file_mmap(struct file *file, unsigned long prot); extern int ima_module_check(struct file *file); +extern int ima_read_file(struct file *file, enum kernel_read_file_id id); extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); @@ -48,6 +49,11 @@ static inline int ima_module_check(struct file *file) return 0; } +static inline int ima_read_file(struct file *file, enum kernel_read_file_id id) +{ + return 0; +} + static inline int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id) { diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 7d04a1220223..d32b7bd13635 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -552,6 +552,12 @@ * the kernel module to load. If the module is being loaded from a blob, * this argument will be NULL. * Return 0 if permission is granted. + * @kernel_read_file: + * Read a file specified by userspace. + * @file contains the file structure pointing to the file being read + * by the kernel. + * @id kernel read file identifier + * Return 0 if permission is granted. * @kernel_post_read_file: * Read a file specified by userspace. * @file contains the file structure pointing to the file being read @@ -1455,6 +1461,7 @@ union security_list_options { int (*kernel_create_files_as)(struct cred *new, struct inode *inode); int (*kernel_module_request)(char *kmod_name); int (*kernel_module_from_file)(struct file *file); + int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id); int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id); int (*task_fix_setuid)(struct cred *new, const struct cred *old, @@ -1715,6 +1722,7 @@ struct security_hook_heads { struct list_head cred_transfer; struct list_head kernel_act_as; struct list_head kernel_create_files_as; + struct list_head kernel_read_file; struct list_head kernel_post_read_file; struct list_head kernel_module_request; struct list_head kernel_module_from_file; diff --git a/include/linux/security.h b/include/linux/security.h index cee1349e1155..071fb747fdbb 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -302,6 +302,7 @@ int security_kernel_act_as(struct cred *new, u32 secid); int security_kernel_create_files_as(struct cred *new, struct inode *inode); int security_kernel_module_request(char *kmod_name); int security_kernel_module_from_file(struct file *file); +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id); int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id); int security_task_fix_setuid(struct cred *new, const struct cred *old, @@ -863,6 +864,12 @@ static inline int security_kernel_module_from_file(struct file *file) return 0; } +static inline int security_kernel_read_file(struct file *file, + enum kernel_read_file_id id) +{ + return 0; +} + static inline int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index e9651be17b72..bbb80df28fb1 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -337,6 +337,22 @@ int ima_module_check(struct file *file) return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0); } +/** + * ima_read_file - pre-measure/appraise hook decision based on policy + * @file: pointer to the file to be measured/appraised/audit + * @read_id: caller identifier + * + * Permit reading a file based on policy. The policy rules are written + * in terms of the policy identifier. Appraising the integrity of + * a file requires a file descriptor. + * + * For permission return 0, otherwise return -EACCES. + */ +int ima_read_file(struct file *file, enum kernel_read_file_id read_id) +{ + return 0; +} + /** * ima_post_read_file - in memory collect/appraise/audit measurement * @file: pointer to the file to be measured/appraised/audit diff --git a/security/security.c b/security/security.c index cd85be61c416..8e699f98a600 100644 --- a/security/security.c +++ b/security/security.c @@ -899,6 +899,17 @@ int security_kernel_module_from_file(struct file *file) return ima_module_check(file); } +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id) +{ + int ret; + + ret = call_int_hook(kernel_read_file, 0, file, id); + if (ret) + return ret; + return ima_read_file(file, id); +} +EXPORT_SYMBOL_GPL(security_kernel_read_file); + int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id) { @@ -1696,6 +1707,8 @@ struct security_hook_heads security_hook_heads = { LIST_HEAD_INIT(security_hook_heads.kernel_module_request), .kernel_module_from_file = LIST_HEAD_INIT(security_hook_heads.kernel_module_from_file), + .kernel_read_file = + LIST_HEAD_INIT(security_hook_heads.kernel_read_file), .kernel_post_read_file = LIST_HEAD_INIT(security_hook_heads.kernel_post_read_file), .task_fix_setuid = -- cgit v1.2.3-70-g09d2 From a1db74209483a24c861c848b4bb79a4d945ef6fa Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 30 Dec 2015 07:35:30 -0500 Subject: module: replace copy_module_from_fd with kernel version Replace copy_module_from_fd() with kernel_read_file_from_fd(). Although none of the upstreamed LSMs define a kernel_module_from_file hook, IMA is called, based on policy, to prevent unsigned kernel modules from being loaded by the original kernel module syscall and to measure/appraise signed kernel modules. The security function security_kernel_module_from_file() was called prior to reading a kernel module. Preventing unsigned kernel modules from being loaded by the original kernel module syscall remains on the pre-read kernel_read_file() security hook. Instead of reading the kernel module twice, once for measuring/appraising and again for loading the kernel module, the signature validation is moved to the kernel_post_read_file() security hook. This patch removes the security_kernel_module_from_file() hook and security call. Signed-off-by: Mimi Zohar Acked-by: Kees Cook Acked-by: Luis R. Rodriguez Cc: Rusty Russell --- include/linux/fs.h | 1 + include/linux/ima.h | 6 ---- include/linux/lsm_hooks.h | 7 ---- include/linux/security.h | 5 --- kernel/module.c | 68 +++++---------------------------------- security/integrity/ima/ima_main.c | 35 ++++++++------------ security/security.c | 12 ------- 7 files changed, 22 insertions(+), 112 deletions(-) (limited to 'security/security.c') diff --git a/include/linux/fs.h b/include/linux/fs.h index 9c85deae1bf2..fb08b668c37a 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2578,6 +2578,7 @@ extern int do_pipe_flags(int *, int); enum kernel_read_file_id { READING_FIRMWARE = 1, + READING_MODULE, READING_MAX_ID }; diff --git a/include/linux/ima.h b/include/linux/ima.h index 6adcaea8101c..e6516cbbe9bf 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -18,7 +18,6 @@ extern int ima_bprm_check(struct linux_binprm *bprm); extern int ima_file_check(struct file *file, int mask, int opened); extern void ima_file_free(struct file *file); extern int ima_file_mmap(struct file *file, unsigned long prot); -extern int ima_module_check(struct file *file); extern int ima_read_file(struct file *file, enum kernel_read_file_id id); extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); @@ -44,11 +43,6 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot) return 0; } -static inline int ima_module_check(struct file *file) -{ - return 0; -} - static inline int ima_read_file(struct file *file, enum kernel_read_file_id id) { return 0; diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index d32b7bd13635..cdee11cbcdf1 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -546,12 +546,6 @@ * userspace to load a kernel module with the given name. * @kmod_name name of the module requested by the kernel * Return 0 if successful. - * @kernel_module_from_file: - * Load a kernel module from userspace. - * @file contains the file structure pointing to the file containing - * the kernel module to load. If the module is being loaded from a blob, - * this argument will be NULL. - * Return 0 if permission is granted. * @kernel_read_file: * Read a file specified by userspace. * @file contains the file structure pointing to the file being read @@ -1725,7 +1719,6 @@ struct security_hook_heads { struct list_head kernel_read_file; struct list_head kernel_post_read_file; struct list_head kernel_module_request; - struct list_head kernel_module_from_file; struct list_head task_fix_setuid; struct list_head task_setpgid; struct list_head task_getpgid; diff --git a/include/linux/security.h b/include/linux/security.h index 071fb747fdbb..157f0cb1e4d2 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -859,11 +859,6 @@ static inline int security_kernel_module_request(char *kmod_name) return 0; } -static inline int security_kernel_module_from_file(struct file *file) -{ - return 0; -} - static inline int security_kernel_read_file(struct file *file, enum kernel_read_file_id id) { diff --git a/kernel/module.c b/kernel/module.c index 8358f4697c0c..955410928696 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2654,7 +2654,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len, if (info->len < sizeof(*(info->hdr))) return -ENOEXEC; - err = security_kernel_module_from_file(NULL); + err = security_kernel_read_file(NULL, READING_MODULE); if (err) return err; @@ -2672,63 +2672,6 @@ static int copy_module_from_user(const void __user *umod, unsigned long len, return 0; } -/* Sets info->hdr and info->len. */ -static int copy_module_from_fd(int fd, struct load_info *info) -{ - struct fd f = fdget(fd); - int err; - struct kstat stat; - loff_t pos; - ssize_t bytes = 0; - - if (!f.file) - return -ENOEXEC; - - err = security_kernel_module_from_file(f.file); - if (err) - goto out; - - err = vfs_getattr(&f.file->f_path, &stat); - if (err) - goto out; - - if (stat.size > INT_MAX) { - err = -EFBIG; - goto out; - } - - /* Don't hand 0 to vmalloc, it whines. */ - if (stat.size == 0) { - err = -EINVAL; - goto out; - } - - info->hdr = vmalloc(stat.size); - if (!info->hdr) { - err = -ENOMEM; - goto out; - } - - pos = 0; - while (pos < stat.size) { - bytes = kernel_read(f.file, pos, (char *)(info->hdr) + pos, - stat.size - pos); - if (bytes < 0) { - vfree(info->hdr); - err = bytes; - goto out; - } - if (bytes == 0) - break; - pos += bytes; - } - info->len = pos; - -out: - fdput(f); - return err; -} - static void free_copy(struct load_info *info) { vfree(info->hdr); @@ -3589,8 +3532,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod, SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags) { - int err; struct load_info info = { }; + loff_t size; + void *hdr; + int err; err = may_init_module(); if (err) @@ -3602,9 +3547,12 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags) |MODULE_INIT_IGNORE_VERMAGIC)) return -EINVAL; - err = copy_module_from_fd(fd, &info); + err = kernel_read_file_from_fd(fd, &hdr, &size, INT_MAX, + READING_MODULE); if (err) return err; + info.hdr = hdr; + info.len = size; return load_module(&info, uargs, flags); } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index bbb80df28fb1..5da0b9c00072 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -315,28 +315,6 @@ int ima_file_check(struct file *file, int mask, int opened) } EXPORT_SYMBOL_GPL(ima_file_check); -/** - * ima_module_check - based on policy, collect/store/appraise measurement. - * @file: pointer to the file to be measured/appraised - * - * Measure/appraise kernel modules based on policy. - * - * On success return 0. On integrity appraisal error, assuming the file - * is in policy and IMA-appraisal is in enforcing mode, return -EACCES. - */ -int ima_module_check(struct file *file) -{ - if (!file) { -#ifndef CONFIG_MODULE_SIG_FORCE - if ((ima_appraise & IMA_APPRAISE_MODULES) && - (ima_appraise & IMA_APPRAISE_ENFORCE)) - return -EACCES; /* INTEGRITY_UNKNOWN */ -#endif - return 0; /* We rely on module signature checking */ - } - return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0); -} - /** * ima_read_file - pre-measure/appraise hook decision based on policy * @file: pointer to the file to be measured/appraised/audit @@ -350,6 +328,14 @@ int ima_module_check(struct file *file) */ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) { + if (!file && read_id == READING_MODULE) { +#ifndef CONFIG_MODULE_SIG_FORCE + if ((ima_appraise & IMA_APPRAISE_MODULES) && + (ima_appraise & IMA_APPRAISE_ENFORCE)) + return -EACCES; /* INTEGRITY_UNKNOWN */ +#endif + return 0; /* We rely on module signature checking */ + } return 0; } @@ -378,6 +364,9 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, return 0; } + if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */ + return 0; + if (!file || !buf || size == 0) { /* should never happen */ if (ima_appraise & IMA_APPRAISE_ENFORCE) return -EACCES; @@ -386,6 +375,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, if (read_id == READING_FIRMWARE) func = FIRMWARE_CHECK; + else if (read_id == READING_MODULE) + func = MODULE_CHECK; return process_measurement(file, buf, size, MAY_READ, func, 0); } diff --git a/security/security.c b/security/security.c index 8e699f98a600..3644b0344d29 100644 --- a/security/security.c +++ b/security/security.c @@ -889,16 +889,6 @@ int security_kernel_module_request(char *kmod_name) return call_int_hook(kernel_module_request, 0, kmod_name); } -int security_kernel_module_from_file(struct file *file) -{ - int ret; - - ret = call_int_hook(kernel_module_from_file, 0, file); - if (ret) - return ret; - return ima_module_check(file); -} - int security_kernel_read_file(struct file *file, enum kernel_read_file_id id) { int ret; @@ -1705,8 +1695,6 @@ struct security_hook_heads security_hook_heads = { LIST_HEAD_INIT(security_hook_heads.kernel_create_files_as), .kernel_module_request = LIST_HEAD_INIT(security_hook_heads.kernel_module_request), - .kernel_module_from_file = - LIST_HEAD_INIT(security_hook_heads.kernel_module_from_file), .kernel_read_file = LIST_HEAD_INIT(security_hook_heads.kernel_read_file), .kernel_post_read_file = -- cgit v1.2.3-70-g09d2