From 9a8d289fbcb7dfd1fc74959e9930b406e76b2002 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 28 Jul 2014 07:59:49 -0400 Subject: ima: fix ima_alloc_atfm() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The patch 3bcced39ea7d: "ima: use ahash API for file hash calculation" from Feb 26, 2014, leads to the following static checker warning: security/integrity/ima/ima_crypto.c:204 ima_alloc_atfm()          error: buffer overflow 'hash_algo_name' 17 <= 17 Unlike shash tfm memory, which is allocated on initialization, the ahash tfm memory allocation is deferred until needed. This patch fixes the case where ima_ahash_tfm has not yet been allocated and the file's signature/hash xattr contains an invalid hash algorithm. Although we can not verify the xattr, we still need to measure the file. Use the default IMA hash algorithm. Changelog: - set valid algo before testing tfm - based on Dmitry's comment Reported-by: Dan Carpenter Signed-off-by: Mimi Zohar Signed-off-by: Dmitry Kasatkin --- security/integrity/ima/ima_crypto.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 0bd7328..2d103dc 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -200,7 +200,10 @@ static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo) struct crypto_ahash *tfm = ima_ahash_tfm; int rc; - if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) { + if (algo < 0 || algo >= HASH_ALGO__LAST) + algo = ima_hash_algo; + + if (algo != ima_hash_algo || !tfm) { tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0); if (!IS_ERR(tfm)) { if (algo == ima_hash_algo) -- cgit v1.1 From 23c19e2ca736722a9523b64b07cda7efab7b6c57 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 15 Aug 2014 13:28:52 +0300 Subject: ima: prevent buffer overflow in ima_alloc_tfm() This patch fixes the case where the file's signature/hash xattr contains an invalid hash algorithm. Although we can not verify the xattr, we still need to measure the file. Use the default IMA hash algorithm. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 2d103dc..1178b30 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -116,7 +116,10 @@ static struct crypto_shash *ima_alloc_tfm(enum hash_algo algo) struct crypto_shash *tfm = ima_shash_tfm; int rc; - if (algo != ima_hash_algo && algo < HASH_ALGO__LAST) { + if (algo < 0 || algo >= HASH_ALGO__LAST) + algo = ima_hash_algo; + + if (algo != ima_hash_algo) { tfm = crypto_alloc_shash(hash_algo_name[algo], 0, 0); if (IS_ERR(tfm)) { rc = PTR_ERR(tfm); -- cgit v1.1 From 27cd1fc3ae5374a4a86662c67033f15ef27b2461 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Mon, 23 Jun 2014 20:32:56 +0300 Subject: ima: fix fallback to use new_sync_read() 3.16 commit aad4f8bb42af06371aa0e85bf0cd9d52c0494985 'switch simple generic_file_aio_read() users to ->read_iter()' replaced ->aio_read with ->read_iter in most of the file systems and introduced new_sync_read() as a replacement for do_sync_read(). Most of file systems set '->read' and ima_kernel_read is not affected. When ->read is not set, this patch adopts fallback call changes from the vfs_read. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar Cc: 3.16+ --- security/integrity/ima/ima_crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 1178b30..3b26472 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -80,19 +80,19 @@ static int ima_kernel_read(struct file *file, loff_t offset, { mm_segment_t old_fs; char __user *buf = addr; - ssize_t ret; + ssize_t ret = -EINVAL; if (!(file->f_mode & FMODE_READ)) return -EBADF; - if (!file->f_op->read && !file->f_op->aio_read) - return -EINVAL; old_fs = get_fs(); set_fs(get_ds()); if (file->f_op->read) ret = file->f_op->read(file, buf, count, &offset); - else + else if (file->f_op->aio_read) ret = do_sync_read(file, buf, count, &offset); + else if (file->f_op->read_iter) + ret = new_sync_read(file, buf, count, &offset); set_fs(old_fs); return ret; } -- cgit v1.1 From e7d021e28328e0cc47b21cb9c6d8885326b0c2f5 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 15 Aug 2014 14:09:19 +0300 Subject: evm: fix checkpatch warnings This patch fixes checkpatch 'return' warnings introduced with commit 9819cf2 "checkpatch: warn on unnecessary void function return statements". Use scripts/checkpatch.pl --file security/integrity/evm/evm_main.c to produce the warnings. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 3bcb80d..fb71f55 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -352,7 +352,6 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, return; evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); - return; } /** @@ -372,7 +371,6 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) mutex_lock(&inode->i_mutex); evm_update_evmxattr(dentry, xattr_name, NULL, 0); mutex_unlock(&inode->i_mutex); - return; } /** @@ -414,7 +412,6 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid) if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) evm_update_evmxattr(dentry, NULL, NULL, 0); - return; } /* -- cgit v1.1 From 1f1009791b2e81f106d4809007720495ba3ed90c Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 15 Aug 2014 13:49:22 +0300 Subject: evm: prevent passing integrity check if xattr read fails This patch fixes a bug, where evm_verify_hmac() returns INTEGRITY_PASS if inode->i_op->getxattr() returns an error in evm_find_protected_xattrs. Signed-off-by: Dmitry Kasatkin --- security/integrity/evm/evm_main.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index fb71f55..4022012 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -126,14 +126,15 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, rc = vfs_getxattr_alloc(dentry, XATTR_NAME_EVM, (char **)&xattr_data, 0, GFP_NOFS); if (rc <= 0) { - if (rc == 0) - evm_status = INTEGRITY_FAIL; /* empty */ - else if (rc == -ENODATA) { + evm_status = INTEGRITY_FAIL; + if (rc == -ENODATA) { rc = evm_find_protected_xattrs(dentry); if (rc > 0) evm_status = INTEGRITY_NOLABEL; else if (rc == 0) evm_status = INTEGRITY_NOXATTRS; /* new file */ + } else if (rc == -EOPNOTSUPP) { + evm_status = INTEGRITY_UNKNOWN; } goto out; } -- cgit v1.1 From b151d6b00bbb798c58f2f21305e7d43fa763f34f Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 27 Jun 2014 18:04:27 +0300 Subject: ima: provide flag to identify new empty files On ima_file_free(), newly created empty files are not labeled with an initial security.ima value, because the iversion did not change. Commit dff6efc "fs: fix iversion handling" introduced a change in iversion behavior. To verify this change use the shell command: $ (exec >foo) $ getfattr -h -e hex -d -m security foo This patch defines the IMA_NEW_FILE flag. The flag is initially set, when IMA detects that a new file is created, and subsequently checked on the ima_file_free() hook to set the initial security.ima value. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar Cc: 3.14+ --- security/integrity/ima/ima_appraise.c | 7 +++++-- security/integrity/ima/ima_main.c | 12 +++++++----- security/integrity/integrity.h | 1 + 3 files changed, 13 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 86bfd5c..a4605d6 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -202,8 +202,11 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, goto out; cause = "missing-hash"; - status = - (inode->i_size == 0) ? INTEGRITY_PASS : INTEGRITY_NOLABEL; + status = INTEGRITY_NOLABEL; + if (inode->i_size == 0) { + iint->flags |= IMA_NEW_FILE; + status = INTEGRITY_PASS; + } goto out; } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 2917f98..0a2298f 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -124,11 +124,13 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint, return; mutex_lock(&inode->i_mutex); - if (atomic_read(&inode->i_writecount) == 1 && - iint->version != inode->i_version) { - iint->flags &= ~IMA_DONE_MASK; - if (iint->flags & IMA_APPRAISE) - ima_update_xattr(iint, file); + if (atomic_read(&inode->i_writecount) == 1) { + if ((iint->version != inode->i_version) || + (iint->flags & IMA_NEW_FILE)) { + iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE); + if (iint->flags & IMA_APPRAISE) + ima_update_xattr(iint, file); + } } mutex_unlock(&inode->i_mutex); } diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 19b8e31..904e68a 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -31,6 +31,7 @@ #define IMA_DIGSIG 0x01000000 #define IMA_DIGSIG_REQUIRED 0x02000000 #define IMA_PERMIT_DIRECTIO 0x04000000 +#define IMA_NEW_FILE 0x08000000 #define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \ IMA_APPRAISE_SUBMASK) -- cgit v1.1 From 3dcbad52cf18c3c379e96b992d22815439ebbe53 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 2 Sep 2014 16:31:43 +0300 Subject: evm: properly handle INTEGRITY_NOXATTRS EVM status Unless an LSM labels a file during d_instantiate(), newly created files are not labeled with an initial security.evm xattr, until the file closes. EVM, before allowing a protected, security xattr to be written, verifies the existing 'security.evm' value is good. For newly created files without a security.evm label, this verification prevents writing any protected, security xattrs, until the file closes. Following is the example when this happens: fd = open("foo", O_CREAT | O_WRONLY, 0644); setxattr("foo", "security.SMACK64", value, sizeof(value), 0); close(fd); While INTEGRITY_NOXATTRS status is handled in other places, such as evm_inode_setattr(), it does not handle it in all cases in evm_protect_xattr(). By limiting the use of INTEGRITY_NOXATTRS to newly created files, we can now allow setting "protected" xattrs. Changelog: - limit the use of INTEGRITY_NOXATTRS to IMA identified new files Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar Cc: 3.14+ --- security/integrity/evm/evm_main.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 4022012..9685af3 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -285,6 +285,13 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name, goto out; } evm_status = evm_verify_current_integrity(dentry); + if (evm_status == INTEGRITY_NOXATTRS) { + struct integrity_iint_cache *iint; + + iint = integrity_iint_find(dentry->d_inode); + if (iint && (iint->flags & IMA_NEW_FILE)) + return 0; + } out: if (evm_status != INTEGRITY_PASS) integrity_audit_msg(AUDIT_INTEGRITY_METADATA, dentry->d_inode, -- cgit v1.1 From 3034a146820c26fe6da66a45f6340fe87fe0983a Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 27 Jun 2014 18:15:44 +0300 Subject: ima: pass 'opened' flag to identify newly created files Empty files and missing xattrs do not guarantee that a file was just created. This patch passes FILE_CREATED flag to IMA to reliably identify new files. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar Cc: 3.14+ --- security/integrity/ima/ima.h | 4 ++-- security/integrity/ima/ima_appraise.c | 4 ++-- security/integrity/ima/ima_main.c | 16 ++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 57da4bd..0fb456c 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -177,7 +177,7 @@ void ima_delete_rules(void); int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len); + int xattr_len, int opened); int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, @@ -193,7 +193,7 @@ static inline int ima_appraise_measurement(int func, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len) + int xattr_len, int opened) { return INTEGRITY_UNKNOWN; } diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index a4605d6..225fd94 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -183,7 +183,7 @@ int ima_read_xattr(struct dentry *dentry, int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len) + int xattr_len, int opened) { static const char op[] = "appraise_data"; char *cause = "unknown"; @@ -203,7 +203,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, cause = "missing-hash"; status = INTEGRITY_NOLABEL; - if (inode->i_size == 0) { + if (opened & FILE_CREATED) { iint->flags |= IMA_NEW_FILE; status = INTEGRITY_PASS; } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 0a2298f..f82cf9b 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -157,7 +157,7 @@ void ima_file_free(struct file *file) } static int process_measurement(struct file *file, const char *filename, - int mask, int function) + int mask, int function, int opened) { struct inode *inode = file_inode(file); struct integrity_iint_cache *iint; @@ -226,7 +226,7 @@ static int process_measurement(struct file *file, const char *filename, xattr_value, xattr_len); if (action & IMA_APPRAISE_SUBMASK) rc = ima_appraise_measurement(_func, iint, file, pathname, - xattr_value, xattr_len); + xattr_value, xattr_len, opened); if (action & IMA_AUDIT) ima_audit_measurement(iint, pathname); kfree(pathbuf); @@ -255,7 +255,7 @@ out: int ima_file_mmap(struct file *file, unsigned long prot) { if (file && (prot & PROT_EXEC)) - return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK); + return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK, 0); return 0; } @@ -277,7 +277,7 @@ int ima_bprm_check(struct linux_binprm *bprm) return process_measurement(bprm->file, (strcmp(bprm->filename, bprm->interp) == 0) ? bprm->filename : bprm->interp, - MAY_EXEC, BPRM_CHECK); + MAY_EXEC, BPRM_CHECK, 0); } /** @@ -290,12 +290,12 @@ int ima_bprm_check(struct linux_binprm *bprm) * 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_file_check(struct file *file, int mask) +int ima_file_check(struct file *file, int mask, int opened) { ima_rdwr_violation_check(file); return process_measurement(file, NULL, mask & (MAY_READ | MAY_WRITE | MAY_EXEC), - FILE_CHECK); + FILE_CHECK, opened); } EXPORT_SYMBOL_GPL(ima_file_check); @@ -318,7 +318,7 @@ int ima_module_check(struct file *file) #endif return 0; /* We rely on module signature checking */ } - return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK); + return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK, 0); } int ima_fw_from_file(struct file *file, char *buf, size_t size) @@ -329,7 +329,7 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size) return -EACCES; /* INTEGRITY_UNKNOWN */ return 0; } - return process_measurement(file, NULL, MAY_EXEC, FIRMWARE_CHECK); + return process_measurement(file, NULL, MAY_EXEC, FIRMWARE_CHECK, 0); } static int __init init_ima(void) -- cgit v1.1 From d9a2e5d788d39f7593e2af5e1a365e2b9300679f Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 2 Jul 2014 15:12:26 +0300 Subject: integrity: prevent flooding with 'Request for unknown key' If file has IMA signature, IMA in enforce mode, but key is missing then file access is blocked and single error message is printed. If IMA appraisal is enabled in fix mode, then system runs as usual but might produce tons of 'Request for unknown key' messages. This patch switches 'pr_warn' to 'pr_err_ratelimited'. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/digsig_asymmetric.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index 9eae480..37e0d98 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include +#include #include #include #include @@ -45,8 +46,8 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid) } if (IS_ERR(key)) { - pr_warn("Request for unknown key '%s' err %ld\n", - name, PTR_ERR(key)); + pr_err_ratelimited("Request for unknown key '%s' err %ld\n", + name, PTR_ERR(key)); switch (PTR_ERR(key)) { /* Hide some search errors */ case -EACCES: -- cgit v1.1 From 65d98f3be25f7ee96af655f97e153d3d8d3d8ef9 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 19 Aug 2014 14:56:18 +0300 Subject: integrity: remove declaration of non-existing functions Commit f381c27 "integrity: move ima inode integrity data management" (re)moved few functions but left their declarations in header files. This patch removes them and also removes duplicated declaration of integrity_iint_find(). Commit c7de7ad "ima: remove unused cleanup functions". This patch removes these definitions as well. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 9 --------- security/integrity/integrity.h | 1 - 2 files changed, 10 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 0fb456c..c6990a7 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -90,10 +90,7 @@ extern struct list_head ima_measurements; /* list of all measurements */ /* Internal IMA function definitions */ int ima_init(void); -void ima_cleanup(void); int ima_fs_init(void); -void ima_fs_cleanup(void); -int ima_inode_alloc(struct inode *inode); int ima_add_template_entry(struct ima_template_entry *entry, int violation, const char *op, struct inode *inode, const unsigned char *filename); @@ -151,12 +148,6 @@ int ima_store_template(struct ima_template_entry *entry, int violation, void ima_free_template_entry(struct ima_template_entry *entry); const char *ima_d_path(struct path *path, char **pathbuf); -/* rbtree tree calls to lookup, insert, delete - * integrity data associated with an inode. - */ -struct integrity_iint_cache *integrity_iint_insert(struct inode *inode); -struct integrity_iint_cache *integrity_iint_find(struct inode *inode); - /* IMA policy related functions */ enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, FIRMWARE_CHECK, POST_SETATTR }; diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 904e68a..c0379d1 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -117,7 +117,6 @@ struct integrity_iint_cache { /* rbtree tree calls to lookup, insert, delete * integrity data associated with an inode. */ -struct integrity_iint_cache *integrity_iint_insert(struct inode *inode); struct integrity_iint_cache *integrity_iint_find(struct inode *inode); #define INTEGRITY_KEYRING_EVM 0 -- cgit v1.1 From f68c05f4d2d4e19c40f4ac1e769cc0a2f9f544a0 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 22 Aug 2014 09:43:55 +0300 Subject: ima: simplify conditional statement to improve performance Precede bit testing before string comparison makes code faster. Also refactor statement as a single line pointer assignment. Logic is following: we set 'xattr_ptr' to read xattr value when we will do appraisal or in any case when measurement template is other than 'ima'. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f82cf9b..f7b85bf 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -206,10 +206,8 @@ static int process_measurement(struct file *file, const char *filename, } template_desc = ima_template_desc_current(); - if (strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) == 0) { - if (action & IMA_APPRAISE_SUBMASK) - xattr_ptr = &xattr_value; - } else + if ((action & IMA_APPRAISE_SUBMASK) || + strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) xattr_ptr = &xattr_value; rc = ima_collect_measurement(iint, file, xattr_ptr, &xattr_len); -- cgit v1.1 From 3a8a2eadc4946ce3af39b3447c32532324538f75 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 3 Sep 2014 10:19:57 +0300 Subject: ima: remove unnecessary extra variable 'function' variable value can be changed instead of allocating extra '_func' variable. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f7b85bf..aaf5552 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -164,7 +164,7 @@ static int process_measurement(struct file *file, const char *filename, struct ima_template_desc *template_desc; char *pathbuf = NULL; const char *pathname = NULL; - int rc = -ENOMEM, action, must_appraise, _func; + int rc = -ENOMEM, action, must_appraise; struct evm_ima_xattr_data *xattr_value = NULL, **xattr_ptr = NULL; int xattr_len = 0; @@ -182,7 +182,8 @@ static int process_measurement(struct file *file, const char *filename, must_appraise = action & IMA_APPRAISE; /* Is the appraise rule hook specific? */ - _func = (action & IMA_FILE_APPRAISE) ? FILE_CHECK : function; + if (action & IMA_FILE_APPRAISE) + function = FILE_CHECK; mutex_lock(&inode->i_mutex); @@ -201,7 +202,7 @@ static int process_measurement(struct file *file, const char *filename, /* Nothing to do, just return existing appraised status */ if (!action) { if (must_appraise) - rc = ima_get_cache_status(iint, _func); + rc = ima_get_cache_status(iint, function); goto out_digsig; } @@ -223,7 +224,7 @@ static int process_measurement(struct file *file, const char *filename, ima_store_measurement(iint, file, pathname, xattr_value, xattr_len); if (action & IMA_APPRAISE_SUBMASK) - rc = ima_appraise_measurement(_func, iint, file, pathname, + rc = ima_appraise_measurement(function, iint, file, pathname, xattr_value, xattr_len, opened); if (action & IMA_AUDIT) ima_audit_measurement(iint, pathname); -- cgit v1.1 From e4a9c5196566bd47ac92f6e5ef7f48412ded7176 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 3 Sep 2014 10:19:58 +0300 Subject: ima: add missing '__init' keywords Add missing keywords to the function definition to cleanup to discard initialization code. Signed-off-by: Dmitry Kasatkin Reviewed-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 2 -- security/integrity/ima/ima_crypto.c | 2 +- security/integrity/ima/ima_template.c | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index c6990a7..8e4bb88 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -107,8 +107,6 @@ void ima_print_digest(struct seq_file *m, u8 *digest, int size); struct ima_template_desc *ima_template_desc_current(void); int ima_init_template(void); -int ima_init_template(void); - /* * used to protect h_table and sha_table */ diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 3b26472..d34e7df 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -97,7 +97,7 @@ static int ima_kernel_read(struct file *file, loff_t offset, return ret; } -int ima_init_crypto(void) +int __init ima_init_crypto(void) { long rc; diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index a076a96..f682606 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -152,7 +152,7 @@ out: return result; } -static int init_defined_templates(void) +static int __init init_defined_templates(void) { int i = 0; int result = 0; @@ -178,7 +178,7 @@ struct ima_template_desc *ima_template_desc_current(void) return ima_template; } -int ima_init_template(void) +int __init ima_init_template(void) { int result; -- cgit v1.1 From 86f2bc024966d962d4d7575468e226e2269d198c Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 20 Aug 2014 12:37:57 +0300 Subject: ima: remove unnecessary appraisal test ima_get_action() sets the "action" flags based on policy. Before collecting, measuring, appraising, or auditing the file, the "action" flag is updated based on the cached iint->flags. This patch removes the subsequent unnecessary appraisal test in ima_appraise_measurement(). Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 225fd94..013ec3f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -192,8 +192,6 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, enum integrity_status status = INTEGRITY_UNKNOWN; int rc = xattr_len, hash_start = 0; - if (!ima_appraise) - return 0; if (!inode->i_op->getxattr) return INTEGRITY_UNKNOWN; -- cgit v1.1 From 17f4bad3abc7c09f42987d89ccccab02c03455a9 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 19 Aug 2014 16:48:39 +0300 Subject: ima: remove usage of filename parameter In all cases except ima_bprm_check() the filename was not defined and ima_d_path() was used to find the full path. Unfortunately, the bprm filename is a relative pathname (eg. .//filename). ima_bprm_check() selects between bprm->interp and bprm->filename. The following dump demonstrates the differences between using filename and interp. bprm->filename filename: ./foo.sh, pathname: /root/bin/foo.sh filename: ./foo.sh, pathname: /bin/dash bprm->interp filename: ./foo.sh, pathname: /root/bin/foo.sh filename: /bin/sh, pathname: /bin/dash In both cases the pathnames are currently the same. This patch removes usage of filename and interp in favor of d_absolute_path. Changes v3: - 11 extra bytes for "deleted" not needed (Mimi) - purpose "replace relative bprm filename with full pathname" (Mimi) Changes v2: - use d_absolute_path() instead of d_path to work in chroot environments. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 5 ++--- security/integrity/ima/ima_main.c | 19 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index d9cd5ce..65c41a9 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -330,10 +330,9 @@ const char *ima_d_path(struct path *path, char **pathbuf) { char *pathname = NULL; - /* We will allow 11 spaces for ' (deleted)' to be appended */ - *pathbuf = kmalloc(PATH_MAX + 11, GFP_KERNEL); + *pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); if (*pathbuf) { - pathname = d_path(path, *pathbuf, PATH_MAX + 11); + pathname = d_absolute_path(path, *pathbuf, PATH_MAX); if (IS_ERR(pathname)) { kfree(*pathbuf); *pathbuf = NULL; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index aaf5552..673a37e 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -156,8 +156,8 @@ void ima_file_free(struct file *file) ima_check_last_writer(iint, inode, file); } -static int process_measurement(struct file *file, const char *filename, - int mask, int function, int opened) +static int process_measurement(struct file *file, int mask, int function, + int opened) { struct inode *inode = file_inode(file); struct integrity_iint_cache *iint; @@ -218,7 +218,7 @@ static int process_measurement(struct file *file, const char *filename, goto out_digsig; } - pathname = filename ?: ima_d_path(&file->f_path, &pathbuf); + pathname = ima_d_path(&file->f_path, &pathbuf); if (action & IMA_MEASURE) ima_store_measurement(iint, file, pathname, @@ -254,7 +254,7 @@ out: int ima_file_mmap(struct file *file, unsigned long prot) { if (file && (prot & PROT_EXEC)) - return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK, 0); + return process_measurement(file, MAY_EXEC, MMAP_CHECK, 0); return 0; } @@ -273,10 +273,7 @@ int ima_file_mmap(struct file *file, unsigned long prot) */ int ima_bprm_check(struct linux_binprm *bprm) { - return process_measurement(bprm->file, - (strcmp(bprm->filename, bprm->interp) == 0) ? - bprm->filename : bprm->interp, - MAY_EXEC, BPRM_CHECK, 0); + return process_measurement(bprm->file, MAY_EXEC, BPRM_CHECK, 0); } /** @@ -292,7 +289,7 @@ int ima_bprm_check(struct linux_binprm *bprm) int ima_file_check(struct file *file, int mask, int opened) { ima_rdwr_violation_check(file); - return process_measurement(file, NULL, + return process_measurement(file, mask & (MAY_READ | MAY_WRITE | MAY_EXEC), FILE_CHECK, opened); } @@ -317,7 +314,7 @@ int ima_module_check(struct file *file) #endif return 0; /* We rely on module signature checking */ } - return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK, 0); + return process_measurement(file, MAY_EXEC, MODULE_CHECK, 0); } int ima_fw_from_file(struct file *file, char *buf, size_t size) @@ -328,7 +325,7 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size) return -EACCES; /* INTEGRITY_UNKNOWN */ return 0; } - return process_measurement(file, NULL, MAY_EXEC, FIRMWARE_CHECK, 0); + return process_measurement(file, MAY_EXEC, FIRMWARE_CHECK, 0); } static int __init init_ima(void) -- cgit v1.1 From b4148db51720a6b25a981ea72185312d4b6634fe Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Thu, 8 May 2014 11:23:53 +0300 Subject: ima: initialize only required template IMA uses only one template. This patch initializes only required template to avoid unnecessary memory allocations. Signed-off-by: Dmitry Kasatkin Reviewed-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index f682606..e854862 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -152,24 +152,6 @@ out: return result; } -static int __init init_defined_templates(void) -{ - int i = 0; - int result = 0; - - /* Init defined templates. */ - for (i = 0; i < ARRAY_SIZE(defined_templates); i++) { - struct ima_template_desc *template = &defined_templates[i]; - - result = template_desc_init_fields(template->fmt, - &(template->fields), - &(template->num_fields)); - if (result < 0) - return result; - } - return result; -} - struct ima_template_desc *ima_template_desc_current(void) { if (!ima_template) @@ -180,11 +162,9 @@ struct ima_template_desc *ima_template_desc_current(void) int __init ima_init_template(void) { - int result; - - result = init_defined_templates(); - if (result < 0) - return result; + struct ima_template_desc *template = ima_template_desc_current(); - return 0; + return template_desc_init_fields(template->fmt, + &(template->fields), + &(template->num_fields)); } -- cgit v1.1 From 1ae8f41c23ff6a75c1432faed7281aea5ce7c236 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Thu, 17 Apr 2014 14:41:06 +0300 Subject: integrity: move asymmetric keys config option For better visual appearance it is better to co-locate asymmetric key options together with signature support. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/Kconfig | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 245c6d9..f79d853 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -17,6 +17,18 @@ config INTEGRITY_SIGNATURE This is useful for evm and module keyrings, when keys are usually only added from initramfs. +config INTEGRITY_ASYMMETRIC_KEYS + boolean "Enable asymmetric keys support" + depends on INTEGRITY_SIGNATURE + default n + select ASYMMETRIC_KEY_TYPE + select ASYMMETRIC_PUBLIC_KEY_SUBTYPE + select PUBLIC_KEY_ALGO_RSA + select X509_CERTIFICATE_PARSER + help + This option enables digital signature verification using + asymmetric keys. + config INTEGRITY_AUDIT bool "Enables integrity auditing support " depends on INTEGRITY && AUDIT @@ -32,17 +44,5 @@ config INTEGRITY_AUDIT be enabled by specifying 'integrity_audit=1' on the kernel command line. -config INTEGRITY_ASYMMETRIC_KEYS - boolean "Enable asymmetric keys support" - depends on INTEGRITY_SIGNATURE - default n - select ASYMMETRIC_KEY_TYPE - select ASYMMETRIC_PUBLIC_KEY_SUBTYPE - select PUBLIC_KEY_ALGO_RSA - select X509_CERTIFICATE_PARSER - help - This option enables digital signature verification using - asymmetric keys. - source security/integrity/ima/Kconfig source security/integrity/evm/Kconfig -- cgit v1.1 From 7ef84e65ecc60289281e8e7e83a8bb6a97d7df5c Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Thu, 17 Apr 2014 15:07:15 +0300 Subject: integrity: base integrity subsystem kconfig options on integrity The integrity subsystem has lots of options and takes more than half of the security menu. This patch consolidates the options under "integrity", which are hidden if not enabled. This change does not affect existing configurations. Re-configuration is not needed. Changes v4: - no need to change "integrity subsystem" to menuconfig as options are hidden, when not enabled. (Mimi) - add INTEGRITY Kconfig help description Changes v3: - dependency to INTEGRITY removed when behind 'if INTEGRITY' Changes v2: - previous patch moved integrity out of the 'security' menu. This version keeps integrity as a security option (Mimi). Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/Kconfig | 22 ++++++++++++++++++---- security/integrity/evm/Kconfig | 8 -------- security/integrity/ima/Kconfig | 2 -- 3 files changed, 18 insertions(+), 14 deletions(-) (limited to 'security') diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index f79d853..b76235a 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -1,11 +1,23 @@ # config INTEGRITY - def_bool y - depends on IMA || EVM + bool "Integrity subsystem" + depends on SECURITY + default y + help + This option enables the integrity subsystem, which is comprised + of a number of different components including the Integrity + Measurement Architecture (IMA), Extended Verification Module + (EVM), IMA-appraisal extension, digital signature verification + extension and audit measurement log support. + + Each of these components can be enabled/disabled separately. + Refer to the individual components for additional details. + +if INTEGRITY config INTEGRITY_SIGNATURE boolean "Digital signature verification using multiple keyrings" - depends on INTEGRITY && KEYS + depends on KEYS default n select SIGNATURE help @@ -31,7 +43,7 @@ config INTEGRITY_ASYMMETRIC_KEYS config INTEGRITY_AUDIT bool "Enables integrity auditing support " - depends on INTEGRITY && AUDIT + depends on AUDIT default y help In addition to enabling integrity auditing support, this @@ -46,3 +58,5 @@ config INTEGRITY_AUDIT source security/integrity/ima/Kconfig source security/integrity/evm/Kconfig + +endif # if INTEGRITY diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index d606f3d..df586fa 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -1,6 +1,5 @@ config EVM boolean "EVM support" - depends on SECURITY select KEYS select ENCRYPTED_KEYS select CRYPTO_HMAC @@ -12,10 +11,6 @@ config EVM If you are unsure how to answer this question, answer N. -if EVM - -menu "EVM options" - config EVM_ATTR_FSUUID bool "FSUUID (version 2)" default y @@ -47,6 +42,3 @@ config EVM_EXTRA_SMACK_XATTRS additional info to the calculation, requires existing EVM labeled file systems to be relabeled. -endmenu - -endif diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 08758fb..e099875 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -2,8 +2,6 @@ # config IMA bool "Integrity Measurement Architecture(IMA)" - depends on SECURITY - select INTEGRITY select SECURITYFS select CRYPTO select CRYPTO_HMAC -- cgit v1.1 From a2d61ed525f3458a913147fd02b1a5cf15e7551b Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 2 Jul 2014 15:42:19 +0300 Subject: integrity: make integrity files as 'integrity' module The kernel print macros use the KBUILD_MODNAME, which is initialized to the module name. The current integrity/Makefile makes every file as its own module, so pr_xxx messages are prefixed with the file name instead of the module. Similar to the evm/Makefile and ima/Makefile, this patch fixes the integrity/Makefile to use the single name 'integrity'. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/integrity/Makefile b/security/integrity/Makefile index 0793f48..8d1f4bf 100644 --- a/security/integrity/Makefile +++ b/security/integrity/Makefile @@ -3,11 +3,11 @@ # obj-$(CONFIG_INTEGRITY) += integrity.o -obj-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o -obj-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o -obj-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o integrity-y := iint.o +integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o +integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o +integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o subdir-$(CONFIG_IMA) += ima obj-$(CONFIG_IMA) += ima/ -- cgit v1.1