[PATCH v4 00/12] ima: appraisal extension

March 29th, 2012 - 10:50 am ET by Mimi Zohar | Report spam
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.

New hooks:
ima_inode_setxattr(), ima_inode_removexattr(), ima_inode_post_setattr(),
and ima_defer_fput()

IMA-appraisal extends the measurement policy ABI with two new keywords:
appraise/dont_appraise and adds a new boot parameter 'ima_appraise_tcb'
to appraise all files owned by root. Like the ima_tcb measurement policy,
the ima_appraise_tcb policy does not appraise pseudo filesystem files
(eg. debugfs, tmpfs, securityfs, selinuxfs or ramfs.)

Additional rules can be added to the default IMA measurement/appraisal
policy, which take advantage of the SELinux labels, for a more fine
grained policy.

Locking changes:

The ima-appraisal extension maintains the file integrity measurement as
an extended attribute 'security.ima'. ima_file_free(), called on __fput(),
updates 'security.ima' to reflect any changes made to the file. In fix
mode, process_measurement() writes 'security.ima' to reflect the current
file hash. Writing extended attributes and other file metadata (eg. chmod),
requires taking the i_mutex. Both ima_file_free() and process_measurement()
took the iint->mutex and then the i_mutex, while chmod() took the locks in
reverse order. To resolve the potential lock inversion deadlock, the
redundant iint->mutex was eliminated.

Writing 'security.ima' from __fput() caused an mmap_sem/i_mutex lockdep,
when an mmapped file was closed before it was munmapped. To resolve this
lockdep, ima_defer_fput() defers the __fput for files, that were closed
prior to the munmap and were mmapped write, by incrementing the f_count
and creating/adding it to the workqueue.

Prereqs:
vfs: fix IMA lockdep circular locking dependency
vfs: iversion truncate bug fix

Changelog v4:
- ima_defer_fput() performance improvements

Changelog v3:
- defined the boot command line parameter 'ima_appraise_tcb' to permit
measuring without appraising, and appraising without measuring.
- use slab mempool to defer __fput() work
- change appraisal default for filesystems without xattr support to fail

Changelog v2:
- Split the "ima: allocating iint improvements" patch, making the
spinlock to rwlock/read_lock change into a separate patch.
- Removed the "vfs: Correctly set the dir i_mutex lockdep class" dependency.
- New: "ima: delay calling __fput()"
- Minor changes listed in individual patch descriptions

Changelog v1:
- Initial posting of the IMA-appraisal patches, separately from EVM.

Dmitry Kasatkin (3):
ima: free securityfs violations file
ima: allocating iint improvements
ima: digital signature verification support

Mimi Zohar (9):
vfs: extend vfs_removexattr locking
vfs: move ima_file_free before releasing the file
ima: integrity appraisal extension
ima: add appraise action keywords and default rules
ima: replace iint spinlock with rwlock/read_lock
ima: add inode_post_setattr call
ima: add ima_inode_setxattr/removexattr function and calls
ima: defer calling __fput()
ima: add support for different security.ima data types

Documentation/ABI/testing/ima_policy | 25 ++-
Documentation/kernel-parameters.txt | 8 +
fs/attr.c | 2 +
fs/file_table.c | 7 +-
fs/xattr.c | 6 +-
include/linux/ima.h | 32 +++
include/linux/integrity.h | 7 +-
include/linux/xattr.h | 3 +
security/integrity/evm/evm_main.c | 3 +
security/integrity/iint.c | 64 +++-
security/integrity/ima/Kconfig | 15 ++
security/integrity/ima/Makefile | 2 +
security/integrity/ima/ima.h | 37 ++++-
security/integrity/ima/ima_api.c | 56 ++++--
security/integrity/ima/ima_appraise.c | 344 +++++++++++++++++++++++++++++++++
security/integrity/ima/ima_crypto.c | 8 +-
security/integrity/ima/ima_fs.c | 1 +
security/integrity/ima/ima_main.c | 89 ++++++
security/integrity/ima/ima_policy.c | 181 +++++++++++++--
security/integrity/integrity.h | 11 +-
security/security.c | 6 +
21 files changed, 758 insertions(+), 149 deletions(-)
create mode 100644 security/integrity/ima/ima_appraise.c

1.7.6.5

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
email Follow the discussionReplies 6 repliesReplies Make a reply

Replies

