From 2fe5d6def1672ae6635dd71867bf36dcfaa7434b Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 13 Feb 2012 10:15:05 -0500 Subject: ima: integrity appraisal extension IMA currently maintains an integrity measurement list used to assert the integrity of the running system to a third party. The IMA-appraisal extension adds local integrity validation and enforcement of the measurement against a "good" value stored as an extended attribute 'security.ima'. The initial methods for validating 'security.ima' are hashed based, which provides file data integrity, and digital signature based, which in addition to providing file data integrity, provides authenticity. This patch creates and maintains the 'security.ima' xattr, containing the file data hash measurement. Protection of the xattr is provided by EVM, if enabled and configured. Based on policy, IMA calls evm_verifyxattr() to verify a file's metadata integrity and, assuming success, compares the file's current hash value with the one stored as an extended attribute in 'security.ima'. Changelov v4: - changed iint cache flags to hex values Changelog v3: - change appraisal default for filesystems without xattr support to fail Changelog v2: - fix audit msg 'res' value - removed unused 'ima_appraise=' values Changelog v1: - removed unused iint mutex (Dmitry Kasatkin) - setattr hook must not reset appraised (Dmitry Kasatkin) - evm_verifyxattr() now differentiates between no 'security.evm' xattr (INTEGRITY_NOLABEL) and no EVM 'protected' xattrs included in the 'security.evm' (INTEGRITY_NOXATTRS). - replace hash_status with ima_status (Dmitry Kasatkin) - re-initialize slab element ima_status on free (Dmitry Kasatkin) - include 'security.ima' in EVM if CONFIG_IMA_APPRAISE, not CONFIG_IMA - merged half "ima: ima_must_appraise_or_measure API change" (Dmitry Kasatkin) - removed unnecessary error variable in process_measurement() (Dmitry Kasatkin) - use ima_inode_post_setattr() stub function, if IMA_APPRAISE not configured (moved ima_inode_post_setattr() to ima_appraise.c) - make sure ima_collect_measurement() can read file Changelog: - add 'iint' to evm_verifyxattr() call (Dimitry Kasatkin) - fix the race condition between chmod, which takes the i_mutex and then iint->mutex, and ima_file_free() and process_measurement(), which take the locks in the reverse order, by eliminating iint->mutex. (Dmitry Kasatkin) - cleanup of ima_appraise_measurement() (Dmitry Kasatkin) - changes as a result of the iint not allocated for all regular files, but only for those measured/appraised. - don't try to appraise new/empty files - expanded ima_appraisal description in ima/Kconfig - IMA appraise definitions required even if IMA_APPRAISE not enabled - add return value to ima_must_appraise() stub - unconditionally set status = INTEGRITY_PASS *after* testing status, not before. (Found by Joe Perches) Signed-off-by: Mimi Zohar Signed-off-by: Dmitry Kasatkin --- include/linux/xattr.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include/linux') diff --git a/include/linux/xattr.h b/include/linux/xattr.h index e5d122031542..77a3e686d566 100644 --- a/include/linux/xattr.h +++ b/include/linux/xattr.h @@ -33,6 +33,9 @@ #define XATTR_EVM_SUFFIX "evm" #define XATTR_NAME_EVM XATTR_SECURITY_PREFIX XATTR_EVM_SUFFIX +#define XATTR_IMA_SUFFIX "ima" +#define XATTR_NAME_IMA XATTR_SECURITY_PREFIX XATTR_IMA_SUFFIX + #define XATTR_SELINUX_SUFFIX "selinux" #define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX -- cgit v1.2.3 From bf2276d10ce58ff44ab8857266a6718024496af6 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 19 Oct 2011 12:04:40 +0300 Subject: ima: allocating iint improvements With IMA-appraisal's removal of the iint mutex and taking the i_mutex instead, allocating the iint becomes a lot simplier, as we don't need to be concerned with two processes racing to allocate the iint. This patch cleans up and improves performance for allocating the iint. - removed redundant double i_mutex locking - combined iint allocation with tree search Changelog v2: - removed the rwlock/read_lock changes from this patch Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- include/linux/integrity.h | 7 +++--- security/integrity/iint.c | 45 +++++++++++++++++---------------------- security/integrity/ima/ima_main.c | 13 ++++------- 3 files changed, 27 insertions(+), 38 deletions(-) (limited to 'include/linux') diff --git a/include/linux/integrity.h b/include/linux/integrity.h index a0c41256cb92..66c5fe9550a5 100644 --- a/include/linux/integrity.h +++ b/include/linux/integrity.h @@ -22,13 +22,14 @@ enum integrity_status { /* List of EVM protected security xattrs */ #ifdef CONFIG_INTEGRITY -extern int integrity_inode_alloc(struct inode *inode); +extern struct integrity_iint_cache *integrity_inode_get(struct inode *inode); extern void integrity_inode_free(struct inode *inode); #else -static inline int integrity_inode_alloc(struct inode *inode) +static inline struct integrity_iint_cache * + integrity_inode_get(struct inode *inode) { - return 0; + return NULL; } static inline void integrity_inode_free(struct inode *inode) diff --git a/security/integrity/iint.c b/security/integrity/iint.c index e600986aa49f..c91a436e13ac 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -80,24 +80,26 @@ static void iint_free(struct integrity_iint_cache *iint) } /** - * integrity_inode_alloc - allocate an iint associated with an inode + * integrity_inode_get - find or allocate an iint associated with an inode * @inode: pointer to the inode + * @return: allocated iint + * + * Caller must lock i_mutex */ -int integrity_inode_alloc(struct inode *inode) +struct integrity_iint_cache *integrity_inode_get(struct inode *inode) { struct rb_node **p; - struct rb_node *new_node, *parent = NULL; - struct integrity_iint_cache *new_iint, *test_iint; - int rc; + struct rb_node *node, *parent = NULL; + struct integrity_iint_cache *iint, *test_iint; - new_iint = kmem_cache_alloc(iint_cache, GFP_NOFS); - if (!new_iint) - return -ENOMEM; + iint = integrity_iint_find(inode); + if (iint) + return iint; - new_iint->inode = inode; - new_node = &new_iint->rb_node; + iint = kmem_cache_alloc(iint_cache, GFP_NOFS); + if (!iint) + return NULL; - mutex_lock(&inode->i_mutex); /* i_flags */ spin_lock(&integrity_iint_lock); p = &integrity_iint_tree.rb_node; @@ -105,29 +107,20 @@ int integrity_inode_alloc(struct inode *inode) parent = *p; test_iint = rb_entry(parent, struct integrity_iint_cache, rb_node); - rc = -EEXIST; if (inode < test_iint->inode) p = &(*p)->rb_left; - else if (inode > test_iint->inode) - p = &(*p)->rb_right; else - goto out_err; + p = &(*p)->rb_right; } + iint->inode = inode; + node = &iint->rb_node; inode->i_flags |= S_IMA; - rb_link_node(new_node, parent, p); - rb_insert_color(new_node, &integrity_iint_tree); + rb_link_node(node, parent, p); + rb_insert_color(node, &integrity_iint_tree); spin_unlock(&integrity_iint_lock); - mutex_unlock(&inode->i_mutex); /* i_flags */ - - return 0; -out_err: - spin_unlock(&integrity_iint_lock); - mutex_unlock(&inode->i_mutex); /* i_flags */ - iint_free(new_iint); - - return rc; + return iint; } /** diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 6eb28d47e74b..df6521296051 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -162,19 +162,14 @@ static int process_measurement(struct file *file, const unsigned char *filename, if (!action) return 0; -retry: - iint = integrity_iint_find(inode); - if (!iint) { - rc = integrity_inode_alloc(inode); - if (!rc || rc == -EEXIST) - goto retry; - return rc; - } - must_appraise = action & IMA_APPRAISE; mutex_lock(&inode->i_mutex); + iint = integrity_inode_get(inode); + if (!iint) + goto out; + /* Determine if already appraised/measured based on bitmask * (IMA_MEASURE, IMA_MEASURED, IMA_APPRAISE, IMA_APPRAISED) */ iint->flags |= action; -- cgit v1.2.3 From 9957a5043e7b0b7361cdf48eea22b2900293e63a Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 9 Mar 2011 22:57:53 -0500 Subject: ima: add inode_post_setattr call Changing an inode's metadata may result in our not needing to appraise the file. In such cases, we must remove 'security.ima'. Changelog v1: - use ima_inode_post_setattr() stub function, if IMA_APPRAISE not configured Signed-off-by: Mimi Zohar Acked-by: Serge Hallyn Acked-by: Dmitry Kasatkin --- fs/attr.c | 2 ++ include/linux/ima.h | 10 ++++++++++ 2 files changed, 12 insertions(+) (limited to 'include/linux') diff --git a/fs/attr.c b/fs/attr.c index 29e38a1f7f77..cce7df53b694 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -14,6 +14,7 @@ #include #include #include +#include /** * inode_change_ok - check if attribute changes to an inode are allowed @@ -247,6 +248,7 @@ int notify_change(struct dentry * dentry, struct iattr * attr) if (!error) { fsnotify_change(dentry, ia_valid); + ima_inode_post_setattr(dentry); evm_inode_post_setattr(dentry, ia_valid); } diff --git a/include/linux/ima.h b/include/linux/ima.h index 6ac8e50c6cf5..e2bfbb1e9af6 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -39,5 +39,15 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot) { return 0; } + #endif /* CONFIG_IMA_H */ + +#ifdef CONFIG_IMA_APPRAISE +extern void ima_inode_post_setattr(struct dentry *dentry); +#else +static inline void ima_inode_post_setattr(struct dentry *dentry) +{ + return; +} +#endif /* CONFIG_IMA_APPRAISE_H */ #endif /* _LINUX_IMA_H */ -- cgit v1.2.3 From 42c63330f2b05aa6077c1bfc2798c04afe54f6b2 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Thu, 10 Mar 2011 18:54:15 -0500 Subject: ima: add ima_inode_setxattr/removexattr function and calls Based on xattr_permission comments, the restriction to modify 'security' xattr is left up to the underlying fs or lsm. Ensure that not just anyone can modify or remove 'security.ima'. Changelog v1: - Unless IMA-APPRAISE is configured, use stub ima_inode_removexattr()/setxattr() functions. (Moved ima_inode_removexattr()/setxattr() to ima_appraise.c) Changelog: - take i_mutex to fix locking (Dmitry Kasatkin) - ima_reset_appraise_flags should only be called when modifying or removing the 'security.ima' xattr. Requires CAP_SYS_ADMIN privilege. (Incorporated fix from Roberto Sassu) - Even if allowed to update security.ima, reset the appraisal flags, forcing re-appraisal. - Replace CAP_MAC_ADMIN with CAP_SYS_ADMIN - static inline ima_inode_setxattr()/ima_inode_removexattr() stubs - ima_protect_xattr should be static Signed-off-by: Mimi Zohar Signed-off-by: Dmitry Kasatkin --- include/linux/ima.h | 17 +++++++++++ security/integrity/ima/ima_appraise.c | 57 +++++++++++++++++++++++++++++++++++ security/security.c | 6 ++++ 3 files changed, 80 insertions(+) (limited to 'include/linux') diff --git a/include/linux/ima.h b/include/linux/ima.h index e2bfbb1e9af6..2c7223d7e73b 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -44,10 +44,27 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot) #ifdef CONFIG_IMA_APPRAISE extern void ima_inode_post_setattr(struct dentry *dentry); +extern int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, + const void *xattr_value, size_t xattr_value_len); +extern int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name); #else static inline void ima_inode_post_setattr(struct dentry *dentry) { return; } + +static inline int ima_inode_setxattr(struct dentry *dentry, + const char *xattr_name, + const void *xattr_value, + size_t xattr_value_len) +{ + return 0; +} + +static inline int ima_inode_removexattr(struct dentry *dentry, + const char *xattr_name) +{ + return 0; +} #endif /* CONFIG_IMA_APPRAISE_H */ #endif /* _LINUX_IMA_H */ diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 681cb6e72257..becc7e09116d 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -169,3 +169,60 @@ void ima_inode_post_setattr(struct dentry *dentry) rc = inode->i_op->removexattr(dentry, XATTR_NAME_IMA); return; } + +/* + * ima_protect_xattr - protect 'security.ima' + * + * Ensure that not just anyone can modify or remove 'security.ima'. + */ +static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name, + const void *xattr_value, size_t xattr_value_len) +{ + if (strcmp(xattr_name, XATTR_NAME_IMA) == 0) { + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + return 1; + } + return 0; +} + +static void ima_reset_appraise_flags(struct inode *inode) +{ + struct integrity_iint_cache *iint; + + if (!ima_initialized || !ima_appraise || !S_ISREG(inode->i_mode)) + return; + + iint = integrity_iint_find(inode); + if (!iint) + return; + + iint->flags &= ~(IMA_COLLECTED | IMA_APPRAISED | IMA_MEASURED); + return; +} + +int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, + const void *xattr_value, size_t xattr_value_len) +{ + int result; + + result = ima_protect_xattr(dentry, xattr_name, xattr_value, + xattr_value_len); + if (result == 1) { + ima_reset_appraise_flags(dentry->d_inode); + result = 0; + } + return result; +} + +int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name) +{ + int result; + + result = ima_protect_xattr(dentry, xattr_name, NULL, 0); + if (result == 1) { + ima_reset_appraise_flags(dentry->d_inode); + result = 0; + } + return result; +} diff --git a/security/security.c b/security/security.c index 68c1b9b45d93..d23b43522a5a 100644 --- a/security/security.c +++ b/security/security.c @@ -571,6 +571,9 @@ int security_inode_setxattr(struct dentry *dentry, const char *name, if (unlikely(IS_PRIVATE(dentry->d_inode))) return 0; ret = security_ops->inode_setxattr(dentry, name, value, size, flags); + if (ret) + return ret; + ret = ima_inode_setxattr(dentry, name, value, size); if (ret) return ret; return evm_inode_setxattr(dentry, name, value, size); @@ -606,6 +609,9 @@ int security_inode_removexattr(struct dentry *dentry, const char *name) if (unlikely(IS_PRIVATE(dentry->d_inode))) return 0; ret = security_ops->inode_removexattr(dentry, name); + if (ret) + return ret; + ret = ima_inode_removexattr(dentry, name); if (ret) return ret; return evm_inode_removexattr(dentry, name); -- cgit v1.2.3