#1 Mimi Zohar
March 29th, 2012 - 10:50 am ET | Report spam
IMA-appraisal currently verifies the integrity of a file based on a
known 'good' measurement value. This patch reserves the first byte
of 'security.ima' as a place holder for the type of method used for
verifying file data integrity.

Changelog v1:
- Use the newly defined 'struct evm_ima_xattr_data'

Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar

security/integrity/ima/ima_api.c | 6 +++
security/integrity/ima/ima_appraise.c | 23 +++++++++++++-
security/integrity/integrity.h | 2 +-
3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 55deeb1..b5cbef5 100644
a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -147,8 +147,8 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
if (!(iint->flags & IMA_COLLECTED)) {
u64 i_version = file->f_dentry->d_inode->i_version;

- memset(iint->digest, 0, IMA_DIGEST_SIZE);
- result = ima_calc_hash(file, iint->digest);
+ iint->ima_xattr.type = IMA_XATTR_DIGEST;
+ result = ima_calc_hash(file, iint->ima_xattr.digest);
if (!result) {
iint->version = i_version;
iint->flags |= IMA_COLLECTED;
@@ -196,7 +196,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
return;
}
memset(&entry->template, 0, sizeof(entry->template));
- memcpy(entry->template.digest, iint->digest, IMA_DIGEST_SIZE);
+ memcpy(entry->template.digest, iint->ima_xattr.digest, IMA_DIGEST_SIZE);
strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX);

result = ima_store_template(entry, violation, inode);
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 220b20e..9a7d1eb 100644
a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -46,9 +46,9 @@ int ima_must_appraise(struct inode *inode, enum ima_hooks func, int mask)
static void ima_fix_xattr(struct dentry *dentry,
struct integrity_iint_cache *iint)
{
- iint->digest[0] = IMA_XATTR_DIGEST;
- __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
- iint->digest, IMA_DIGEST_SIZE + 1, 0);
+ iint->ima_xattr.type = IMA_XATTR_DIGEST;
+ __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA, (u8 *)&iint->ima_xattr,
+ sizeof iint->ima_xattr, 0);
}

/*
@@ -64,7 +64,7 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint,
{
struct dentry *dentry = file->f_dentry;
struct inode *inode = dentry->d_inode;
- u8 xattr_value[IMA_DIGEST_SIZE];
+ struct evm_ima_xattr_data xattr_value;
enum integrity_status status = INTEGRITY_UNKNOWN;
const char *op = "appraise_data";
char *cause = "unknown";
@@ -78,8 +78,8 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint,
if (iint->flags & IMA_APPRAISED)
return iint->ima_status;

- rc = inode->i_op->getxattr(dentry, XATTR_NAME_IMA, xattr_value,
- IMA_DIGEST_SIZE);
+ rc = inode->i_op->getxattr(dentry, XATTR_NAME_IMA, (u8 *)&xattr_value,
+ sizeof xattr_value);
if (rc <= 0) {
if (rc && rc != -ENODATA)
goto out;
@@ -90,7 +90,8 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint,
goto out;
}

- status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+ status = evm_verifyxattr(dentry, XATTR_NAME_IMA, (u8 *)&xattr_value,
+ rc, iint);
if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
if ((status == INTEGRITY_NOLABEL)
|| (status == INTEGRITY_NOXATTRS))
@@ -100,14 +101,16 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint,
goto out;
}

- rc = memcmp(xattr_value, iint->digest, IMA_DIGEST_SIZE);
+ rc = memcmp(xattr_value.digest, iint->ima_xattr.digest,
+ IMA_DIGEST_SIZE);
if (rc) {
status = INTEGRITY_FAIL;
cause = "invalid-hash";
print_hex_dump_bytes("security.ima: ", DUMP_PREFIX_NONE,
- xattr_value, IMA_DIGEST_SIZE);
+ &xattr_value, sizeof xattr_value);
print_hex_dump_bytes("collected: ", DUMP_PREFIX_NONE,
- iint->digest, IMA_DIGEST_SIZE);
+ (u8 *)&iint->ima_xattr,
+ sizeof iint->ima_xattr);
goto out;
}
status = INTEGRITY_PASS;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 295702d..c145331 100644
a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -39,7 +39,7 @@ struct integrity_iint_cache {
struct inode *inode; /* back pointer to inode in question */
u64 version; /* track inode changes */
unsigned char flags;
- u8 digest[SHA1_DIGEST_SIZE];
+ struct evm_ima_xattr_data ima_xattr;
enum integrity_status ima_status;
enum integrity_status evm_status;
};
1.7.6.5

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Similar topics