2010-04-21 21:50:28

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 00/14] EVM

Extended Verification Module(EVM) detects offline tampering of the
security extended attributes (e.g. security.selinux, security.SMACK64,
security.ima), which is the basis for LSM permission decisions and,
with this set of patches, integrity appraisal decisions. To detect
offline tampering of the extended attributes, EVM maintains an
HMAC-sha1 across a set of security extended attributes, storing the
HMAC as the extended attribute 'security.evm'. To verify the integrity
of an extended attribute, EVM exports evm_verifyxattr(), which
re-calculates the HMAC and compares it with the version stored in
'security.evm'.

IMA currently maintains an integrity measurement list, containing
the hashes of all executables, mmaped execute files, and files open
for read by root (assuming the default measurement policy). The
measurement list, with other information, can be used to assert the
integrity of the running system to a third party. The "ima: integrity
appraisal extension" patch extends IMA with local measurement
appraisal. The extension stores and maintains the file integrity
measurement as an extended attribute 'security.ima', which EVM can be
configured to protect.

DAC/MAC protect the integrity of a running system. An offline attack
can bypass these protection mechanisms by mounting the disk under a
different operating system and modifying the file data/metadata. If
the disk is subsequently remounted under the EVM + DAC/MAC + IMA
protected OS, then the hash of the file data won't match the hash stored
in the IMA xattr, or the TPM-calculated HMAC of the file's metadata won't
be valid. Therefore, IMA + MAC + EVM can protect system integrity online
and detect offline tampering.

This patch set applies to the security-testing/next tree. They
prereq remove-kref-set.patch (https://patchwork.kernel.org/patch/86079,
linux-next commit 8e47f1004).

Much appreciation to Dave Hansen, Serge Hallyn, and Matt Helsley for
reviewing the patches.

Mimi

Mimi Zohar (14):
integrity: move ima inode integrity data management
security: move LSM xattrnames to xattr.h
xattr: define vfs_getxattr_alloc and vfs_xattr_cmp
evm: re-release
ima: move ima_file_free before releasing the file
security: imbed evm calls in security hooks
evm: inode post removexattr
evm: imbed evm_inode_post_setattr
evm: inode_post_init
fs: add evm_inode_post_init calls
ima: integrity appraisal extension
ima: appraise default rules
ima: inode post_setattr
ima: add ima_inode_setxattr and ima_inode_removexattr


2010-04-21 21:50:41

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 02/14] security: move LSM xattrnames to xattr.h

Make the security extended attributes names global.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 39e5ff5..90012b9 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -49,9 +49,6 @@ typedef struct __user_cap_data_struct {
} __user *cap_user_data_t;


-#define XATTR_CAPS_SUFFIX "capability"
-#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
-
#define VFS_CAP_REVISION_MASK 0xFF000000
#define VFS_CAP_REVISION_SHIFT 24
#define VFS_CAP_FLAGS_MASK ~VFS_CAP_REVISION_MASK
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index fb9b7e6..d079bec 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -33,6 +33,16 @@
#define XATTR_USER_PREFIX "user."
#define XATTR_USER_PREFIX_LEN (sizeof (XATTR_USER_PREFIX) - 1)

+/* Security namespace */
+#define XATTR_SELINUX_SUFFIX "selinux"
+#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
+
+#define XATTR_SMACK_SUFFIX "SMACK64"
+#define XATTR_NAME_SMACK XATTR_SECURITY_PREFIX XATTR_SMACK_SUFFIX
+
+#define XATTR_CAPS_SUFFIX "capability"
+#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
+
struct inode;
struct dentry;

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ebee467..1973abe 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -87,9 +87,6 @@
#include "netlabel.h"
#include "audit.h"

-#define XATTR_SELINUX_SUFFIX "selinux"
-#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
-
#define NUM_SEL_MNT_OPTS 5

extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
diff --git a/security/smack/smack.h b/security/smack/smack.h
index c6e9aca..9c773e3 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -126,10 +126,8 @@ struct smack_known {
/*
* xattr names
*/
-#define XATTR_SMACK_SUFFIX "SMACK64"
#define XATTR_SMACK_IPIN "SMACK64IPIN"
#define XATTR_SMACK_IPOUT "SMACK64IPOUT"
-#define XATTR_NAME_SMACK XATTR_SECURITY_PREFIX XATTR_SMACK_SUFFIX
#define XATTR_NAME_SMACKIPIN XATTR_SECURITY_PREFIX XATTR_SMACK_IPIN
#define XATTR_NAME_SMACKIPOUT XATTR_SECURITY_PREFIX XATTR_SMACK_IPOUT

--
1.6.6.1

2010-04-21 21:50:46

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 01/14] integrity: move ima inode integrity data management

Move the inode integrity data(iint) management up to the
integrity layer in order to share the integrity data area
among the different integrity models.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 975837e..4dce900 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -15,8 +15,6 @@ struct linux_binprm;

#ifdef CONFIG_IMA
extern int ima_bprm_check(struct linux_binprm *bprm);
-extern int ima_inode_alloc(struct inode *inode);
-extern void ima_inode_free(struct inode *inode);
extern int ima_file_check(struct file *file, int mask);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
@@ -28,16 +26,6 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
return 0;
}

-static inline int ima_inode_alloc(struct inode *inode)
-{
- return 0;
-}
-
-static inline void ima_inode_free(struct inode *inode)
-{
- return;
-}
-
static inline int ima_file_check(struct file *file, int mask)
{
return 0;
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
new file mode 100644
index 0000000..276081f
--- /dev/null
+++ b/include/linux/integrity.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2009 IBM Corporation
+ * Author: Mimi Zohar <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#ifndef _LINUX_INTEGRITY_H
+#define _LINUX_INTEGRITY_H
+
+#ifdef CONFIG_INTEGRITY
+extern int integrity_inode_alloc(struct inode *inode);
+extern void integrity_inode_free(struct inode *inode);
+
+#else
+static inline int integrity_inode_alloc(struct inode *inode)
+{
+ return 0;
+}
+
+static inline void integrity_inode_free(struct inode *inode)
+{
+ return;
+}
+#endif /* CONFIG_INTEGRITY_H */
+#endif /* _LINUX_INTEGRITY_H */
diff --git a/security/Kconfig b/security/Kconfig
index 226b955..4f00287 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -141,7 +141,7 @@ source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig

-source security/integrity/ima/Kconfig
+source security/integrity/Kconfig

choice
prompt "Default security module"
diff --git a/security/Makefile b/security/Makefile
index da20a19..9269caa 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -22,5 +22,5 @@ obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o
obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o

# Object integrity file lists
-subdir-$(CONFIG_IMA) += integrity/ima
-obj-$(CONFIG_IMA) += integrity/ima/built-in.o
+subdir-$(CONFIG_INTEGRITY) += integrity
+obj-$(CONFIG_INTEGRITY) += integrity/built-in.o
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
new file mode 100644
index 0000000..2704691
--- /dev/null
+++ b/security/integrity/Kconfig
@@ -0,0 +1,6 @@
+#
+config INTEGRITY
+ def_bool y
+ depends on IMA
+
+source security/integrity/ima/Kconfig
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
new file mode 100644
index 0000000..6eddd61
--- /dev/null
+++ b/security/integrity/Makefile
@@ -0,0 +1,10 @@
+#
+# Makefile for caching inode integrity data (iint)
+#
+
+obj-$(CONFIG_INTEGRITY) += integrity.o
+
+integrity-y := iint.o
+
+subdir-$(CONFIG_IMA) += ima
+obj-$(CONFIG_IMA) += ima/built-in.o
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
new file mode 100644
index 0000000..e2bdd8f
--- /dev/null
+++ b/security/integrity/iint.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright (C) 2008-2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: iint.c
+ * - implements the integrity hooks: integrity_inode_alloc,
+ * integrity_inode_free
+ * - cache integrity information associated with an inode
+ * using a radix tree.
+ */
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/fs.h>
+#include <linux/radix-tree.h>
+#include "integrity.h"
+
+RADIX_TREE(integrity_iint_store, GFP_ATOMIC);
+DEFINE_SPINLOCK(integrity_iint_lock);
+
+static struct kmem_cache *iint_cache __read_mostly;
+
+/* integrity_iint_find_get - return the iint associated with an inode
+ *
+ * integrity_iint_find_get gets a reference to the iint. Caller must
+ * remember to put the iint reference.
+ */
+struct integrity_iint_cache *integrity_iint_find_get(struct inode *inode)
+{
+ struct integrity_iint_cache *iint;
+
+ rcu_read_lock();
+ iint = radix_tree_lookup(&integrity_iint_store, (unsigned long)inode);
+ if (!iint)
+ goto out;
+ kref_get(&iint->refcount);
+out:
+ rcu_read_unlock();
+ return iint;
+}
+
+/**
+ * integrity_inode_alloc - allocate an iint associated with an inode
+ * @inode: pointer to the inode
+ */
+int integrity_inode_alloc(struct inode *inode)
+{
+ struct integrity_iint_cache *iint = NULL;
+ int rc = 0;
+
+ iint = kmem_cache_alloc(iint_cache, GFP_NOFS);
+ if (!iint)
+ return -ENOMEM;
+
+ rc = radix_tree_preload(GFP_NOFS);
+ if (rc < 0)
+ goto out;
+
+ spin_lock(&integrity_iint_lock);
+ rc = radix_tree_insert(&integrity_iint_store, (unsigned long)inode,
+ iint);
+ spin_unlock(&integrity_iint_lock);
+ radix_tree_preload_end();
+out:
+ if (rc < 0)
+ kmem_cache_free(iint_cache, iint);
+
+ return rc;
+}
+
+/* iint_free - called when the iint refcount goes to zero */
+void iint_free(struct kref *kref)
+{
+ struct integrity_iint_cache *iint =
+ container_of(kref, struct integrity_iint_cache, refcount);
+ iint->version = 0;
+ iint->flags = 0UL;
+ if (iint->readcount != 0) {
+ printk(KERN_INFO "%s: readcount: %ld\n", __func__,
+ iint->readcount);
+ iint->readcount = 0;
+ }
+ if (iint->writecount != 0) {
+ printk(KERN_INFO "%s: writecount: %ld\n", __func__,
+ iint->writecount);
+ iint->writecount = 0;
+ }
+ if (iint->opencount != 0) {
+ printk(KERN_INFO "%s: opencount: %ld\n", __func__,
+ iint->opencount);
+ iint->opencount = 0;
+ }
+ kref_init(&iint->refcount);
+ kmem_cache_free(iint_cache, iint);
+}
+
+void iint_rcu_free(struct rcu_head *rcu_head)
+{
+ struct integrity_iint_cache *iint =
+ container_of(rcu_head, struct integrity_iint_cache, rcu);
+ kref_put(&iint->refcount, iint_free);
+}
+
+/**
+ * integrity_inode_free - called on security_inode_free
+ * @inode: pointer to the inode
+ *
+ * Free the integrity information(iint) associated with an inode.
+ */
+void integrity_inode_free(struct inode *inode)
+{
+ struct integrity_iint_cache *iint;
+
+ spin_lock(&integrity_iint_lock);
+ iint = radix_tree_delete(&integrity_iint_store, (unsigned long)inode);
+ spin_unlock(&integrity_iint_lock);
+ if (iint)
+ call_rcu(&iint->rcu, iint_rcu_free);
+}
+
+static void init_once(void *foo)
+{
+ struct integrity_iint_cache *iint = foo;
+
+ memset(iint, 0, sizeof *iint);
+ iint->version = 0;
+ iint->flags = 0UL;
+ mutex_init(&iint->mutex);
+ iint->readcount = 0;
+ iint->writecount = 0;
+ iint->opencount = 0;
+ kref_init(&iint->refcount);
+}
+
+static int __init integrity_iintcache_init(void)
+{
+ iint_cache =
+ kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache),
+ 0, SLAB_PANIC, init_once);
+ return 0;
+}
+
+security_initcall(integrity_iintcache_init);
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 3d7846d..dc60e40 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -4,6 +4,7 @@ config IMA
bool "Integrity Measurement Architecture(IMA)"
depends on ACPI
depends on SECURITY
+ select INTEGRITY
select SECURITYFS
select CRYPTO
select CRYPTO_HMAC
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 787c4cb..5690c02 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -6,4 +6,4 @@
obj-$(CONFIG_IMA) += ima.o

ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
- ima_policy.o ima_iint.o ima_audit.o
+ ima_policy.o ima_audit.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 16d100d..a58ac9f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -24,11 +24,13 @@
#include <linux/tpm.h>
#include <linux/audit.h>

+#include "../integrity.h"
+
enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_ASCII };
enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };

/* digest size for IMA, fits SHA1 or MD5 */
-#define IMA_DIGEST_SIZE 20
+#define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
#define IMA_EVENT_NAME_LEN_MAX 255

#define IMA_HASH_BITS 9
@@ -94,38 +96,22 @@ static inline unsigned long ima_hash_key(u8 *digest)
return hash_long(*digest, IMA_HASH_BITS);
}

-/* iint cache flags */
-#define IMA_MEASURED 1
-
-/* integrity data associated with an inode */
-struct ima_iint_cache {
- u64 version; /* track inode changes */
- unsigned long flags;
- u8 digest[IMA_DIGEST_SIZE];
- struct mutex mutex; /* protects: version, flags, digest */
- long readcount; /* measured files readcount */
- long writecount; /* measured files writecount */
- long opencount; /* opens reference count */
- struct kref refcount; /* ima_iint_cache reference count */
- struct rcu_head rcu;
-};
-
/* LIM API function definitions */
-int ima_must_measure(struct ima_iint_cache *iint, struct inode *inode,
+int ima_must_measure(struct integrity_iint_cache *iint, struct inode *inode,
int mask, int function);
-int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file);
-void ima_store_measurement(struct ima_iint_cache *iint, struct file *file,
+int ima_collect_measurement(struct integrity_iint_cache *iint,
+ struct file *file);
+void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
const unsigned char *filename);
int ima_store_template(struct ima_template_entry *entry, int violation,
struct inode *inode);
-void ima_template_show(struct seq_file *m, void *e,
- enum ima_show_type show);
+void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show);

/* radix tree calls to lookup, insert, delete
* integrity data associated with an inode.
*/
-struct ima_iint_cache *ima_iint_insert(struct inode *inode);
-struct ima_iint_cache *ima_iint_find_get(struct inode *inode);
+struct integrity_iint_cache *ima_iint_insert(struct inode *inode);
+struct integrity_iint_cache *ima_iint_find_get(struct inode *inode);
void iint_free(struct kref *kref);
void iint_rcu_free(struct rcu_head *rcu);

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 2a5e0bc..bb307fb 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -110,7 +110,7 @@ err_out:
* For matching a DONT_MEASURE policy, no policy, or other
* error, return an error code.
*/
-int ima_must_measure(struct ima_iint_cache *iint, struct inode *inode,
+int ima_must_measure(struct integrity_iint_cache *iint, struct inode *inode,
int mask, int function)
{
int must_measure;
@@ -132,7 +132,8 @@ int ima_must_measure(struct ima_iint_cache *iint, struct inode *inode,
*
* Return 0 on success, error code otherwise
*/
-int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file)
+int ima_collect_measurement(struct integrity_iint_cache *iint,
+ struct file *file)
{
int result = -EEXIST;

@@ -162,8 +163,8 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file)
*
* Must be called with iint->mutex held.
*/
-void ima_store_measurement(struct ima_iint_cache *iint, struct file *file,
- const unsigned char *filename)
+void ima_store_measurement(struct integrity_iint_cache *iint,
+ struct file *file, const unsigned char *filename)
{
const char *op = "add_template_measure";
const char *audit_cause = "ENOMEM";
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
deleted file mode 100644
index 01ddf31..0000000
--- a/security/integrity/ima/ima_iint.c
+++ /dev/null
@@ -1,145 +0,0 @@
-/*
- * Copyright (C) 2008 IBM Corporation
- *
- * Authors:
- * Mimi Zohar <[email protected]>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation, version 2 of the
- * License.
- *
- * File: ima_iint.c
- * - implements the IMA hooks: ima_inode_alloc, ima_inode_free
- * - cache integrity information associated with an inode
- * using a radix tree.
- */
-#include <linux/module.h>
-#include <linux/spinlock.h>
-#include <linux/radix-tree.h>
-#include "ima.h"
-
-RADIX_TREE(ima_iint_store, GFP_ATOMIC);
-DEFINE_SPINLOCK(ima_iint_lock);
-
-static struct kmem_cache *iint_cache __read_mostly;
-
-/* ima_iint_find_get - return the iint associated with an inode
- *
- * ima_iint_find_get gets a reference to the iint. Caller must
- * remember to put the iint reference.
- */
-struct ima_iint_cache *ima_iint_find_get(struct inode *inode)
-{
- struct ima_iint_cache *iint;
-
- rcu_read_lock();
- iint = radix_tree_lookup(&ima_iint_store, (unsigned long)inode);
- if (!iint)
- goto out;
- kref_get(&iint->refcount);
-out:
- rcu_read_unlock();
- return iint;
-}
-
-/**
- * ima_inode_alloc - allocate an iint associated with an inode
- * @inode: pointer to the inode
- */
-int ima_inode_alloc(struct inode *inode)
-{
- struct ima_iint_cache *iint = NULL;
- int rc = 0;
-
- iint = kmem_cache_alloc(iint_cache, GFP_NOFS);
- if (!iint)
- return -ENOMEM;
-
- rc = radix_tree_preload(GFP_NOFS);
- if (rc < 0)
- goto out;
-
- spin_lock(&ima_iint_lock);
- rc = radix_tree_insert(&ima_iint_store, (unsigned long)inode, iint);
- spin_unlock(&ima_iint_lock);
- radix_tree_preload_end();
-out:
- if (rc < 0)
- kmem_cache_free(iint_cache, iint);
-
- return rc;
-}
-
-/* iint_free - called when the iint refcount goes to zero */
-void iint_free(struct kref *kref)
-{
- struct ima_iint_cache *iint = container_of(kref, struct ima_iint_cache,
- refcount);
- iint->version = 0;
- iint->flags = 0UL;
- if (iint->readcount != 0) {
- printk(KERN_INFO "%s: readcount: %ld\n", __func__,
- iint->readcount);
- iint->readcount = 0;
- }
- if (iint->writecount != 0) {
- printk(KERN_INFO "%s: writecount: %ld\n", __func__,
- iint->writecount);
- iint->writecount = 0;
- }
- if (iint->opencount != 0) {
- printk(KERN_INFO "%s: opencount: %ld\n", __func__,
- iint->opencount);
- iint->opencount = 0;
- }
- kref_init(&iint->refcount);
- kmem_cache_free(iint_cache, iint);
-}
-
-void iint_rcu_free(struct rcu_head *rcu_head)
-{
- struct ima_iint_cache *iint = container_of(rcu_head,
- struct ima_iint_cache, rcu);
- kref_put(&iint->refcount, iint_free);
-}
-
-/**
- * ima_inode_free - called on security_inode_free
- * @inode: pointer to the inode
- *
- * Free the integrity information(iint) associated with an inode.
- */
-void ima_inode_free(struct inode *inode)
-{
- struct ima_iint_cache *iint;
-
- spin_lock(&ima_iint_lock);
- iint = radix_tree_delete(&ima_iint_store, (unsigned long)inode);
- spin_unlock(&ima_iint_lock);
- if (iint)
- call_rcu(&iint->rcu, iint_rcu_free);
-}
-
-static void init_once(void *foo)
-{
- struct ima_iint_cache *iint = foo;
-
- memset(iint, 0, sizeof *iint);
- iint->version = 0;
- iint->flags = 0UL;
- mutex_init(&iint->mutex);
- iint->readcount = 0;
- iint->writecount = 0;
- iint->opencount = 0;
- kref_init(&iint->refcount);
-}
-
-static int __init ima_iintcache_init(void)
-{
- iint_cache =
- kmem_cache_create("iint_cache", sizeof(struct ima_iint_cache), 0,
- SLAB_PANIC, init_once);
- return 0;
-}
-security_initcall(ima_iintcache_init);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 983037f..7eeb76e 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -18,6 +18,7 @@
*/
#include <linux/module.h>
#include <linux/file.h>
+#include <linux/fs.h>
#include <linux/binfmts.h>
#include <linux/mount.h>
#include <linux/mman.h>
@@ -96,7 +97,7 @@ out:
*/
enum iint_pcr_error { TOMTOU, OPEN_WRITERS };
static void ima_read_write_check(enum iint_pcr_error error,
- struct ima_iint_cache *iint,
+ struct integrity_iint_cache *iint,
struct inode *inode,
const unsigned char *filename)
{
@@ -117,7 +118,7 @@ static void ima_read_write_check(enum iint_pcr_error error,
/*
* Update the counts given an fmode_t
*/
-static void ima_inc_counts(struct ima_iint_cache *iint, fmode_t mode)
+static void ima_inc_counts(struct integrity_iint_cache *iint, fmode_t mode)
{
BUG_ON(!mutex_is_locked(&iint->mutex));

@@ -144,12 +145,12 @@ void ima_counts_get(struct file *file)
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = dentry->d_inode;
fmode_t mode = file->f_mode;
- struct ima_iint_cache *iint;
+ struct integrity_iint_cache *iint;
int rc;

if (!ima_initialized || !S_ISREG(inode->i_mode))
return;
- iint = ima_iint_find_get(inode);
+ iint = integrity_iint_find_get(inode);
if (!iint)
return;
mutex_lock(&iint->mutex);
@@ -172,8 +173,8 @@ out:
/*
* Decrement ima counts
*/
-static void ima_dec_counts(struct ima_iint_cache *iint, struct inode *inode,
- struct file *file)
+static void ima_dec_counts(struct integrity_iint_cache *iint,
+ struct inode *inode, struct file *file)
{
mode_t mode = file->f_mode;
BUG_ON(!mutex_is_locked(&iint->mutex));
@@ -210,11 +211,11 @@ static void ima_dec_counts(struct ima_iint_cache *iint, struct inode *inode,
void ima_file_free(struct file *file)
{
struct inode *inode = file->f_dentry->d_inode;
- struct ima_iint_cache *iint;
+ struct integrity_iint_cache *iint;

if (!ima_initialized || !S_ISREG(inode->i_mode))
return;
- iint = ima_iint_find_get(inode);
+ iint = integrity_iint_find_get(inode);
if (!iint)
return;

@@ -228,12 +229,12 @@ static int process_measurement(struct file *file, const unsigned char *filename,
int mask, int function)
{
struct inode *inode = file->f_dentry->d_inode;
- struct ima_iint_cache *iint;
+ struct integrity_iint_cache *iint;
int rc;

if (!ima_initialized || !S_ISREG(inode->i_mode))
return 0;
- iint = ima_iint_find_get(inode);
+ iint = integrity_iint_find_get(inode);
if (!iint)
return -ENOMEM;

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
new file mode 100644
index 0000000..f1013c9
--- /dev/null
+++ b/security/integrity/integrity.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2009-2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: integrity.h
+ */
+#include <linux/types.h>
+#include <linux/integrity.h>
+#include <crypto/sha.h>
+
+#define MAX_DIGEST_SIZE SHA1_DIGEST_SIZE
+
+/* iint cache flags */
+#define IMA_MEASURED 1
+
+/* integrity data associated with an inode */
+struct integrity_iint_cache {
+ u64 version; /* track inode changes */
+ unsigned long flags;
+ u8 digest[MAX_DIGEST_SIZE];
+ struct mutex mutex; /* protects: version, flags, digest */
+ long readcount; /* measured files readcount */
+ long writecount; /* measured files writecount */
+ long opencount; /* opens reference count */
+ struct kref refcount; /* ima_iint_cache reference count */
+ struct rcu_head rcu;
+};
+
+/* radix 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_get(struct inode *inode);
+void iint_free(struct kref *kref);
+void iint_rcu_free(struct rcu_head *rcu);
diff --git a/security/security.c b/security/security.c
index 8585019..e42a78e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -16,6 +16,7 @@
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/security.h>
+#include <linux/integrity.h>
#include <linux/ima.h>

/* Boot-time LSM user choice */
@@ -339,7 +340,7 @@ int security_inode_alloc(struct inode *inode)
ret = security_ops->inode_alloc_security(inode);
if (ret)
return ret;
- ret = ima_inode_alloc(inode);
+ ret = integrity_inode_alloc(inode);
if (ret)
security_inode_free(inode);
return ret;
@@ -347,7 +348,7 @@ int security_inode_alloc(struct inode *inode)

void security_inode_free(struct inode *inode)
{
- ima_inode_free(inode);
+ integrity_inode_free(inode);
security_ops->inode_free_security(inode);
}

--
1.6.6.1

2010-04-21 21:51:11

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 04/14] evm: re-release

EVM protects a file's security extended attributes against integrity
attacks. It maintains an HMAC-sha1 value across the extended attributes,
storing the value as the extended attribute 'security.evm'. EVM has gone
through a number of iterations, initially as an LSM module, subsequently
as a LIM integrity provider, and now, when co-located with a security_
hook, embedded directly in the security_ hook, similar to IMA.

This is the first part of a local file integrity verification system.
While this part does authenticate the selected extended attributes, and
cryptographically bind them to the inode, coming extensions will bind
other directory and inode metadata for more complete protection. The
set of protected security extended attributes is configured at compile.

EVM depends on the Kernel Key Retention System to provide it with the
kernel master key for the HMAC operation. The kernel master key is
securely loaded onto the root's keyring, typically by 'loadkernkey',
which either uses the TPM sealed secret key, if available, or a
password requested from the console. To signal EVM, that the key has
been loaded onto the keyring, 'echo 1 > <securityfs>/evm'. This is
normally done in the initrd, which has already been measured as part
of the trusted boot. (Refer to http://linux-ima.sourceforge.net/#EVM.)

EVM adds the following three calls to the existing security hooks,
evm_inode_setxattr(), evm_inode_post_setxattr(), and
evm_inode_removexattr.

To initialize and update the 'security.evm' extended attribute, EVM
defines three calls: evm_inode_post_init(), evm_inode_post_setattr()
and evm_inode_post_removexattr() hooks.

To verify the integrity of an extended attribute, EVM exports
evm_verifyxattr().

Signed-off-by: Mimi Zohar <[email protected]>

diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index 276081f..fa9c199 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -10,6 +10,13 @@
#ifndef _LINUX_INTEGRITY_H
#define _LINUX_INTEGRITY_H

+enum integrity_status {
+ INTEGRITY_PASS = 0,
+ INTEGRITY_FAIL,
+ INTEGRITY_NOLABEL,
+ INTEGRITY_UNKNOWN,
+};
+
#ifdef CONFIG_INTEGRITY
extern int integrity_inode_alloc(struct inode *inode);
extern void integrity_inode_free(struct inode *inode);
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 8698de3..d1afa51 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -34,6 +34,9 @@
#define XATTR_USER_PREFIX_LEN (sizeof (XATTR_USER_PREFIX) - 1)

/* Security namespace */
+#define XATTR_EVM_SUFFIX "evm"
+#define XATTR_NAME_EVM XATTR_SECURITY_PREFIX XATTR_EVM_SUFFIX
+
#define XATTR_SELINUX_SUFFIX "selinux"
#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 2704691..4bf00ac 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -1,6 +1,7 @@
#
config INTEGRITY
def_bool y
- depends on IMA
+ depends on IMA || EVM

source security/integrity/ima/Kconfig
+source security/integrity/evm/Kconfig
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 6eddd61..0ae44ae 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -8,3 +8,5 @@ integrity-y := iint.o

subdir-$(CONFIG_IMA) += ima
obj-$(CONFIG_IMA) += ima/built-in.o
+subdir-$(CONFIG_EVM) += evm
+obj-$(CONFIG_EVM) += evm/built-in.o
diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
new file mode 100644
index 0000000..8a546f7
--- /dev/null
+++ b/security/integrity/evm/Kconfig
@@ -0,0 +1,13 @@
+config EVM
+ boolean "EVM support"
+ depends on SECURITY && KEYS
+ select CRYPTO_HMAC
+ select CRYPTO_MD5
+ select CRYPTO_SHA1
+ default n
+ help
+ A configurable set of security extended attributes are HMAC
+ protected against modification using the TPM's kernel root key,
+ if configured, or with a pass-phrase.
+
+ If you are unsure how to answer this question, answer N.
diff --git a/security/integrity/evm/Makefile b/security/integrity/evm/Makefile
new file mode 100644
index 0000000..3d38273
--- /dev/null
+++ b/security/integrity/evm/Makefile
@@ -0,0 +1,7 @@
+#
+# Makefile for building the Extended Verification Module(EVM)
+#
+obj-$(CONFIG_EVM) += evm.o
+
+evm-y := evm_main.o evm_crypto.o evm_secfs.o
+
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
new file mode 100644
index 0000000..0b6f7b2
--- /dev/null
+++ b/security/integrity/evm/evm.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2005-2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm.h
+ *
+ */
+#include <linux/security.h>
+#include "../integrity.h"
+
+extern int evm_initialized;
+extern char *evm_hmac;
+extern int evm_hmac_size;
+
+/* List of EVM protected security xattrs */
+extern char *evm_config_xattrnames[];
+
+extern int evm_init_tpmkernkey(void);
+extern int evm_update_evmxattr(struct dentry *dentry,
+ const char *req_xattr_name,
+ const char *req_xattr_value,
+ size_t req_xattr_value_len,
+ struct integrity_iint_cache *iint);
+extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
+ const char *req_xattr_value,
+ size_t req_xattr_value_len, char *digest);
+extern int evm_init_secfs(void);
+extern void evm_cleanup_secfs(void);
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
new file mode 100644
index 0000000..4f1be71
--- /dev/null
+++ b/security/integrity/evm/evm_crypto.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2005-2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm_crypto.c
+ * Using root's kernel master key (kmk), calculate the HMAC
+ */
+
+#include <linux/module.h>
+#include <linux/crypto.h>
+#include <linux/xattr.h>
+#include <keys/user-type.h>
+#include <linux/scatterlist.h>
+#include "evm.h"
+
+#define TPMKEY "evm_key"
+#define MAX_TPMKEY 128
+static unsigned char tpm_key[MAX_TPMKEY];
+static int tpm_keylen = MAX_TPMKEY;
+
+static int init_desc(struct hash_desc *desc)
+{
+ int rc;
+
+ desc->tfm = crypto_alloc_hash(evm_hmac, 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(desc->tfm)) {
+ pr_info("Can not allocate %s (reason: %ld)\n",
+ evm_hmac, PTR_ERR(desc->tfm));
+ rc = PTR_ERR(desc->tfm);
+ return rc;
+ }
+ desc->flags = 0;
+ crypto_hash_setkey(desc->tfm, tpm_key, tpm_keylen);
+ rc = crypto_hash_init(desc);
+ if (rc)
+ crypto_free_hash(desc->tfm);
+ return rc;
+}
+
+/* Protect against 'cutting & pasting' security.evm xattr, include inode
+ * specific info.
+ *
+ * (Additional directory/file metadata needs to be added for more complete
+ * protection.)
+ */
+static void hmac_add_misc(struct hash_desc *desc, struct inode *inode,
+ char *digest)
+{
+ struct h_misc {
+ unsigned long ino;
+ __u32 generation;
+ uid_t uid;
+ gid_t gid;
+ umode_t mode;
+ } hmac_misc;
+ struct scatterlist sg[1];
+
+ memset(&hmac_misc, 0, sizeof hmac_misc);
+ hmac_misc.ino = inode->i_ino;
+ hmac_misc.generation = inode->i_generation;
+ hmac_misc.uid = inode->i_uid;
+ hmac_misc.gid = inode->i_gid;
+ hmac_misc.mode = inode->i_mode;
+ sg_init_one(sg, &hmac_misc, sizeof hmac_misc);
+ crypto_hash_update(desc, sg, sizeof hmac_misc);
+ crypto_hash_final(desc, digest);
+}
+
+/*
+ * Calculate the HMAC value across the set of protected security xattrs.
+ *
+ * Instead of retrieving the requested xattr, for performance, calculate
+ * the hmac using the requested xattr value. Don't alloc/free memory for
+ * each xattr, but attempt to re-use the previously allocated memory.
+ */
+int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
+ const char *req_xattr_value, size_t req_xattr_value_len,
+ char *digest)
+{
+ struct inode *inode = dentry->d_inode;
+ struct hash_desc desc;
+ struct scatterlist sg[1];
+ char **xattrname;
+ size_t xattr_size = 0;
+ char *xattr_value = NULL;
+ int error;
+ int size;
+
+ if (!inode->i_op || !inode->i_op->getxattr)
+ return -EOPNOTSUPP;
+ error = init_desc(&desc);
+ if (error)
+ return error;
+
+ error = -ENODATA;
+ for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
+ if ((req_xattr_name && req_xattr_value)
+ && !strcmp(*xattrname, req_xattr_name)) {
+ error = 0;
+ sg_init_one(sg, req_xattr_value, req_xattr_value_len);
+ crypto_hash_update(&desc, sg, req_xattr_value_len);
+ continue;
+ }
+ size = vfs_getxattr_alloc(dentry, *xattrname,
+ &xattr_value, xattr_size, GFP_NOFS);
+ if (size == -ENOMEM) {
+ error = -ENOMEM;
+ goto out;
+ }
+ if (size < 0)
+ continue;
+
+ error = 0;
+ xattr_size = size;
+ sg_init_one(sg, xattr_value, xattr_size);
+ crypto_hash_update(&desc, sg, xattr_size);
+ }
+ hmac_add_misc(&desc, inode, digest);
+ kfree(xattr_value);
+out:
+ crypto_free_hash(desc.tfm);
+ return error;
+}
+
+/*
+ * Calculate the hmac and update security.evm xattr
+ */
+int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
+ const char *xattr_value, size_t xattr_value_len,
+ struct integrity_iint_cache *iint)
+{
+ struct inode *inode = dentry->d_inode;
+ int rc = 0;
+
+ memset(iint->hmac, 0, sizeof iint->hmac);
+ rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
+ xattr_value_len, iint->hmac);
+ if (rc == 0)
+ rc = inode->i_op->setxattr(dentry, XATTR_NAME_EVM,
+ iint->hmac, evm_hmac_size, 0);
+ else if (rc == -ENODATA)
+ rc = inode->i_op->removexattr(dentry, XATTR_NAME_EVM);
+ return rc;
+}
+
+/*
+ * Get the key from the TPM for the SHA1-HMAC
+ */
+int evm_init_tpmkernkey(void)
+{
+ struct key *kmk;
+ struct user_key_payload *ukp;
+ int len;
+
+ kmk = request_key(&key_type_user, TPMKEY, NULL);
+ if (IS_ERR(kmk))
+ return -ENOENT;
+
+ down_read(&kmk->sem);
+ ukp = kmk->payload.data;
+ len = ukp->datalen;
+ if (len > MAX_TPMKEY)
+ len = MAX_TPMKEY;
+ tpm_keylen = len;
+ memcpy(tpm_key, ukp->data, len);
+
+ /* burn the original key contents */
+ memset(ukp->data, 0, len);
+ up_read(&kmk->sem);
+ key_put(kmk);
+ return 0;
+}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
new file mode 100644
index 0000000..59dc148
--- /dev/null
+++ b/security/integrity/evm/evm_main.c
@@ -0,0 +1,296 @@
+/*
+ * Copyright (C) 2005-2010 IBM Corporation
+ *
+ * Author:
+ * Mimi Zohar <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm_main.c
+ * implements evm_inode_setxattr, evm_inode_post_setxattr,
+ * evm_inode_removexattr, and evm_verifyxattr
+ */
+
+#include <linux/module.h>
+#include <linux/crypto.h>
+#include <linux/xattr.h>
+#include <linux/integrity.h>
+#include "evm.h"
+
+int evm_initialized;
+
+char *evm_hmac = "hmac(sha1)";
+int evm_hmac_size = SHA1_DIGEST_SIZE;
+
+char *evm_config_xattrnames[] = {
+#ifdef CONFIG_SECURITY_SELINUX
+ XATTR_NAME_SELINUX,
+#endif
+#ifdef CONFIG_SECURITY_SMACK
+ XATTR_NAME_SMACK,
+#endif
+ XATTR_NAME_CAPS,
+ NULL
+};
+
+/*
+ * evm_verify_hmac - calculate and compare the HMAC with the EVM xattr
+ *
+ * Compute the HMAC on the dentry's protected set of extended attributes
+ * and compare it against the stored security.evm xattr. (For performance,
+ * use the previoulsy retrieved xattr value and length to calculate the
+ * HMAC.)
+ *
+ * Returns integrity status
+ */
+static enum integrity_status evm_verify_hmac(struct dentry *dentry,
+ const char *xattr_name,
+ char *xattr_value,
+ size_t xattr_value_len,
+ struct integrity_iint_cache *iint)
+{
+ char hmac_val[MAX_DIGEST_SIZE];
+ int rc;
+
+ if (iint->hmac_status != INTEGRITY_UNKNOWN)
+ return iint->hmac_status;
+
+ memset(hmac_val, 0, sizeof hmac_val);
+ rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
+ xattr_value_len, hmac_val);
+ if (rc < 0)
+ return INTEGRITY_UNKNOWN;
+
+ rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, hmac_val, sizeof hmac_val,
+ GFP_NOFS);
+ if (rc < 0)
+ goto err_out;
+ iint->hmac_status = INTEGRITY_PASS;
+ return iint->hmac_status;
+
+err_out:
+ switch (rc) {
+ case -ENODATA: /* file not labelled */
+ iint->hmac_status = INTEGRITY_NOLABEL;
+ break;
+ case -EINVAL:
+ iint->hmac_status = INTEGRITY_FAIL;
+ break;
+ default:
+ iint->hmac_status = INTEGRITY_UNKNOWN;
+ }
+ return iint->hmac_status;
+}
+
+static int evm_protected_xattr(const char *req_xattr_name)
+{
+ char **xattrname;
+ int found = 0;
+
+ for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
+ if (strncmp(req_xattr_name, *xattrname,
+ strlen(req_xattr_name)) == 0) {
+ found = 1;
+ break;
+ }
+ }
+ return found;
+}
+
+/**
+ * evm_verifyxattr - verify the integrity of the requested xattr
+ * @dentry: object of the verify xattr
+ * @xattr_name: requested xattr
+ * @xattr_value: requested xattr value
+ * @xattr_value_len: requested xattr value length
+ *
+ * Calculate the HMAC for the given dentry and verify it
+ * against the stored security.evm xattr. For performance,
+ * use the xattr value and length previously retrieved
+ * to calculate the HMAC.
+ *
+ * Returns the xattr integrity status.
+ */
+enum integrity_status evm_verifyxattr(struct dentry *dentry,
+ const char *xattr_name,
+ void *xattr_value, size_t xattr_value_len)
+{
+ struct inode *inode = dentry->d_inode;
+ struct integrity_iint_cache *iint;
+ enum integrity_status status;
+
+ if (!evm_initialized || !evm_protected_xattr(xattr_name))
+ return INTEGRITY_UNKNOWN;
+
+ iint = integrity_iint_find_get(inode);
+ if (!iint)
+ return -ENOMEM;
+ mutex_lock(&iint->evm_mutex);
+ status = evm_verify_hmac(dentry, xattr_name, xattr_value,
+ xattr_value_len, iint);
+ mutex_unlock(&iint->evm_mutex);
+ kref_put(&iint->refcount, iint_free);
+ return status;
+}
+EXPORT_SYMBOL_GPL(evm_verifyxattr);
+
+/*
+ * evm_protect_xattr - protect the EVM extended attribute
+ *
+ * Prevent security.evm from being modified or removed.
+ */
+static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len)
+{
+ if ((!capable(CAP_MAC_ADMIN))
+ && (strcmp(xattr_name, XATTR_NAME_EVM) == 0))
+ return -EPERM;
+ return 0;
+}
+
+/**
+ * evm_inode_setxattr - protect the EVM extended attribute
+ * @dentry: pointer to the affected dentry
+ * @xattr_name: pointer to the affected extended attribute name
+ * @xattr_value: pointer to the new extended attribute value
+ * @xattr_value_len: pointer to the new extended attribute value length
+ *
+ * Prevent 'security.evm' from being modified
+ */
+int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len)
+{
+ return evm_protect_xattr(dentry, xattr_name, xattr_value,
+ xattr_value_len);
+}
+
+/**
+ * evm_inode_removexattr - protect the EVM extended attribute
+ * @dentry: pointer to the affected dentry
+ * @xattr_name: pointer to the affected extended attribute name
+ *
+ * Prevent 'security.evm' from being removed.
+ */
+int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name)
+{
+ return evm_protect_xattr(dentry, xattr_name, NULL, 0);
+}
+
+/*
+ * After updating/removing an extended attribute defined in /etc/evm.config,
+ * update the HMAC(security.evm) to reflect the change.
+ */
+static void evm_postxattr(struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len)
+{
+ struct inode *inode = dentry->d_inode;
+ struct integrity_iint_cache *iint;
+ int rc;
+
+ iint = integrity_iint_find_get(inode);
+ if (!iint)
+ return;
+ mutex_lock(&iint->evm_mutex);
+ iint->hmac_status = INTEGRITY_UNKNOWN;
+ rc = evm_update_evmxattr(dentry, xattr_name, xattr_value,
+ xattr_value_len, iint);
+ if (!rc)
+ iint->hmac_status = INTEGRITY_PASS;
+ mutex_unlock(&iint->evm_mutex);
+ kref_put(&iint->refcount, iint_free);
+}
+
+/**
+ * evm_inode_post_setxattr - update 'security.evm' to reflect the changes
+ * @dentry: pointer to the affected dentry
+ * @xattr_name: pointer to the affected extended attribute name
+ * @xattr_value: pointer to the new extended attribute value
+ * @xattr_value_len: pointer to the new extended attribute value length
+ *
+ * Update the HMAC stored in 'security.evm' to reflect the change.
+ */
+void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len)
+{
+ if (!evm_initialized || !evm_protected_xattr(xattr_name))
+ return;
+
+ evm_postxattr(dentry, xattr_name, xattr_value, xattr_value_len);
+ return;
+}
+
+/**
+ * evm_inode_post_removexattr - update 'security.evm' after removing the xattr
+ * @dentry: pointer to the affected dentry
+ * @xattr_name: pointer to the affected extended attribute name
+ *
+ * Update the HMAC stored in 'security.evm' to reflect removal of the xattr.
+ */
+void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
+{
+ if (!evm_initialized || !evm_protected_xattr(xattr_name))
+ return;
+
+ evm_postxattr(dentry, xattr_name, NULL, 0);
+ return;
+}
+
+/**
+ * evm_inode_post_setattr - update 'security.evm' after modifying metadata
+ * @dentry: pointer to the affected dentry
+ * @ia_valid: for the UID and GID status
+ *
+ * For now, update the HMAC stored in 'security.evm' to reflect UID/GID
+ * changes.
+ */
+void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
+{
+ if (!evm_initialized)
+ return;
+
+ if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
+ evm_postxattr(dentry, NULL, NULL, 0);
+ return;
+}
+
+static struct crypto_hash *tfm_hmac; /* preload crypto alg */
+static int __init init_evm(void)
+{
+ int error;
+
+ tfm_hmac = crypto_alloc_hash(evm_hmac, 0, CRYPTO_ALG_ASYNC);
+ error = evm_init_secfs();
+ if (error < 0) {
+ printk(KERN_INFO "EVM: Error registering secfs\n");
+ goto err;
+ }
+err:
+ return error;
+}
+
+static void __exit cleanup_evm(void)
+{
+ evm_cleanup_secfs();
+ crypto_free_hash(tfm_hmac);
+}
+
+/*
+ * evm_display_config - list the EVM protected security extended attributes
+ */
+static int __init evm_display_config(void)
+{
+ char **xattrname;
+
+ for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++)
+ printk(KERN_INFO "EVM: %s\n", *xattrname);
+ return 0;
+}
+
+pure_initcall(evm_display_config);
+late_initcall(init_evm);
+
+MODULE_DESCRIPTION("Extended Verification Module");
+MODULE_LICENSE("GPL");
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
new file mode 100644
index 0000000..eeb2182
--- /dev/null
+++ b/security/integrity/evm/evm_secfs.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm_secfs.c
+ * - Used to signal when key is on keyring
+ * - Get the key and enable EVM
+ */
+
+#include <linux/uaccess.h>
+#include <linux/module.h>
+#include "evm.h"
+
+static struct dentry *evm_init_tpm;
+
+/**
+ * evm_read_key - read() for <securityfs>/evm
+ *
+ * @filp: file pointer, not actually used
+ * @buf: where to put the result
+ * @count: maximum to send along
+ * @ppos: where to start
+ *
+ * Returns number of bytes read or error code, as appropriate
+ */
+static ssize_t evm_read_key(struct file *filp, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ char temp[80];
+ ssize_t rc;
+
+ if (*ppos != 0)
+ return 0;
+
+ sprintf(temp, "%d", evm_initialized);
+ rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+
+ return rc;
+}
+
+/**
+ * evm_write_key - write() for <securityfs>/evm
+ * @file: file pointer, not actually used
+ * @buf: where to get the data from
+ * @count: bytes sent
+ * @ppos: where to start
+ *
+ * Used to signal that key is on the kernel key ring.
+ * - get the integrity hmac key from the kernel key ring
+ * - create list of hmac protected extended attributes
+ * Returns number of bytes written or error code, as appropriate
+ */
+static ssize_t evm_write_key(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ char temp[80];
+ int i, error;
+
+ if (!capable(CAP_MAC_ADMIN) || evm_initialized)
+ return -EPERM;
+
+ if (count >= sizeof(temp) || count == 0)
+ return -EINVAL;
+
+ if (copy_from_user(temp, buf, count) != 0)
+ return -EFAULT;
+
+ temp[count] = '\0';
+
+ if ((sscanf(temp, "%d", &i) != 1) || (i != 1))
+ return -EINVAL;
+
+ error = evm_init_tpmkernkey();
+ printk(KERN_INFO "EVM: %s (%d)\n", (error < 0) ?
+ "tpmkernkey initialization failed" : "tpmkernkey initialized",
+ i);
+ if (!error)
+ evm_initialized = 1;
+ return count;
+}
+
+static const struct file_operations evm_key_ops = {
+ .read = evm_read_key,
+ .write = evm_write_key,
+};
+
+int __init evm_init_secfs(void)
+{
+ int error = 0;
+
+ evm_init_tpm = securityfs_create_file("evm", S_IRUSR | S_IRGRP,
+ NULL, NULL, &evm_key_ops);
+ if (!evm_init_tpm || IS_ERR(evm_init_tpm))
+ error = -EFAULT;
+ return error;
+}
+
+void __exit evm_cleanup_secfs(void)
+{
+ if (evm_init_tpm)
+ securityfs_remove(evm_init_tpm);
+}
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index e2bdd8f..66013f2 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -133,9 +133,11 @@ static void init_once(void *foo)
iint->version = 0;
iint->flags = 0UL;
mutex_init(&iint->mutex);
+ mutex_init(&iint->evm_mutex);
iint->readcount = 0;
iint->writecount = 0;
iint->opencount = 0;
+ iint->hmac_status = INTEGRITY_UNKNOWN;
kref_init(&iint->refcount);
}

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index f1013c9..c83e475 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -31,6 +31,9 @@ struct integrity_iint_cache {
long opencount; /* opens reference count */
struct kref refcount; /* ima_iint_cache reference count */
struct rcu_head rcu;
+ struct mutex evm_mutex; /* protects: hmac_status, hmac */
+ enum integrity_status hmac_status;
+ u8 hmac[MAX_DIGEST_SIZE];
};

/* radix tree calls to lookup, insert, delete
--
1.6.6.1

2010-04-21 21:51:20

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 06/14] security: imbed evm calls in security hooks

Imbed the evm calls evm_inode_setxattr(), evm_inode_post_setxattr(),
evm_inode_removexattr() in the security hooks. evm_inode_setxattr()
protects security.evm xattr. evm_inode_post_setxattr() and
evm_inode_removexattr() updates the hmac associated with an inode.

(Assumes an LSM module protects the setting/removing of xattr.)

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/include/linux/evm.h b/include/linux/evm.h
new file mode 100644
index 0000000..1a30beb
--- /dev/null
+++ b/include/linux/evm.h
@@ -0,0 +1,54 @@
+/*
+ * evm.h
+ *
+ * Copyright (c) 2009 IBM Corporation
+ * Author: Mimi Zohar <[email protected]>
+ */
+
+#ifndef _LINUX_EVM_H
+#define _LINUX_EVM_H
+
+#include <linux/integrity.h>
+
+#ifdef CONFIG_EVM
+extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
+ char *xattr_name,
+ char *xattr_value,
+ size_t xattr_value_len);
+extern int evm_inode_setxattr(struct dentry *dentry, const char *name,
+ const void *value, size_t size);
+extern void evm_inode_post_setxattr(struct dentry *dentry,
+ const char *xattr_name,
+ const void *xattr_value,
+ size_t xattr_value_len);
+extern int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name);
+#else
+static enum integrity_status evm_verifyxattr(struct dentry *dentry,
+ char *xattr_name,
+ char *xattr_value,
+ size_t xattr_value_len)
+{
+ return INTEGRITY_UNKNOWN;
+}
+
+static inline int evm_inode_setxattr(struct dentry *dentry, const char *name,
+ const void *value, size_t size)
+{
+ return 0;
+}
+
+static inline void evm_inode_post_setxattr(struct dentry *dentry,
+ const char *xattr_name,
+ const void *xattr_value,
+ size_t xattr_value_len)
+{
+ return;
+}
+
+static inline int evm_inode_removexattr(struct dentry *dentry,
+ const char *xattr_name)
+{
+ return 0;
+}
+#endif /* CONFIG_EVM_H */
+#endif /* LINUX_EVM_H */
diff --git a/security/security.c b/security/security.c
index e42a78e..6b1e50e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -18,6 +18,7 @@
#include <linux/security.h>
#include <linux/integrity.h>
#include <linux/ima.h>
+#include <linux/evm.h>

/* Boot-time LSM user choice */
static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
@@ -549,9 +550,14 @@ int security_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
int security_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
{
+ int ret;
+
if (unlikely(IS_PRIVATE(dentry->d_inode)))
return 0;
- return security_ops->inode_setxattr(dentry, name, value, size, flags);
+ ret = security_ops->inode_setxattr(dentry, name, value, size, flags);
+ if (ret)
+ return ret;
+ return evm_inode_setxattr(dentry, name, value, size);
}

void security_inode_post_setxattr(struct dentry *dentry, const char *name,
@@ -560,6 +566,7 @@ void security_inode_post_setxattr(struct dentry *dentry, const char *name,
if (unlikely(IS_PRIVATE(dentry->d_inode)))
return;
security_ops->inode_post_setxattr(dentry, name, value, size, flags);
+ evm_inode_post_setxattr(dentry, name, value, size);
}

int security_inode_getxattr(struct dentry *dentry, const char *name)
@@ -578,9 +585,14 @@ int security_inode_listxattr(struct dentry *dentry)

int security_inode_removexattr(struct dentry *dentry, const char *name)
{
+ int ret;
+
if (unlikely(IS_PRIVATE(dentry->d_inode)))
return 0;
- return security_ops->inode_removexattr(dentry, name);
+ ret = security_ops->inode_removexattr(dentry, name);
+ if (ret)
+ return ret;
+ return evm_inode_removexattr(dentry, name);
}

int security_inode_need_killpriv(struct dentry *dentry)
--
1.6.6.1

2010-04-21 21:51:26

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 09/14] evm: inode_post_init

Initialize 'security.evm' for new files. Reduce number of arguments
by defining 'struct xattr'.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/include/linux/evm.h b/include/linux/evm.h
index 8626263..3602bd1 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -9,6 +9,7 @@
#define _LINUX_EVM_H

#include <linux/integrity.h>
+#include <linux/xattr.h>

#ifdef CONFIG_EVM
extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
@@ -25,6 +26,9 @@ extern void evm_inode_post_setxattr(struct dentry *dentry,
extern int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name);
extern void evm_inode_post_removexattr(struct dentry *dentry,
const char *xattr_name);
+extern int evm_inode_post_init_security(struct inode *inode,
+ const struct xattr *new,
+ struct xattr *evm);
#else
static enum integrity_status evm_verifyxattr(struct dentry *dentry,
char *xattr_name,
@@ -65,5 +69,12 @@ static inline void evm_inode_post_removexattr(struct dentry *dentry,
return;
}

+static inline int evm_inode_post_init_security(struct inode *inode,
+ const struct xattr *new,
+ struct xattr *evm)
+{
+ return -EOPNOTSUPP;
+}
+
#endif /* CONFIG_EVM_H */
#endif /* LINUX_EVM_H */
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index d1afa51..e90377e 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -60,6 +60,12 @@ struct xattr_handler {
size_t size, int flags, int handler_flags);
};

+struct xattr {
+ char *name;
+ void *value;
+ size_t value_len;
+};
+
ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 0b6f7b2..a4b6907 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -12,6 +12,7 @@
* File: evm.h
*
*/
+#include <linux/xattr.h>
#include <linux/security.h>
#include "../integrity.h"

@@ -31,5 +32,7 @@ extern int evm_update_evmxattr(struct dentry *dentry,
extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
const char *req_xattr_value,
size_t req_xattr_value_len, char *digest);
+extern int evm_init_hmac(struct inode *inode, struct xattr *xattr,
+ char *hmac_val);
extern int evm_init_secfs(void);
extern void evm_cleanup_secfs(void);
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 4f1be71..5fe322d 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -150,6 +150,25 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
return rc;
}

+int evm_init_hmac(struct inode *inode, struct xattr *lsm_xattr, char *hmac_val)
+{
+ struct hash_desc desc;
+ struct scatterlist sg[1];
+ int error;
+
+ error = init_desc(&desc);
+ if (error != 0) {
+ printk(KERN_INFO "init_desc failed\n");
+ return error;
+ }
+
+ sg_init_one(sg, lsm_xattr->value, lsm_xattr->value_len);
+ crypto_hash_update(&desc, sg, lsm_xattr->value_len);
+ hmac_add_misc(&desc, inode, hmac_val);
+ crypto_free_hash(desc.tfm);
+ return 0;
+}
+
/*
* Get the key from the TPM for the SHA1-HMAC
*/
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 59dc148..acc5176 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -96,6 +96,12 @@ static int evm_protected_xattr(const char *req_xattr_name)
found = 1;
break;
}
+ if (strncmp(req_xattr_name,
+ *xattrname + XATTR_SECURITY_PREFIX_LEN,
+ strlen(req_xattr_name)) == 0) {
+ found = 1;
+ break;
+ }
}
return found;
}
@@ -256,6 +262,36 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
return;
}

+/*
+ * evm_inode_post_init_security - initializes security.evm
+ */
+int evm_inode_post_init_security(struct inode *inode, struct xattr *lsm_xattr,
+ struct xattr *evm_xattr)
+{
+ u8 *hmac_val;
+ int rc;
+
+ if (!evm_initialized || !evm_protected_xattr(lsm_xattr->name))
+ return -EOPNOTSUPP;
+
+ hmac_val = kzalloc(MAX_DIGEST_SIZE, GFP_NOFS);
+ if (!hmac_val)
+ return -ENOMEM;
+
+ rc = evm_init_hmac(inode, lsm_xattr, hmac_val);
+ if (rc < 0)
+ goto out;
+
+ evm_xattr->value = hmac_val;
+ evm_xattr->value_len = evm_hmac_size;
+ evm_xattr->name = XATTR_EVM_SUFFIX;
+ return 0;
+out:
+ kfree(hmac_val);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(evm_inode_post_init_security);
+
static struct crypto_hash *tfm_hmac; /* preload crypto alg */
static int __init init_evm(void)
{
--
1.6.6.1

2010-04-21 21:51:33

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 10/14] fs: add evm_inode_post_init calls

After creating the initial LSM security extended attribute, call
evm_inode_post_init_security() to create the 'security.evm'
extended attribute.

(Support for other fs still needed.)

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index c815584..3987a1b 100644
--- a/fs/ext2/xattr_security.c
+++ b/fs/ext2/xattr_security.c
@@ -8,6 +8,7 @@
#include <linux/fs.h>
#include <linux/ext2_fs.h>
#include <linux/security.h>
+#include <linux/evm.h>
#include "xattr.h"

static size_t
@@ -49,21 +50,37 @@ int
ext2_init_security(struct inode *inode, struct inode *dir)
{
int err;
- size_t len;
- void *value;
- char *name;
+ struct xattr lsm_xattr;
+ struct xattr evm_xattr;

- err = security_inode_init_security(inode, dir, &name, &value, &len);
+ err = security_inode_init_security(inode, dir, &lsm_xattr.name,
+ &lsm_xattr.value,
+ &lsm_xattr.value_len);
if (err) {
if (err == -EOPNOTSUPP)
return 0;
return err;
}
err = ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY,
- name, value, len, 0);
- kfree(name);
- kfree(value);
+ lsm_xattr.name, lsm_xattr.value,
+ lsm_xattr.value_len, 0);
+ if (err < 0)
+ goto out;
+
+ err = evm_inode_post_init_security(inode, &lsm_xattr, &evm_xattr);
+ if (err)
+ goto out;
+ err = ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY,
+ evm_xattr.name, evm_xattr.value,
+ evm_xattr.value_len, 0);
+ kfree(evm_xattr.value);
+out:
+ kfree(lsm_xattr.name);
+ kfree(lsm_xattr.value);
+ if (err == -EOPNOTSUPP)
+ return 0;
return err;
+
}

struct xattr_handler ext2_xattr_security_handler = {
diff --git a/fs/ext3/xattr_security.c b/fs/ext3/xattr_security.c
index 4743487..eba57a9 100644
--- a/fs/ext3/xattr_security.c
+++ b/fs/ext3/xattr_security.c
@@ -9,6 +9,7 @@
#include <linux/ext3_jbd.h>
#include <linux/ext3_fs.h>
#include <linux/security.h>
+#include <linux/evm.h>
#include "xattr.h"

static size_t
@@ -51,20 +52,35 @@ int
ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir)
{
int err;
- size_t len;
- void *value;
- char *name;
+ struct xattr lsm_xattr;
+ struct xattr evm_xattr;

- err = security_inode_init_security(inode, dir, &name, &value, &len);
+ err = security_inode_init_security(inode, dir, &lsm_xattr.name,
+ &lsm_xattr.value,
+ &lsm_xattr.value_len);
if (err) {
if (err == -EOPNOTSUPP)
return 0;
return err;
}
err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
- name, value, len, 0);
- kfree(name);
- kfree(value);
+ lsm_xattr.name, lsm_xattr.value,
+ lsm_xattr.value_len, 0);
+ if (err < 0)
+ goto out;
+
+ err = evm_inode_post_init_security(inode, &lsm_xattr, &evm_xattr);
+ if (err)
+ goto out;
+ err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
+ evm_xattr.name, evm_xattr.value,
+ evm_xattr.value_len, 0);
+ kfree(evm_xattr.value);
+out:
+ kfree(lsm_xattr.name);
+ kfree(lsm_xattr.value);
+ if (err == -EOPNOTSUPP)
+ return 0;
return err;
}

diff --git a/fs/ext4/xattr_security.c b/fs/ext4/xattr_security.c
index 983c253..d8ecbee 100644
--- a/fs/ext4/xattr_security.c
+++ b/fs/ext4/xattr_security.c
@@ -7,6 +7,7 @@
#include <linux/string.h>
#include <linux/fs.h>
#include <linux/security.h>
+#include <linux/evm.h>
#include "ext4_jbd2.h"
#include "ext4.h"
#include "xattr.h"
@@ -51,20 +52,35 @@ int
ext4_init_security(handle_t *handle, struct inode *inode, struct inode *dir)
{
int err;
- size_t len;
- void *value;
- char *name;
+ struct xattr lsm_xattr;
+ struct xattr evm_xattr;

- err = security_inode_init_security(inode, dir, &name, &value, &len);
+ err = security_inode_init_security(inode, dir, &lsm_xattr.name,
+ &lsm_xattr.value,
+ &lsm_xattr.value_len);
if (err) {
if (err == -EOPNOTSUPP)
return 0;
return err;
}
err = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_SECURITY,
- name, value, len, 0);
- kfree(name);
- kfree(value);
+ lsm_xattr.name, lsm_xattr.value,
+ lsm_xattr.value_len, 0);
+ if (err < 0)
+ goto out;
+
+ err = evm_inode_post_init_security(inode, &lsm_xattr, &evm_xattr);
+ if (err)
+ goto out;
+ err = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_SECURITY,
+ evm_xattr.name, evm_xattr.value,
+ evm_xattr.value_len, 0);
+ kfree(evm_xattr.value);
+out:
+ kfree(lsm_xattr.name);
+ kfree(lsm_xattr.value);
+ if (err == -EOPNOTSUPP)
+ return 0;
return err;
}

--
1.6.6.1

2010-04-21 21:51:38

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 08/14] evm: imbed evm_inode_post_setattr

Changing the inode's metadata may require the 'security.evm' extended
attribute to be re-calculated and updated.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/fs/attr.c b/fs/attr.c
index 0815e93..5d9ff4e 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -13,6 +13,7 @@
#include <linux/fsnotify.h>
#include <linux/fcntl.h>
#include <linux/security.h>
+#include <linux/evm.h>

/* Taken over from the old code... */

@@ -218,8 +219,10 @@ int notify_change(struct dentry * dentry, struct iattr * attr)
if (ia_valid & ATTR_SIZE)
up_write(&dentry->d_inode->i_alloc_sem);

- if (!error)
+ if (!error) {
fsnotify_change(dentry, ia_valid);
+ evm_inode_post_setattr(dentry, ia_valid);
+ }

return error;
}
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 93edadd..8626263 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -15,6 +15,7 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
char *xattr_name,
char *xattr_value,
size_t xattr_value_len);
+extern void evm_inode_post_setattr(struct dentry *dentry, int ia_valid);
extern int evm_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size);
extern void evm_inode_post_setxattr(struct dentry *dentry,
@@ -33,6 +34,11 @@ static enum integrity_status evm_verifyxattr(struct dentry *dentry,
return INTEGRITY_UNKNOWN;
}

+static inline void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
+{
+ return;
+}
+
static inline int evm_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size)
{
--
1.6.6.1

2010-04-21 21:51:44

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 12/14] ima: appraise default rules

Unlike the IMA measurement policy, the appraise policy can not be
dependent on runtime process information, such as the task uid,
as the 'security.ima' xattr is written on file close and must
be updated each time the file changes, regardless of the current
task uid. The appraise default policy appraises all files owned
by root.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index aabd615..7cc028d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -122,6 +122,8 @@ void iint_rcu_free(struct rcu_head *rcu);
enum ima_hooks { FILE_CHECK = 1, FILE_MMAP, BPRM_CHECK, POST_SETATTR };

int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask);
+int ima_match_appraise_policy(struct inode *inode, enum ima_hooks func,
+ int mask);
void ima_init_policy(void);
void ima_update_policy(void);
ssize_t ima_parse_add_rule(char *);
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 0afb1b4..ad8e0ac 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -28,7 +28,19 @@ __setup("ima_appraise=", default_appraise_setup);
int ima_must_appraise(struct integrity_iint_cache *iint, struct inode *inode,
enum ima_hooks func, int mask)
{
- return 0;
+ int must_appraise, rc = 0;
+
+ if (!ima_appraise || !inode->i_op->getxattr)
+ return 0;
+ else if (iint->flags & IMA_APPRAISED)
+ return 0;
+
+ must_appraise = ima_match_appraise_policy(inode, func, mask);
+ if (must_appraise) {
+ iint->flags |= IMA_APPRAISE;
+ rc = 1;
+ }
+ return rc;
}

static void ima_fix_xattr(struct dentry *dentry,
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 778a735..7c9f15a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -23,8 +23,11 @@
#define IMA_MASK 0x0002
#define IMA_FSMAGIC 0x0004
#define IMA_UID 0x0008
+#define IMA_OWNER 0x0010

-enum ima_action { UNKNOWN = -1, DONT_MEASURE = 0, MEASURE };
+enum ima_action { UNKNOWN = -1,
+ DONT_MEASURE = 0, MEASURE,
+ DONT_APPRAISE, APPRAISE};

#define MAX_LSM_RULES 6
enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
@@ -39,6 +42,7 @@ struct ima_measure_rule_entry {
int mask;
unsigned long fsmagic;
uid_t uid;
+ uid_t owner;
struct {
void *rule; /* LSM file metadata specific */
int type; /* audit type */
@@ -47,7 +51,7 @@ struct ima_measure_rule_entry {

/*
* Without LSM specific knowledge, the default policy can only be
- * written in terms of .action, .func, .mask, .fsmagic, and .uid
+ * written in terms of .action, .func, .mask, .fsmagic, .uid, and .owner
*/

/*
@@ -69,6 +73,13 @@ static struct ima_measure_rule_entry default_rules[] = {
.flags = IMA_FUNC | IMA_MASK},
{.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = 0,
.flags = IMA_FUNC | IMA_MASK | IMA_UID},
+ {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = APPRAISE,.owner = 0,.flags = IMA_OWNER},
};

static LIST_HEAD(measure_default_rules);
@@ -109,6 +120,8 @@ static bool ima_match_rules(struct ima_measure_rule_entry *rule,
return false;
if ((rule->flags & IMA_UID) && rule->uid != tsk->cred->uid)
return false;
+ if ((rule->flags & IMA_OWNER) && rule->owner != inode->i_uid)
+ return false;
for (i = 0; i < MAX_LSM_RULES; i++) {
int rc = 0;
u32 osid, sid;
@@ -165,6 +178,9 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask)
list_for_each_entry(entry, ima_measure, list) {
bool rc;

+ if ((entry->action == APPRAISE) ||
+ (entry->action == DONT_APPRAISE))
+ continue;
rc = ima_match_rules(entry, inode, func, mask);
if (rc)
return entry->action;
@@ -172,6 +188,28 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask)
return 0;
}

+int ima_match_appraise_policy(struct inode *inode, enum ima_hooks func,
+ int mask)
+{
+ struct ima_measure_rule_entry *entry;
+
+ list_for_each_entry(entry, ima_measure, list) {
+ bool rc;
+
+ if ((entry->action == MEASURE) ||
+ (entry->action == DONT_MEASURE))
+ continue;
+ rc = ima_match_rules(entry, inode, func, mask);
+ if (rc) {
+ if (entry->action == DONT_APPRAISE)
+ return 0;
+ if (entry->action == APPRAISE)
+ return 1;
+ }
+ }
+ return 0;
+}
+
/**
* ima_init_policy - initialize the default measure rules.
*
@@ -219,6 +257,7 @@ void ima_update_policy(void)
enum {
Opt_err = -1,
Opt_measure = 1, Opt_dont_measure,
+ Opt_appraise, Opt_dont_appraise,
Opt_obj_user, Opt_obj_role, Opt_obj_type,
Opt_subj_user, Opt_subj_role, Opt_subj_type,
Opt_func, Opt_mask, Opt_fsmagic, Opt_uid
@@ -227,6 +266,8 @@ enum {
static match_table_t policy_tokens = {
{Opt_measure, "measure"},
{Opt_dont_measure, "dont_measure"},
+ {Opt_appraise, "appraise"},
+ {Opt_dont_appraise, "dont_appraise"},
{Opt_obj_user, "obj_user=%s"},
{Opt_obj_role, "obj_role=%s"},
{Opt_obj_type, "obj_type=%s"},
@@ -299,6 +340,22 @@ static int ima_parse_rule(char *rule, struct ima_measure_rule_entry *entry)

entry->action = DONT_MEASURE;
break;
+ case Opt_appraise:
+ ima_log_string(ab, "%s ", "appraise");
+
+ if (entry->action != UNKNOWN)
+ result = -EINVAL;
+
+ entry->action = APPRAISE;
+ break;
+ case Opt_dont_appraise:
+ ima_log_string(ab, "%s ", "dont_appraise");
+
+ if (entry->action != UNKNOWN)
+ result = -EINVAL;
+
+ entry->action = DONT_APPRAISE;
+ break;
case Opt_func:
ima_log_string(ab, "func", args[0].from);

--
1.6.6.1

2010-04-21 21:51:47

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 11/14] ima: integrity appraisal extension

This patch adds local measurement integrity appraisal to IMA. (Taken
from the original EVM postings.) New is the creation and maintainance
of an extended attribute 'security.ima' containing the file hash
measurement. Protection of the xattr is provided by EVM, if enabled
and configured.

Based on policy, IMA calls evm_verifyxattr() to verify the security.ima
xattr integrity and, assuming success, compares the xattr hash value with
the collected file measurement.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 23f95ea..26eecc9 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -962,6 +962,10 @@ and is between 256 and 4096 characters. It is defined in the file
ihash_entries= [KNL]
Set number of hash buckets for inode cache.

+ ima_appraise= [IMA] appraise integrity measurements
+ Format: { "off" | "enforce" | "log" | "fix" }
+ default: "enforce"
+
ima_audit= [IMA]
Format: { "0" | "1" }
0 -- integrity auditing messages. (Default)
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index e90377e..4f6608f 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -37,6 +37,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

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index acc5176..278e0de 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -32,6 +32,9 @@ char *evm_config_xattrnames[] = {
#ifdef CONFIG_SECURITY_SMACK
XATTR_NAME_SMACK,
#endif
+#ifdef CONFIG_IMA
+ XATTR_NAME_IMA,
+#endif
XATTR_NAME_CAPS,
NULL
};
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 66013f2..b182ce5 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -137,6 +137,7 @@ static void init_once(void *foo)
iint->readcount = 0;
iint->writecount = 0;
iint->opencount = 0;
+ iint->hash_status = INTEGRITY_UNKNOWN;
iint->hmac_status = INTEGRITY_UNKNOWN;
kref_init(&iint->refcount);
}
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index dc60e40..39abced 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -55,3 +55,17 @@ config IMA_LSM_RULES
default y
help
Disabling this option will disregard LSM based policy rules.
+
+config IMA_APPRAISE
+ bool "Appraise integrity measurements"
+ depends on IMA
+ default n
+ help
+ This option enables local measurement integrity appraisal.
+ To protect extended attributes from offline attack, enable
+ and configure EVM.
+
+ For more information on integrity appraisal refer to:
+ <http://linux-ima.sourceforge.net>
+ If unsure, say N.
+
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 5690c02..bd31516 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -7,3 +7,5 @@ obj-$(CONFIG_IMA) += ima.o

ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
ima_policy.o ima_audit.o
+
+ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a58ac9f..aabd615 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -37,9 +37,11 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
#define IMA_MEASURE_HTABLE_SIZE (1 << IMA_HASH_BITS)

/* set during initialization */
+extern char *ima_xattr_name;
extern int ima_initialized;
extern int ima_used_chip;
extern char *ima_hash;
+extern int ima_appraise;

/* IMA inode template definition */
struct ima_template_data {
@@ -97,8 +99,9 @@ static inline unsigned long ima_hash_key(u8 *digest)
}

/* LIM API function definitions */
-int ima_must_measure(struct integrity_iint_cache *iint, struct inode *inode,
- int mask, int function);
+int ima_must_appraise_or_measure(struct integrity_iint_cache *iint,
+ struct inode *inode, int mask, int function,
+ int *must_measure, int *must_appraise);
int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file);
void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
@@ -116,7 +119,7 @@ void iint_free(struct kref *kref);
void iint_rcu_free(struct rcu_head *rcu);

/* IMA policy related functions */
-enum ima_hooks { FILE_CHECK = 1, FILE_MMAP, BPRM_CHECK };
+enum ima_hooks { FILE_CHECK = 1, FILE_MMAP, BPRM_CHECK, POST_SETATTR };

int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask);
void ima_init_policy(void);
@@ -124,6 +127,39 @@ void ima_update_policy(void);
ssize_t ima_parse_add_rule(char *);
void ima_delete_rules(void);

+/* Appraise integrity measurements */
+#ifdef CONFIG_IMA_APPRAISE
+#define IMA_APPRAISE_ENABLED 0x01
+#define IMA_APPRAISE_ENFORCE 0x02
+#define IMA_APPRAISE_LOG 0x04
+#define IMA_APPRAISE_FIX 0x08
+
+int ima_appraise_measurement(struct integrity_iint_cache *iint,
+ struct file *file, const unsigned char *filename);
+int ima_must_appraise(struct integrity_iint_cache *iint, struct inode *inode,
+ enum ima_hooks func, int mask);
+void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
+
+#else
+static inline int ima_appraise_measurement(struct integrity_iint_cache *iint,
+ struct file *file,
+ const unsigned char *filename)
+{
+ return INTEGRITY_UNKNOWN;
+}
+
+static inline int ima_must_appraise(struct integrity_iint_cache *iint,
+ struct inode *inode,
+ enum ima_hooks func, int mask)
+{
+}
+
+static inline void ima_update_xattr(struct integrity_iint_cache *iint,
+ struct file *file)
+{
+}
+#endif
+
/* LSM based policy rules require audit */
#ifdef CONFIG_IMA_LSM_RULES

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index bb307fb..8c5375c 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -9,12 +9,17 @@
* License.
*
* File: ima_api.c
- * Implements must_measure, collect_measurement, store_measurement,
- * and store_template.
+ * Implements must_appraise_or_measure, collect_measurement,
+ * appraise_measurement, store_measurement and store_template.
*/
#include <linux/module.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/xattr.h>
+#include <linux/evm.h>

#include "ima.h"
+
static const char *IMA_TEMPLATE_NAME = "ima";

/*
@@ -92,10 +97,12 @@ err_out:
}

/**
- * ima_must_measure - measure decision based on policy.
+ * ima_must_appraise_or_measure - appraise & measure decision based on policy.
* @inode: pointer to inode to measure
* @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXECUTE)
* @function: calling function (FILE_CHECK, BPRM_CHECK, FILE_MMAP)
+ * @must_measure: pointer to measure flag
+ * @must_appraise: pointer to appraise flag
*
* The policy is defined in terms of keypairs:
* subj=, obj=, type=, func=, mask=, fsmagic=
@@ -106,20 +113,32 @@ err_out:
*
* Must be called with iint->mutex held.
*
- * Return 0 to measure. Return 1 if already measured.
+ * Set must_appraise to 1 to appraise measurement.
+ * Set must_measure to 1 to add to measurement list.
+ *
* For matching a DONT_MEASURE policy, no policy, or other
- * error, return an error code.
+ * error, return an error code, otherwise 0.
*/
-int ima_must_measure(struct integrity_iint_cache *iint, struct inode *inode,
- int mask, int function)
+int ima_must_appraise_or_measure(struct integrity_iint_cache *iint,
+ struct inode *inode, int mask, int function,
+ int *must_measure, int *must_appraise)
{
- int must_measure;
+ int rc;

- if (iint->flags & IMA_MEASURED)
- return 1;
+ if (must_appraise)
+ *must_appraise = ima_must_appraise(iint, inode, function, mask);

- must_measure = ima_match_policy(inode, function, mask);
- return must_measure ? 0 : -EACCES;
+ if (iint->flags & IMA_MEASURED) {
+ if (must_measure)
+ *must_measure = 0;
+ return 0;
+ }
+ rc = ima_match_policy(inode, function, mask);
+ if (rc)
+ iint->flags |= IMA_MEASURE;
+ if (must_measure)
+ *must_measure = rc ? 1 : 0;
+ return rc ? 0 : -EACCES;
}

/*
@@ -135,15 +154,17 @@ int ima_must_measure(struct integrity_iint_cache *iint, struct inode *inode,
int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file)
{
- int result = -EEXIST;
+ int result = 0;

- if (!(iint->flags & IMA_MEASURED)) {
+ 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);
- if (!result)
+ if (!result) {
iint->version = i_version;
+ iint->flags |= IMA_COLLECTED;
+ }
}
return result;
}
@@ -173,6 +194,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
struct ima_template_entry *entry;
int violation = 0;

+ if (iint->flags & IMA_MEASURED)
+ return;
+
entry = kmalloc(sizeof(*entry), GFP_KERNEL);
if (!entry) {
integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
new file mode 100644
index 0000000..0afb1b4
--- /dev/null
+++ b/security/integrity/ima/ima_appraise.c
@@ -0,0 +1,140 @@
+#include <linux/module.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/xattr.h>
+#include <linux/magic.h>
+#include <linux/evm.h>
+
+#include "ima.h"
+
+static int __init default_appraise_setup(char *str)
+{
+ if (strncmp(str, "off", 3) == 0)
+ ima_appraise = 0;
+ else if (strncmp(str, "log", 3) == 0)
+ ima_appraise = IMA_APPRAISE_ENABLED | IMA_APPRAISE_LOG;
+ else if (strncmp(str, "fix", 3) == 0)
+ ima_appraise = IMA_APPRAISE_ENABLED | IMA_APPRAISE_FIX;
+ return 1;
+}
+
+__setup("ima_appraise=", default_appraise_setup);
+
+/*
+ * ima_must_appraise - set appraise flag
+ *
+ * Return 1 to appraise
+ */
+int ima_must_appraise(struct integrity_iint_cache *iint, struct inode *inode,
+ enum ima_hooks func, int mask)
+{
+ return 0;
+}
+
+static void ima_fix_xattr(struct dentry *dentry,
+ struct integrity_iint_cache *iint)
+{
+ __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
+ iint->digest, IMA_DIGEST_SIZE, 0);
+}
+
+/*
+ * ima_appraise_measurement - appraise file measurement
+ *
+ * Call evm_verifyxattr() to verify the integrity of 'security.ima'.
+ * Assuming success, compare the xattr hash with the collected measurement.
+ *
+ * Return 0 on success, error code otherwise
+ */
+int ima_appraise_measurement(struct integrity_iint_cache *iint,
+ struct file *file, const unsigned char *filename)
+{
+ struct dentry *dentry = file->f_dentry;
+ struct inode *inode = dentry->d_inode;
+ u8 xattr_value[IMA_DIGEST_SIZE];
+ enum integrity_status status = INTEGRITY_UNKNOWN;
+ const char *op = "appraise_data";
+ char *cause = "unknown";
+ int rc;
+
+ if (!ima_appraise || !inode->i_op->getxattr)
+ return 0;
+ if (iint->flags & IMA_APPRAISED)
+ return iint->hash_status;
+
+ rc = inode->i_op->getxattr(dentry, XATTR_NAME_IMA, xattr_value,
+ IMA_DIGEST_SIZE);
+ if (rc < 0)
+ goto err_out;
+ status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc);
+ if ((rc > 0)
+ && ((status == INTEGRITY_PASS) || (status == INTEGRITY_UNKNOWN))) {
+ if (memcmp(xattr_value, iint->digest, IMA_DIGEST_SIZE) != 0) {
+ status = INTEGRITY_FAIL;
+ cause = "invalid-hash";
+ print_hex_dump_bytes("security.ima: ", DUMP_PREFIX_NONE,
+ xattr_value, IMA_DIGEST_SIZE);
+ print_hex_dump_bytes("collected: ", DUMP_PREFIX_NONE,
+ iint->digest, IMA_DIGEST_SIZE);
+ if (ima_appraise & IMA_APPRAISE_FIX)
+ ima_fix_xattr(dentry, iint);
+ integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
+ filename, op, cause, 1, 0);
+ } else {
+ status = INTEGRITY_PASS;
+
+ if ((status == INTEGRITY_UNKNOWN)
+ && (ima_appraise & IMA_APPRAISE_FIX))
+ ima_fix_xattr(dentry, iint);
+ }
+ iint->flags |= IMA_APPRAISED;
+ } else {
+ if (status == INTEGRITY_NOLABEL)
+ cause = "missing-HMAC";
+ else if (status == INTEGRITY_FAIL)
+ cause = "invalid-HMAC";
+
+ if (ima_appraise & IMA_APPRAISE_FIX)
+ ima_fix_xattr(dentry, iint);
+ integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
+ op, cause, 1, 0);
+ }
+ iint->hash_status = status;
+ return status;
+err_out:
+ if (rc == -ENODATA) {
+ if (iint->version == 1)
+ return 0;
+ cause = "missing-hash";
+ if (ima_appraise & IMA_APPRAISE_FIX)
+ ima_fix_xattr(dentry, iint);
+ integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
+ op, cause, 1, 0);
+ iint->hash_status = INTEGRITY_NOLABEL;
+ return iint->hash_status;
+ }
+ printk(KERN_INFO "%s: %d %s\n", __func__, rc, filename);
+ return 0;
+}
+
+/*
+ * ima_update_xattr - update 'security.ima' hash value
+ */
+void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
+{
+ struct dentry *dentry = file->f_dentry;
+ int read = 0;
+ int rc = 0;
+
+ if (!(file->f_mode & FMODE_READ)) {
+ file->f_mode |= FMODE_READ;
+ read = 1;
+ }
+ rc = ima_collect_measurement(iint, file);
+ if (read)
+ file->f_mode &= ~FMODE_READ;
+ if (rc < 0)
+ return;
+ __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
+ iint->digest, IMA_DIGEST_SIZE, 0);
+}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 7eeb76e..635a3be 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -22,11 +22,18 @@
#include <linux/binfmts.h>
#include <linux/mount.h>
#include <linux/mman.h>
+#include <linux/xattr.h>

#include "ima.h"

int ima_initialized;

+#ifdef CONFIG_IMA_APPRAISE
+int ima_appraise = IMA_APPRAISE_ENABLED | IMA_APPRAISE_ENFORCE;
+#else
+int ima_appraise;
+#endif
+
char *ima_hash = "sha1";
static int __init hash_setup(char *str)
{
@@ -154,7 +161,8 @@ void ima_counts_get(struct file *file)
if (!iint)
return;
mutex_lock(&iint->mutex);
- rc = ima_must_measure(iint, inode, MAY_READ, FILE_CHECK);
+ rc = ima_must_appraise_or_measure(iint, inode, MAY_READ, FILE_CHECK,
+ NULL, NULL);
if (rc < 0)
goto out;

@@ -185,8 +193,12 @@ static void ima_dec_counts(struct integrity_iint_cache *iint,
if (mode & FMODE_WRITE) {
iint->writecount--;
if (iint->writecount == 0) {
- if (iint->version != inode->i_version)
- iint->flags &= ~IMA_MEASURED;
+ if (iint->version != inode->i_version) {
+ iint->flags &= ~(IMA_COLLECTED | IMA_APPRAISED
+ | IMA_MEASURED);
+ if (iint->flags & IMA_APPRAISE)
+ ima_update_xattr(iint, file);
+ }
}
}

@@ -199,6 +211,7 @@ static void ima_dec_counts(struct integrity_iint_cache *iint,
iint->opencount);
dump_stack();
}
+ return;
}

/**
@@ -230,7 +243,7 @@ static int process_measurement(struct file *file, const unsigned char *filename,
{
struct inode *inode = file->f_dentry->d_inode;
struct integrity_iint_cache *iint;
- int rc;
+ int rc, err = 0, must_measure, must_appraise;

if (!ima_initialized || !S_ISREG(inode->i_mode))
return 0;
@@ -239,17 +252,25 @@ static int process_measurement(struct file *file, const unsigned char *filename,
return -ENOMEM;

mutex_lock(&iint->mutex);
- rc = ima_must_measure(iint, inode, mask, function);
- if (rc != 0)
+ rc = ima_must_appraise_or_measure(iint, inode, mask, function,
+ &must_measure, &must_appraise);
+ if (!must_measure && !must_appraise) {
+ if (iint->flags & IMA_APPRAISED)
+ err = iint->hash_status;
goto out;
+ }

rc = ima_collect_measurement(iint, file);
- if (!rc)
+ if (rc != 0)
+ goto out;
+ if (must_measure)
ima_store_measurement(iint, file, filename);
+ if (must_appraise)
+ err = ima_appraise_measurement(iint, file, filename);
out:
mutex_unlock(&iint->mutex);
kref_put(&iint->refcount, iint_free);
- return rc;
+ return (!err) ? 0 : -EACCES;
}

/**
@@ -265,14 +286,14 @@ out:
*/
int ima_file_mmap(struct file *file, unsigned long prot)
{
- int rc;
+ int rc = 0;

if (!file)
return 0;
if (prot & PROT_EXEC)
rc = process_measurement(file, file->f_dentry->d_name.name,
MAY_EXEC, FILE_MMAP);
- return 0;
+ return (ima_appraise & IMA_APPRAISE_ENFORCE) ? rc : 0;
}

/**
@@ -294,7 +315,7 @@ int ima_bprm_check(struct linux_binprm *bprm)

rc = process_measurement(bprm->file, bprm->filename,
MAY_EXEC, BPRM_CHECK);
- return 0;
+ return (ima_appraise & IMA_APPRAISE_ENFORCE) ? rc : 0;
}

/**
@@ -314,10 +335,41 @@ int ima_file_check(struct file *file, int mask)
rc = process_measurement(file, file->f_dentry->d_name.name,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC),
FILE_CHECK);
- return 0;
+ return (ima_appraise & IMA_APPRAISE_ENFORCE) ? rc : 0;
}
EXPORT_SYMBOL_GPL(ima_file_check);

+/**
+ * ima_inode_post_setattr - reflect file metadata changes
+ * @dentry: pointer to the affected dentry
+ *
+ * Changes to a dentry's metadata might result in needing to appraise
+ */
+void ima_inode_post_setattr(struct dentry *dentry)
+{
+ struct inode *inode = dentry->d_inode;
+ struct integrity_iint_cache *iint;
+ int must_appraise, rc;
+
+ if (!ima_initialized || !ima_appraise || !S_ISREG(inode->i_mode)
+ || !inode->i_op->removexattr)
+ return;
+
+ iint = integrity_iint_find_get(inode);
+ if (!iint)
+ return;
+
+ mutex_lock(&iint->mutex);
+ iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED);
+ must_appraise = ima_must_appraise(iint, inode, MAY_ACCESS,
+ POST_SETATTR);
+ if (!must_appraise)
+ rc = inode->i_op->removexattr(dentry, XATTR_NAME_IMA);
+ mutex_unlock(&iint->mutex);
+ kref_put(&iint->refcount, iint_free);
+ return;
+}
+
static int __init init_ima(void)
{
int error;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index c83e475..3a9b3de 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -18,13 +18,18 @@
#define MAX_DIGEST_SIZE SHA1_DIGEST_SIZE

/* iint cache flags */
-#define IMA_MEASURED 1
+#define IMA_MEASURE 1
+#define IMA_MEASURED 2
+#define IMA_APPRAISE 4
+#define IMA_APPRAISED 8
+#define IMA_COLLECTED 16

/* integrity data associated with an inode */
struct integrity_iint_cache {
u64 version; /* track inode changes */
unsigned long flags;
u8 digest[MAX_DIGEST_SIZE];
+ enum integrity_status hash_status;
struct mutex mutex; /* protects: version, flags, digest */
long readcount; /* measured files readcount */
long writecount; /* measured files writecount */
--
1.6.6.1

2010-04-21 21:52:06

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 13/14] ima: inode post_setattr

Changing an inode's metadata may result in our not needing to
appraise the file. In such cases, we must remove 'security.ima'.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/fs/attr.c b/fs/attr.c
index 5d9ff4e..9038baf 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -14,6 +14,7 @@
#include <linux/fcntl.h>
#include <linux/security.h>
#include <linux/evm.h>
+#include <linux/ima.h>

/* Taken over from the old code... */

@@ -221,6 +222,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 4dce900..ce82e29 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);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
extern void ima_counts_get(struct file *file);
+extern void ima_inode_post_setattr(struct dentry *dentry);

#else
static inline int ima_bprm_check(struct linux_binprm *bprm)
@@ -46,5 +47,10 @@ static inline void ima_counts_get(struct file *file)
return;
}

+static inline void ima_inode_post_setattr(struct dentry *dentry)
+{
+ return;
+}
+
#endif /* CONFIG_IMA_H */
#endif /* _LINUX_IMA_H */
--
1.6.6.1

2010-04-21 21:52:16

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 14/14] ima: add ima_inode_setxattr and ima_inode_removexattr

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'.

Signed-off-by: Mimi Zohar <[email protected]>

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ce82e29..3307420 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -20,6 +20,9 @@ extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
extern void ima_counts_get(struct file *file);
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 int ima_bprm_check(struct linux_binprm *bprm)
@@ -52,5 +55,15 @@ static inline void ima_inode_post_setattr(struct dentry *dentry)
return;
}

+int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len)
+{
+ return 0;
+}
+
+int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name)
+{
+ return 0;
+}
#endif /* CONFIG_IMA_H */
#endif /* _LINUX_IMA_H */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 635a3be..e5a52a6 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -370,6 +370,32 @@ void ima_inode_post_setattr(struct dentry *dentry)
return;
}

+/*
+ * ima_protect_xattr - protect 'security.ima'
+ *
+ * Ensure that not just anyone can modify or remove 'security.ima'.
+ */
+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)
+ && !capable(CAP_MAC_ADMIN))
+ return -EPERM;
+ return 0;
+}
+
+int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len)
+{
+ return ima_protect_xattr(dentry, xattr_name, xattr_value,
+ xattr_value_len);
+}
+
+int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name)
+{
+ return ima_protect_xattr(dentry, xattr_name, NULL, 0);
+}
+
static int __init init_ima(void)
{
int error;
diff --git a/security/security.c b/security/security.c
index 6b1e50e..9345731 100644
--- a/security/security.c
+++ b/security/security.c
@@ -557,6 +557,9 @@ int security_inode_setxattr(struct dentry *dentry, const char *name,
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);
}

@@ -592,6 +595,9 @@ int security_inode_removexattr(struct dentry *dentry, const char *name)
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);
}

--
1.6.6.1

2010-04-21 21:51:07

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 03/14] xattr: define vfs_getxattr_alloc and vfs_xattr_cmp

vfs_getxattr_alloc() and vfs_xattr_cmp() are two new kernel xattr
helper functions. vfs_getxattr_alloc() first allocates memory for
the requested xattr and then retrieves it. vfs_xattr_cmp() compares
a given value with the contents of an extended attribute.

Signed-off-by: Mimi Zohar <[email protected]>

diff --git a/fs/xattr.c b/fs/xattr.c
index 46f87e8..341ad71 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -159,6 +159,64 @@ out_noalloc:
}
EXPORT_SYMBOL_GPL(xattr_getsecurity);

+/*
+ * vfs_getxattr_alloc - allocate memory, if necessary, before calling getxattr
+ *
+ * Allocate memory, if not already allocated, or re-allocate correct size,
+ * before retrieving the extended attribute.
+ *
+ * Returns the result of alloc, if failed, or the getxattr operation.
+ */
+ssize_t
+vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value,
+ size_t xattr_size, gfp_t flags)
+{
+ struct inode *inode = dentry->d_inode;
+ char *value = *xattr_value;
+ int error;
+
+ error = xattr_permission(inode, name, MAY_READ);
+ if (error)
+ return error;
+
+ if (!inode->i_op->getxattr)
+ return -EOPNOTSUPP;
+
+ error = inode->i_op->getxattr(dentry, name, NULL, 0);
+ if (error < 0)
+ return error;
+
+ if (!value || (error > xattr_size)) {
+ value = krealloc(*xattr_value, error + 1, flags);
+ if (!value)
+ return -ENOMEM;
+ memset(value, 0, error + 1);
+ }
+
+ error = inode->i_op->getxattr(dentry, name, value, error);
+ *xattr_value = value;
+ return error;
+}
+
+/* Compare an extended attribute value with the given value */
+int vfs_xattr_cmp(struct dentry *dentry, const char *xattr_name,
+ const char *value, size_t size, gfp_t flags)
+{
+ char *xattr_value = NULL;
+ int rc;
+
+ rc = vfs_getxattr_alloc(dentry, xattr_name, &xattr_value, 0, flags);
+ if (rc < 0)
+ return rc;
+
+ if ((rc != size) || (memcmp(xattr_value, value, rc) != 0))
+ rc = -EINVAL;
+ else
+ rc = 0;
+ kfree(xattr_value);
+ return rc;
+}
+
ssize_t
vfs_getxattr(struct dentry *dentry, const char *name, void *value, size_t size)
{
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index d079bec..8698de3 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -68,7 +68,10 @@ ssize_t generic_getxattr(struct dentry *dentry, const char *name, void *buffer,
ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size);
int generic_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags);
int generic_removexattr(struct dentry *dentry, const char *name);
-
+ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name,
+ char **xattr_value, size_t size, gfp_t flags);
+int vfs_xattr_cmp(struct dentry *dentry, const char *xattr_name,
+ const char *value, size_t size, gfp_t flags);
#endif /* __KERNEL__ */

#endif /* _LINUX_XATTR_H */
--
1.6.6.1

2010-04-21 21:52:53

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 07/14] evm: inode post removexattr

When an EVM protected extended attribute is removed, update 'security.evm'.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/fs/xattr.c b/fs/xattr.c
index 341ad71..9372e77 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -14,6 +14,7 @@
#include <linux/mount.h>
#include <linux/namei.h>
#include <linux/security.h>
+#include <linux/evm.h>
#include <linux/syscalls.h>
#include <linux/module.h>
#include <linux/fsnotify.h>
@@ -294,8 +295,10 @@ vfs_removexattr(struct dentry *dentry, const char *name)
error = inode->i_op->removexattr(dentry, name);
mutex_unlock(&inode->i_mutex);

- if (!error)
+ if (!error) {
fsnotify_xattr(dentry);
+ evm_inode_post_removexattr(dentry, name);
+ }
return error;
}
EXPORT_SYMBOL_GPL(vfs_removexattr);
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 1a30beb..93edadd 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -22,6 +22,8 @@ extern void evm_inode_post_setxattr(struct dentry *dentry,
const void *xattr_value,
size_t xattr_value_len);
extern int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name);
+extern void evm_inode_post_removexattr(struct dentry *dentry,
+ const char *xattr_name);
#else
static enum integrity_status evm_verifyxattr(struct dentry *dentry,
char *xattr_name,
@@ -50,5 +52,12 @@ static inline int evm_inode_removexattr(struct dentry *dentry,
{
return 0;
}
+
+static inline void evm_inode_post_removexattr(struct dentry *dentry,
+ const char *xattr_name)
+{
+ return;
+}
+
#endif /* CONFIG_EVM_H */
#endif /* LINUX_EVM_H */
--
1.6.6.1

2010-04-21 21:53:15

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 05/14] ima: move ima_file_free before releasing the file

Integrity appraisal measures files on file_free and stores the file
measurement as an xattr. Measure the file before releasing it.

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>

diff --git a/fs/file_table.c b/fs/file_table.c
index 32d12b7..fac3e43 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -250,10 +250,10 @@ void __fput(struct file *file)
if (file->f_op && file->f_op->fasync)
file->f_op->fasync(-1, file, 0);
}
+ ima_file_free(file);
if (file->f_op && file->f_op->release)
file->f_op->release(inode, file);
security_file_free(file);
- ima_file_free(file);
if (unlikely(S_ISCHR(inode->i_mode) && inode->i_cdev != NULL))
cdev_put(inode->i_cdev);
fops_put(file->f_op);
--
1.6.6.1

2010-04-21 22:01:10

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Wed, 21 Apr 2010 17:49:40 -0400 Mimi Zohar wrote:

> Extended Verification Module(EVM) detects offline tampering of the
> security extended attributes (e.g. security.selinux, security.SMACK64,
> security.ima), which is the basis for LSM permission decisions and,
> with this set of patches, integrity appraisal decisions. To detect
> offline tampering of the extended attributes, EVM maintains an
> HMAC-sha1 across a set of security extended attributes, storing the
> HMAC as the extended attribute 'security.evm'. To verify the integrity
> of an extended attribute, EVM exports evm_verifyxattr(), which
> re-calculates the HMAC and compares it with the version stored in
> 'security.evm'.
>
...
>
> Much appreciation to Dave Hansen, Serge Hallyn, and Matt Helsley for
> reviewing the patches.
>
> Mimi
>
> Mimi Zohar (14):
> integrity: move ima inode integrity data management
> security: move LSM xattrnames to xattr.h
> xattr: define vfs_getxattr_alloc and vfs_xattr_cmp
> evm: re-release
> ima: move ima_file_free before releasing the file
> security: imbed evm calls in security hooks
> evm: inode post removexattr
> evm: imbed evm_inode_post_setattr
> evm: inode_post_init
> fs: add evm_inode_post_init calls
> ima: integrity appraisal extension
> ima: appraise default rules
> ima: inode post_setattr
> ima: add ima_inode_setxattr and ima_inode_removexattr
> --

A summary diffstat would be good to see in patch 00/14.

Lacking that, at least each individual patch should have a diffstat summary
in it. Please read Documentation/SubmittingPatches.

---
~Randy

2010-04-21 22:19:04

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Wed, 2010-04-21 at 14:58 -0700, Randy Dunlap wrote:
> On Wed, 21 Apr 2010 17:49:40 -0400 Mimi Zohar wrote:
>
> > Extended Verification Module(EVM) detects offline tampering of the
> > security extended attributes (e.g. security.selinux, security.SMACK64,
> > security.ima), which is the basis for LSM permission decisions and,
> > with this set of patches, integrity appraisal decisions. To detect
> > offline tampering of the extended attributes, EVM maintains an
> > HMAC-sha1 across a set of security extended attributes, storing the
> > HMAC as the extended attribute 'security.evm'. To verify the integrity
> > of an extended attribute, EVM exports evm_verifyxattr(), which
> > re-calculates the HMAC and compares it with the version stored in
> > 'security.evm'.
> >
> ...
> >
> > Much appreciation to Dave Hansen, Serge Hallyn, and Matt Helsley for
> > reviewing the patches.
> >
> > Mimi
> >
> > Mimi Zohar (14):
> > integrity: move ima inode integrity data management
> > security: move LSM xattrnames to xattr.h
> > xattr: define vfs_getxattr_alloc and vfs_xattr_cmp
> > evm: re-release
> > ima: move ima_file_free before releasing the file
> > security: imbed evm calls in security hooks
> > evm: inode post removexattr
> > evm: imbed evm_inode_post_setattr
> > evm: inode_post_init
> > fs: add evm_inode_post_init calls
> > ima: integrity appraisal extension
> > ima: appraise default rules
> > ima: inode post_setattr
> > ima: add ima_inode_setxattr and ima_inode_removexattr
> > --
>
> A summary diffstat would be good to see in patch 00/14.
>
> Lacking that, at least each individual patch should have a diffstat summary
> in it. Please read Documentation/SubmittingPatches.
>
> ---
> ~Randy

Only two minor changes from the RFC posting:

0011-ima-integrity-appraisal-extension.patch adds a missing
ima_fix_xattr() call.

Index: security-testing-2.6/security/integrity/ima/ima_appraise.c
===================================================================
--- security-testing-2.6.orig/security/integrity/ima/ima_appraise.c
+++ security-testing-2.6/security/integrity/ima/ima_appraise.c
@@ -92,8 +92,13 @@ int ima_appraise_measurement(struct inte
ima_fix_xattr(dentry, iint);
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
filename, op, cause, 1, 0);
- } else
+ } else {
status = INTEGRITY_PASS;
+
+ if ((status == INTEGRITY_UNKNOWN)
+ && (ima_appraise & IMA_APPRAISE_FIX))
+ ima_fix_xattr(dentry, iint);
+ }
iint->flags |= IMA_APPRAISED;
} else {
if (status == INTEGRITY_NOLABEL)

0012-ima-appraise-default-rules.patch replaces audit_log_format() calls
with ima_log_string(), introduced in Eric's patches.

Mimi

2010-04-21 22:25:32

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On 04/21/10 15:18, Mimi Zohar wrote:
> On Wed, 2010-04-21 at 14:58 -0700, Randy Dunlap wrote:
>> On Wed, 21 Apr 2010 17:49:40 -0400 Mimi Zohar wrote:
>>
>>> Extended Verification Module(EVM) detects offline tampering of the
>>> security extended attributes (e.g. security.selinux, security.SMACK64,
>>> security.ima), which is the basis for LSM permission decisions and,
>>> with this set of patches, integrity appraisal decisions. To detect
>>> offline tampering of the extended attributes, EVM maintains an
>>> HMAC-sha1 across a set of security extended attributes, storing the
>>> HMAC as the extended attribute 'security.evm'. To verify the integrity
>>> of an extended attribute, EVM exports evm_verifyxattr(), which
>>> re-calculates the HMAC and compares it with the version stored in
>>> 'security.evm'.
>>>
>> ...
>>>
>>> Much appreciation to Dave Hansen, Serge Hallyn, and Matt Helsley for
>>> reviewing the patches.
>>>
>>> Mimi
>>>
>>> Mimi Zohar (14):
>>> integrity: move ima inode integrity data management
>>> security: move LSM xattrnames to xattr.h
>>> xattr: define vfs_getxattr_alloc and vfs_xattr_cmp
>>> evm: re-release
>>> ima: move ima_file_free before releasing the file
>>> security: imbed evm calls in security hooks
>>> evm: inode post removexattr
>>> evm: imbed evm_inode_post_setattr
>>> evm: inode_post_init
>>> fs: add evm_inode_post_init calls
>>> ima: integrity appraisal extension
>>> ima: appraise default rules
>>> ima: inode post_setattr
>>> ima: add ima_inode_setxattr and ima_inode_removexattr
>>> --
>>
>> A summary diffstat would be good to see in patch 00/14.
>>
>> Lacking that, at least each individual patch should have a diffstat summary
>> in it. Please read Documentation/SubmittingPatches.
>>
>> ---
>> ~Randy
>
> Only two minor changes from the RFC posting:
>
> 0011-ima-integrity-appraisal-extension.patch adds a missing
> ima_fix_xattr() call.


diffstat summary example, from a series of 35 filesystem patches:

Documentation/filesystems/union-mounts.txt | 708 ++++++++++++++++++++++
Documentation/filesystems/vfs.txt | 16 +-
fs/Kconfig | 13 +
fs/Makefile | 1 +
fs/autofs4/autofs_i.h | 1 +
fs/autofs4/init.c | 11 +-
fs/autofs4/root.c | 6 +
fs/compat.c | 9 +
fs/dcache.c | 35 +-
fs/ext2/dir.c | 248 +++++++-
fs/ext2/ext2.h | 4 +
fs/ext2/inode.c | 11 +-
fs/ext2/namei.c | 89 +++-
fs/ext2/super.c | 6 +
fs/jffs2/dir.c | 104 ++++-
fs/jffs2/fs.c | 4 +
fs/jffs2/super.c | 2 +-
fs/libfs.c | 21 +-
fs/namei.c | 793 ++++++++++++++++++++++---
fs/namespace.c | 146 +++++-
fs/nfsd/nfs3xdr.c | 5 +
fs/nfsd/nfs4xdr.c | 5 +
fs/nfsd/nfsxdr.c | 4 +
fs/open.c | 116 +++-
fs/readdir.c | 18 +
fs/super.c | 23 +
fs/union.c | 881 ++++++++++++++++++++++++++++
fs/utimes.c | 13 +-
include/linux/dcache.h | 40 ++
include/linux/ext2_fs.h | 5 +
include/linux/fs.h | 16 +
include/linux/jffs2.h | 8 +
include/linux/mount.h | 7 +-
include/linux/namei.h | 2 +
include/linux/union.h | 77 +++
mm/shmem.c | 195 ++++++-
36 files changed, 3483 insertions(+), 160 deletions(-)
create mode 100644 Documentation/filesystems/union-mounts.txt
create mode 100644 fs/union.c
create mode 100644 include/linux/union.h


This summarizes which files are changed and how much, so that interested
people can know if they want to review the patches.

--
~Randy

2010-04-21 22:41:46

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Wed, 2010-04-21 at 15:23 -0700, Randy Dunlap wrote:
> On 04/21/10 15:18, Mimi Zohar wrote:
> > On Wed, 2010-04-21 at 14:58 -0700, Randy Dunlap wrote:
> >> On Wed, 21 Apr 2010 17:49:40 -0400 Mimi Zohar wrote:
> >>
> >>> Extended Verification Module(EVM) detects offline tampering of the
> >>> security extended attributes (e.g. security.selinux, security.SMACK64,
> >>> security.ima), which is the basis for LSM permission decisions and,
> >>> with this set of patches, integrity appraisal decisions. To detect
> >>> offline tampering of the extended attributes, EVM maintains an
> >>> HMAC-sha1 across a set of security extended attributes, storing the
> >>> HMAC as the extended attribute 'security.evm'. To verify the integrity
> >>> of an extended attribute, EVM exports evm_verifyxattr(), which
> >>> re-calculates the HMAC and compares it with the version stored in
> >>> 'security.evm'.
> >>>
> >> ...
> >>>
> >>> Much appreciation to Dave Hansen, Serge Hallyn, and Matt Helsley for
> >>> reviewing the patches.
> >>>
> >>> Mimi
> >>>
> >>> Mimi Zohar (14):
> >>> integrity: move ima inode integrity data management
> >>> security: move LSM xattrnames to xattr.h
> >>> xattr: define vfs_getxattr_alloc and vfs_xattr_cmp
> >>> evm: re-release
> >>> ima: move ima_file_free before releasing the file
> >>> security: imbed evm calls in security hooks
> >>> evm: inode post removexattr
> >>> evm: imbed evm_inode_post_setattr
> >>> evm: inode_post_init
> >>> fs: add evm_inode_post_init calls
> >>> ima: integrity appraisal extension
> >>> ima: appraise default rules
> >>> ima: inode post_setattr
> >>> ima: add ima_inode_setxattr and ima_inode_removexattr
> >>> --
> >>
> >> A summary diffstat would be good to see in patch 00/14.
> >>
> >> Lacking that, at least each individual patch should have a diffstat summary
> >> in it. Please read Documentation/SubmittingPatches.
> >>
> >> ---
> >> ~Randy
> >

Documentation/kernel-parameters.txt | 4 +
fs/attr.c | 7 +-
fs/ext2/xattr_security.c | 31 +++-
fs/ext3/xattr_security.c | 30 +++-
fs/ext4/xattr_security.c | 30 +++-
fs/file_table.c | 2 +-
fs/xattr.c | 63 ++++++-
include/linux/capability.h | 3 -
include/linux/evm.h | 80 ++++++++
include/linux/ima.h | 27 ++-
include/linux/integrity.h | 35 ++++
include/linux/xattr.h | 27 +++-
security/Kconfig | 2 +-
security/Makefile | 4 +-
security/integrity/Kconfig | 7 +
security/integrity/Makefile | 12 ++
security/integrity/evm/Kconfig | 13 ++
security/integrity/evm/Makefile | 7 +
security/integrity/evm/evm.h | 38 ++++
security/integrity/evm/evm_crypto.c | 198 +++++++++++++++++++
security/integrity/evm/evm_main.c | 335 +++++++++++++++++++++++++++++++++
security/integrity/evm/evm_secfs.c | 108 +++++++++++
security/integrity/iint.c | 153 +++++++++++++++
security/integrity/ima/Kconfig | 15 ++
security/integrity/ima/Makefile | 4 +-
security/integrity/ima/ima.h | 76 +++++---
security/integrity/ima/ima_api.c | 61 +++++--
security/integrity/ima/ima_appraise.c | 152 +++++++++++++++
security/integrity/ima/ima_iint.c | 145 --------------
security/integrity/ima/ima_main.c | 123 ++++++++++---
security/integrity/ima/ima_policy.c | 61 ++++++-
security/integrity/integrity.h | 50 +++++
security/security.c | 27 +++-
security/selinux/hooks.c | 3 -
security/smack/smack.h | 2 -
35 files changed, 1671 insertions(+), 264 deletions(-)
create mode 100644 include/linux/evm.h
create mode 100644 include/linux/integrity.h
create mode 100644 security/integrity/Kconfig
create mode 100644 security/integrity/Makefile
create mode 100644 security/integrity/evm/Kconfig
create mode 100644 security/integrity/evm/Makefile
create mode 100644 security/integrity/evm/evm.h
create mode 100644 security/integrity/evm/evm_crypto.c
create mode 100644 security/integrity/evm/evm_main.c
create mode 100644 security/integrity/evm/evm_secfs.c
create mode 100644 security/integrity/iint.c
create mode 100644 security/integrity/ima/ima_appraise.c
delete mode 100644 security/integrity/ima/ima_iint.c
create mode 100644 security/integrity/integrity.h

Mimi

2010-04-26 18:50:26

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH 03/14] xattr: define vfs_getxattr_alloc and vfs_xattr_cmp

Quoting Mimi Zohar ([email protected]):
> vfs_getxattr_alloc() and vfs_xattr_cmp() are two new kernel xattr
> helper functions. vfs_getxattr_alloc() first allocates memory for
> the requested xattr and then retrieves it. vfs_xattr_cmp() compares
> a given value with the contents of an extended attribute.
>
> Signed-off-by: Mimi Zohar <[email protected]>

(Heh, *thought* I had a hole to point to, but nope, looks good)

Acked-by: Serge Hallyn <[email protected]>

thanks,
-serge

>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 46f87e8..341ad71 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -159,6 +159,64 @@ out_noalloc:
> }
> EXPORT_SYMBOL_GPL(xattr_getsecurity);
>
> +/*
> + * vfs_getxattr_alloc - allocate memory, if necessary, before calling getxattr
> + *
> + * Allocate memory, if not already allocated, or re-allocate correct size,
> + * before retrieving the extended attribute.
> + *
> + * Returns the result of alloc, if failed, or the getxattr operation.
> + */
> +ssize_t
> +vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value,
> + size_t xattr_size, gfp_t flags)
> +{
> + struct inode *inode = dentry->d_inode;
> + char *value = *xattr_value;
> + int error;
> +
> + error = xattr_permission(inode, name, MAY_READ);
> + if (error)
> + return error;
> +
> + if (!inode->i_op->getxattr)
> + return -EOPNOTSUPP;
> +
> + error = inode->i_op->getxattr(dentry, name, NULL, 0);
> + if (error < 0)
> + return error;
> +
> + if (!value || (error > xattr_size)) {
> + value = krealloc(*xattr_value, error + 1, flags);
> + if (!value)
> + return -ENOMEM;
> + memset(value, 0, error + 1);
> + }
> +
> + error = inode->i_op->getxattr(dentry, name, value, error);
> + *xattr_value = value;
> + return error;
> +}
> +
> +/* Compare an extended attribute value with the given value */
> +int vfs_xattr_cmp(struct dentry *dentry, const char *xattr_name,
> + const char *value, size_t size, gfp_t flags)
> +{
> + char *xattr_value = NULL;
> + int rc;
> +
> + rc = vfs_getxattr_alloc(dentry, xattr_name, &xattr_value, 0, flags);
> + if (rc < 0)
> + return rc;
> +
> + if ((rc != size) || (memcmp(xattr_value, value, rc) != 0))
> + rc = -EINVAL;
> + else
> + rc = 0;
> + kfree(xattr_value);
> + return rc;
> +}
> +
> ssize_t
> vfs_getxattr(struct dentry *dentry, const char *name, void *value, size_t size)
> {
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index d079bec..8698de3 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -68,7 +68,10 @@ ssize_t generic_getxattr(struct dentry *dentry, const char *name, void *buffer,
> ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size);
> int generic_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags);
> int generic_removexattr(struct dentry *dentry, const char *name);
> -
> +ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name,
> + char **xattr_value, size_t size, gfp_t flags);
> +int vfs_xattr_cmp(struct dentry *dentry, const char *xattr_name,
> + const char *value, size_t size, gfp_t flags);
> #endif /* __KERNEL__ */
>
> #endif /* _LINUX_XATTR_H */
> --
> 1.6.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-04-26 21:03:35

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH 04/14] evm: re-release

Quoting Mimi Zohar ([email protected]):
> EVM protects a file's security extended attributes against integrity
> attacks. It maintains an HMAC-sha1 value across the extended attributes,
> storing the value as the extended attribute 'security.evm'. EVM has gone
> through a number of iterations, initially as an LSM module, subsequently
> as a LIM integrity provider, and now, when co-located with a security_
> hook, embedded directly in the security_ hook, similar to IMA.
>
> This is the first part of a local file integrity verification system.
> While this part does authenticate the selected extended attributes, and
> cryptographically bind them to the inode, coming extensions will bind
> other directory and inode metadata for more complete protection. The
> set of protected security extended attributes is configured at compile.
>
> EVM depends on the Kernel Key Retention System to provide it with the
> kernel master key for the HMAC operation. The kernel master key is
> securely loaded onto the root's keyring, typically by 'loadkernkey',
> which either uses the TPM sealed secret key, if available, or a
> password requested from the console. To signal EVM, that the key has
> been loaded onto the keyring, 'echo 1 > <securityfs>/evm'. This is
> normally done in the initrd, which has already been measured as part
> of the trusted boot. (Refer to http://linux-ima.sourceforge.net/#EVM.)
>
> EVM adds the following three calls to the existing security hooks,
> evm_inode_setxattr(), evm_inode_post_setxattr(), and
> evm_inode_removexattr.
>
> To initialize and update the 'security.evm' extended attribute, EVM
> defines three calls: evm_inode_post_init(), evm_inode_post_setattr()
> and evm_inode_post_removexattr() hooks.
>
> To verify the integrity of an extended attribute, EVM exports
> evm_verifyxattr().
>
> Signed-off-by: Mimi Zohar <[email protected]>

Acked-by: Serge Hallyn <[email protected]>

thanks,
-serge

2010-06-01 19:29:12

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Mon, 2010-05-31 at 15:08 +0500, Shaz wrote:
> > EVM is based on EA while Aegis does not use EA as far as I can
> > understand from the documentation available. Can we make EVM
> > independent of EA? Even the MAC mechanism is very different then
> > existing LSM based mechanisms.
>
> Have a look at the following:
>
> http://research.nokia.com/files/NRCTR2008010.pdf
> http://research.nokia.com/files/NRCTR2008007.pdf
> http://lwn.net/Articles/372937/

SELinux, Smack, Capabilities, and IMA all use extended attributes. The
purpose of EVM is to detect offline tampering of these security extended
attributes.

The IMA integrity appraisal extension extends IMA with local measurement
appraisal. The extension stores and maintains the file integrity
measurement as an extended attribute 'security.ima', which EVM can be
configured to protect. Instead of storing the hash measurement as an
extended attribute, the file hashes could be loaded in kernel memory, as
long as the appraise policy is appropriately constrained.

Mimi

2010-06-02 07:03:28

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM



On 01/06/10 22:28, ext Mimi Zohar wrote:
> On Mon, 2010-05-31 at 15:08 +0500, Shaz wrote:
>
>>> EVM is based on EA while Aegis does not use EA as far as I can
>>> understand from the documentation available. Can we make EVM
>>> independent of EA? Even the MAC mechanism is very different then
>>> existing LSM based mechanisms.
>>>
>> Have a look at the following:
>>
>> http://research.nokia.com/files/NRCTR2008010.pdf
>> http://research.nokia.com/files/NRCTR2008007.pdf
>> http://lwn.net/Articles/372937/
>>
> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
> purpose of EVM is to detect offline tampering of these security extended
> attributes.
>
> The IMA integrity appraisal extension extends IMA with local measurement
> appraisal. The extension stores and maintains the file integrity
> measurement as an extended attribute 'security.ima', which EVM can be
> configured to protect. Instead of storing the hash measurement as an
> extended attribute, the file hashes could be loaded in kernel memory, as
> long as the appraise policy is appropriately constrained.
>
>
Hi,

Maemo integrity protection solution was based on old DigSig project
which was used to verify
integrity of executables. Signed integrity measurement was embedded to
the ELF header.
When we started to develop it EVM was not available.

And we decided to use a file to keep hashes and other info.

Our goals were
1. Protect also certain data files.
digsig worked only with ELF files.

2. Be mobile friendly
It seems faster to verify signature of one file with hashes instead of
checking signature of every EA.

3. Persistant to offline attacks
EA can be delete. If not all files has EA then it is not possible to
detect removal

4. Do not use EA.
IIRC it was some problems with EA on our system and we could not use them..


EVM looks very interesting and I would like also to review the code and
understand the architecture.
We consider possibility to use EVM if it is going to be in the kernel.

- Dmitry Kasatkin
Maemo/MeeGo security team

> Mimi
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2010-06-02 07:50:55

by Shaz

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Wed, Jun 2, 2010 at 12:03 PM, Dmitry Kasatkin
<[email protected]> wrote:
>
>
> On 01/06/10 22:28, ext Mimi Zohar wrote:
>> On Mon, 2010-05-31 at 15:08 +0500, Shaz wrote:
>>
>>>> EVM is based on EA while Aegis does not use EA as far as I can
>>>> understand from the documentation available. Can we make EVM
>>>> independent of EA? Even the MAC mechanism is very different then
>>>> existing LSM based mechanisms.
>>>>
>>> Have a look at the following:
>>>
>>> http://research.nokia.com/files/NRCTR2008010.pdf
>>> http://research.nokia.com/files/NRCTR2008007.pdf
>>> http://lwn.net/Articles/372937/
>>>
>> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
>> purpose of EVM is to detect offline tampering of these security extended
>> attributes.

MeeGo/Maemo security framework does not use LSM because Maemo/MeeGo
security framework only focuses at process level MAC and for that they
use Dazuko as Nokia research report mentions.

By the way MeeGo 1.0 has no security at the moment so one cannot be
sure if they are going according to their research or what. They are
also not opening the internals of their security framework. Not sure
why if the whole thing is open and Linux Foundation is backing it up.

>> The IMA integrity appraisal extension extends IMA with local measurement
>> appraisal. The extension stores and maintains the file integrity
>> measurement as an extended attribute 'security.ima', which EVM can be
>> configured to protect. ?Instead of storing the hash measurement as an
>> extended attribute, the file hashes could be loaded in kernel memory, as
>> long as the appraise policy is appropriately constrained.
>>
>>
> Hi,
>
> Maemo integrity protection solution was based on old DigSig project
> which was used to verify
> integrity of executables. Signed integrity measurement was embedded to
> the ELF header.
> When we started to develop it EVM was not available.
>
> And we decided to use a file to keep hashes and other info.
>
> Our goals were
> 1. Protect also certain data files.
> digsig worked only with ELF files.
>
> 2. Be mobile friendly
> It seems faster to verify signature of one file with hashes instead of
> checking signature of every EA.

Here Mimi can explain better because I am of the same opinion as yours
if you mean that all signatures lie in one file and you check it from
there. Anyways some sort of policy can reduce checking every EA ... I
guess.

> 3. Persistant to offline attacks
> EA can be delete. If not all files has EA then it is not possible to
> detect removal

Availability of EA on all file systems needs some effort but it's not
a big deal. I have even seen patches for yaffs2.

> 4. Do not use EA.
> IIRC it was some problems with EA on our system and we could not use them..

This is a bad excuse :)

> EVM looks very interesting and I would like also to review the code and
> understand the architecture.
> We consider possibility to use EVM if it is going to be in the kernel.

Please do have a look because we need these features too but in a
light weight manner. We are trying to make available similar
functionality for OpenMoko based software stacks.

--
Shaz

2010-06-02 09:13:16

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM


On 02/06/10 10:50, ext Shaz wrote:
> On Wed, Jun 2, 2010 at 12:03 PM, Dmitry Kasatkin
> <[email protected]> wrote:
>
>>
>> On 01/06/10 22:28, ext Mimi Zohar wrote:
>>
>>> On Mon, 2010-05-31 at 15:08 +0500, Shaz wrote:
>>>
>>>
>>>>> EVM is based on EA while Aegis does not use EA as far as I can
>>>>> understand from the documentation available. Can we make EVM
>>>>> independent of EA? Even the MAC mechanism is very different then
>>>>> existing LSM based mechanisms.
>>>>>
>>>>>
>>>> Have a look at the following:
>>>>
>>>> http://research.nokia.com/files/NRCTR2008010.pdf
>>>> http://research.nokia.com/files/NRCTR2008007.pdf
>>>> http://lwn.net/Articles/372937/
>>>>
>>>>
>>> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
>>> purpose of EVM is to detect offline tampering of these security extended
>>> attributes.
>>>
> MeeGo/Maemo security framework does not use LSM because Maemo/MeeGo
> security framework only focuses at process level MAC and for that they
> use Dazuko as Nokia research report mentions.
>
> By the way MeeGo 1.0 has no security at the moment so one cannot be
> sure if they are going according to their research or what. They are
> also not opening the internals of their security framework. Not sure
> why if the whole thing is open and Linux Foundation is backing it up.
>
>
Maemo/MeeGo does not do exactly what is written in that research report.
It has been done in parallel to our work.
And we do use LSM.

We go approval from company layers to open source the work.
We will see what to do next.

>>> The IMA integrity appraisal extension extends IMA with local measurement
>>> appraisal. The extension stores and maintains the file integrity
>>> measurement as an extended attribute 'security.ima', which EVM can be
>>> configured to protect. Instead of storing the hash measurement as an
>>> extended attribute, the file hashes could be loaded in kernel memory, as
>>> long as the appraise policy is appropriately constrained.
>>>
>>>
>>>
>> Hi,
>>
>> Maemo integrity protection solution was based on old DigSig project
>> which was used to verify
>> integrity of executables. Signed integrity measurement was embedded to
>> the ELF header.
>> When we started to develop it EVM was not available.
>>
>> And we decided to use a file to keep hashes and other info.
>>
>> Our goals were
>> 1. Protect also certain data files.
>> digsig worked only with ELF files.
>>
>> 2. Be mobile friendly
>> It seems faster to verify signature of one file with hashes instead of
>> checking signature of every EA.
>>
> Here Mimi can explain better because I am of the same opinion as yours
> if you mean that all signatures lie in one file and you check it from
> there. Anyways some sort of policy can reduce checking every EA ... I
> guess.
>
>
>> 3. Persistant to offline attacks
>> EA can be delete. If not all files has EA then it is not possible to
>> detect removal
>>
> Availability of EA on all file systems needs some effort but it's not
> a big deal. I have even seen patches for yaffs2.
>
>
The issue was removal.
>> 4. Do not use EA.
>> IIRC it was some problems with EA on our system and we could not use them..
>>
> This is a bad excuse :)
>
>
Yes. not so convincing...
>> EVM looks very interesting and I would like also to review the code and
>> understand the architecture.
>> We consider possibility to use EVM if it is going to be in the kernel.
>>
> Please do have a look because we need these features too but in a
> light weight manner. We are trying to make available similar
> functionality for OpenMoko based software stacks.
>
>

2010-06-02 10:15:51

by Shaz

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Wed, Jun 2, 2010 at 2:12 PM, Dmitry Kasatkin
<[email protected]> wrote:
>
> On 02/06/10 10:50, ext Shaz wrote:
>> On Wed, Jun 2, 2010 at 12:03 PM, Dmitry Kasatkin
>> <[email protected]> wrote:
>>
>>>
>>> On 01/06/10 22:28, ext Mimi Zohar wrote:
>>>
>>>> On Mon, 2010-05-31 at 15:08 +0500, Shaz wrote:
>>>>
>>>>
>>>>>> EVM is based on EA while Aegis does not use EA as far as I can
>>>>>> understand from the documentation available. Can we make EVM
>>>>>> independent of EA? Even the MAC mechanism is very different then
>>>>>> existing LSM based mechanisms.
>>>>>>
>>>>>>
>>>>> Have a look at the following:
>>>>>
>>>>> http://research.nokia.com/files/NRCTR2008010.pdf
>>>>> http://research.nokia.com/files/NRCTR2008007.pdf
>>>>> http://lwn.net/Articles/372937/
>>>>>
>>>>>
>>>> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
>>>> purpose of EVM is to detect offline tampering of these security extended
>>>> attributes.
>>>>
>> MeeGo/Maemo security framework does not use LSM because Maemo/MeeGo
>> security framework only focuses at process level MAC and for that they
>> use Dazuko as Nokia research report mentions.
>>
>> By the way MeeGo 1.0 has no security at the moment so one cannot be
>> sure if they are going according to their research or what. They are
>> also not opening the internals of their security framework. Not sure
>> why if the whole thing is open and Linux Foundation is backing it up.
>>
>>
> Maemo/MeeGo does not do exactly what is written in that research report.
> It has been done in parallel to our work.
> And we do use LSM.

Elina was the first author of the report and she is the person who has
presented whatever is available from Maemo project regarding security.
Therefore it is very odd that your work is in parallel to the report!

> We go approval from company layers to open source the work.
> We will see what to do next.

I cannot understand what the Linux Foundation will have to say about this?

>>>> The IMA integrity appraisal extension extends IMA with local measurement
>>>> appraisal. The extension stores and maintains the file integrity
>>>> measurement as an extended attribute 'security.ima', which EVM can be
>>>> configured to protect. ?Instead of storing the hash measurement as an
>>>> extended attribute, the file hashes could be loaded in kernel memory, as
>>>> long as the appraise policy is appropriately constrained.
>>>>
>>>>
>>>>
>>> Hi,
>>>
>>> Maemo integrity protection solution was based on old DigSig project
>>> which was used to verify
>>> integrity of executables. Signed integrity measurement was embedded to
>>> the ELF header.
>>> When we started to develop it EVM was not available.
>>>
>>> And we decided to use a file to keep hashes and other info.
>>>
>>> Our goals were
>>> 1. Protect also certain data files.
>>> digsig worked only with ELF files.
>>>
>>> 2. Be mobile friendly
>>> It seems faster to verify signature of one file with hashes instead of
>>> checking signature of every EA.
>>>
>> Here Mimi can explain better because I am of the same opinion as yours
>> if you mean that all signatures lie in one file and you check it from
>> there. Anyways some sort of policy can reduce checking every EA ... I
>> guess.
>>
>>
>>> 3. Persistant to offline attacks
>>> EA can be delete. If not all files has EA then it is not possible to
>>> detect removal
>>>
>> Availability of EA on all file systems needs some effort but it's not
>> a big deal. I have even seen patches for yaffs2.
>>
>>
> The issue was removal.

EA is a filesystem functionality and if the attributes get deleted or
tampered than EVM will report is as un-trusted. I haven't done
practical work on EVM yet but this is how it should be. Mimi can
clarify this.

>>> 4. Do not use EA.
>>> IIRC it was some problems with EA on our system and we could not use them..
>>>
>> This is a bad excuse :)
>>
>>
> Yes. not so convincing...
>>> EVM looks very interesting and I would like also to review the code and
>>> understand the architecture.

It is very odd that you guys are unaware of what is opensource since a decade!

>>> We consider possibility to use EVM if it is going to be in the kernel.
>>>
>> Please do have a look because we need these features too but in a
>> light weight manner. We are trying to make available similar
>> functionality for OpenMoko based software stacks.
>>
>>
>



--
Shaz

2010-06-02 10:23:28

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM



On 02/06/10 13:15, ext Shaz wrote:
> On Wed, Jun 2, 2010 at 2:12 PM, Dmitry Kasatkin
> <[email protected]> wrote:
>
>> On 02/06/10 10:50, ext Shaz wrote:
>>
>>> On Wed, Jun 2, 2010 at 12:03 PM, Dmitry Kasatkin
>>> <[email protected]> wrote:
>>>
>>>
>>>> On 01/06/10 22:28, ext Mimi Zohar wrote:
>>>>
>>>>
>>>>> On Mon, 2010-05-31 at 15:08 +0500, Shaz wrote:
>>>>>
>>>>>
>>>>>
>>>>>>> EVM is based on EA while Aegis does not use EA as far as I can
>>>>>>> understand from the documentation available. Can we make EVM
>>>>>>> independent of EA? Even the MAC mechanism is very different then
>>>>>>> existing LSM based mechanisms.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Have a look at the following:
>>>>>>
>>>>>> http://research.nokia.com/files/NRCTR2008010.pdf
>>>>>> http://research.nokia.com/files/NRCTR2008007.pdf
>>>>>> http://lwn.net/Articles/372937/
>>>>>>
>>>>>>
>>>>>>
>>>>> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
>>>>> purpose of EVM is to detect offline tampering of these security extended
>>>>> attributes.
>>>>>
>>>>>
>>> MeeGo/Maemo security framework does not use LSM because Maemo/MeeGo
>>> security framework only focuses at process level MAC and for that they
>>> use Dazuko as Nokia research report mentions.
>>>
>>> By the way MeeGo 1.0 has no security at the moment so one cannot be
>>> sure if they are going according to their research or what. They are
>>> also not opening the internals of their security framework. Not sure
>>> why if the whole thing is open and Linux Foundation is backing it up.
>>>
>>>
>>>
>> Maemo/MeeGo does not do exactly what is written in that research report.
>> It has been done in parallel to our work.
>> And we do use LSM.
>>
> Elina was the first author of the report and she is the person who has
> presented whatever is available from Maemo project regarding security.
> Therefore it is very odd that your work is in parallel to the report!
>
>
Elena was not working for Maemo at that time.
They had own objectives in their research.
Then after moving to Maemo she was assigned to present maemo stuff.
But I guess we never said that we follow that research work.
Yeah.. might look confusing but that is how it went.

- Dmitry

>> We go approval from company layers to open source the work.
>> We will see what to do next.
>>
> I cannot understand what the Linux Foundation will have to say about this?
>
>
>>>>> The IMA integrity appraisal extension extends IMA with local measurement
>>>>> appraisal. The extension stores and maintains the file integrity
>>>>> measurement as an extended attribute 'security.ima', which EVM can be
>>>>> configured to protect. Instead of storing the hash measurement as an
>>>>> extended attribute, the file hashes could be loaded in kernel memory, as
>>>>> long as the appraise policy is appropriately constrained.
>>>>>
>>>>>
>>>>>
>>>>>
>>>> Hi,
>>>>
>>>> Maemo integrity protection solution was based on old DigSig project
>>>> which was used to verify
>>>> integrity of executables. Signed integrity measurement was embedded to
>>>> the ELF header.
>>>> When we started to develop it EVM was not available.
>>>>
>>>> And we decided to use a file to keep hashes and other info.
>>>>
>>>> Our goals were
>>>> 1. Protect also certain data files.
>>>> digsig worked only with ELF files.
>>>>
>>>> 2. Be mobile friendly
>>>> It seems faster to verify signature of one file with hashes instead of
>>>> checking signature of every EA.
>>>>
>>>>
>>> Here Mimi can explain better because I am of the same opinion as yours
>>> if you mean that all signatures lie in one file and you check it from
>>> there. Anyways some sort of policy can reduce checking every EA ... I
>>> guess.
>>>
>>>
>>>
>>>> 3. Persistant to offline attacks
>>>> EA can be delete. If not all files has EA then it is not possible to
>>>> detect removal
>>>>
>>>>
>>> Availability of EA on all file systems needs some effort but it's not
>>> a big deal. I have even seen patches for yaffs2.
>>>
>>>
>>>
>> The issue was removal.
>>
> EA is a filesystem functionality and if the attributes get deleted or
> tampered than EVM will report is as un-trusted. I haven't done
> practical work on EVM yet but this is how it should be. Mimi can
> clarify this.
>
>
>>>> 4. Do not use EA.
>>>> IIRC it was some problems with EA on our system and we could not use them..
>>>>
>>>>
>>> This is a bad excuse :)
>>>
>>>
>>>
>> Yes. not so convincing...
>>
>>>> EVM looks very interesting and I would like also to review the code and
>>>> understand the architecture.
>>>>
> It is very odd that you guys are unaware of what is opensource since a decade!
>
>
>>>> We consider possibility to use EVM if it is going to be in the kernel.
>>>>
>>>>
>>> Please do have a look because we need these features too but in a
>>> light weight manner. We are trying to make available similar
>>> functionality for OpenMoko based software stacks.
>>>
>>>
>>>
>>
>
>
>

2010-06-02 14:18:52

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Wed, 2010-06-02 at 12:12 +0300, Dmitry Kasatkin wrote:
> On 02/06/10 10:50, ext Shaz wrote:
> > On Wed, Jun 2, 2010 at 12:03 PM, Dmitry Kasatkin
> > <[email protected]> wrote:
> >
> >>
> >> On 01/06/10 22:28, ext Mimi Zohar wrote:
> >>
> >>> On Mon, 2010-05-31 at 15:08 +0500, Shaz wrote:
> >>>
> >>>
> >>>>> EVM is based on EA while Aegis does not use EA as far as I can
> >>>>> understand from the documentation available. Can we make EVM
> >>>>> independent of EA? Even the MAC mechanism is very different then
> >>>>> existing LSM based mechanisms.
> >>>>>
> >>>>>
> >>>> Have a look at the following:
> >>>>
> >>>> http://research.nokia.com/files/NRCTR2008010.pdf
> >>>> http://research.nokia.com/files/NRCTR2008007.pdf
> >>>> http://lwn.net/Articles/372937/
> >>>>
> >>>>
> >>> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
> >>> purpose of EVM is to detect offline tampering of these security extended
> >>> attributes.
> >>>
> > MeeGo/Maemo security framework does not use LSM because Maemo/MeeGo
> > security framework only focuses at process level MAC and for that they
> > use Dazuko as Nokia research report mentions.
> >
> > By the way MeeGo 1.0 has no security at the moment so one cannot be
> > sure if they are going according to their research or what. They are
> > also not opening the internals of their security framework. Not sure
> > why if the whole thing is open and Linux Foundation is backing it up.
> >
> >
> Maemo/MeeGo does not do exactly what is written in that research report.
> It has been done in parallel to our work.
> And we do use LSM.
>
> We go approval from company layers to open source the work.
> We will see what to do next.
>
> >>> The IMA integrity appraisal extension extends IMA with local measurement
> >>> appraisal. The extension stores and maintains the file integrity
> >>> measurement as an extended attribute 'security.ima', which EVM can be
> >>> configured to protect. Instead of storing the hash measurement as an
> >>> extended attribute, the file hashes could be loaded in kernel memory, as
> >>> long as the appraise policy is appropriately constrained.
> >>>
> >>>
> >>>
> >> Hi,
> >>
> >> Maemo integrity protection solution was based on old DigSig project
> >> which was used to verify
> >> integrity of executables. Signed integrity measurement was embedded to
> >> the ELF header.
> >> When we started to develop it EVM was not available.
> >>
> >> And we decided to use a file to keep hashes and other info.
> >>
> >> Our goals were
> >> 1. Protect also certain data files.
> >> digsig worked only with ELF files.
> >>
> >> 2. Be mobile friendly
> >> It seems faster to verify signature of one file with hashes instead of
> >> checking signature of every EA.
> >>
> > Here Mimi can explain better because I am of the same opinion as yours
> > if you mean that all signatures lie in one file and you check it from
> > there. Anyways some sort of policy can reduce checking every EA ... I
> > guess.

Yes, verifying one file containing the hashes would be faster than
verifying individual hashes stored as extended attributes (xattrs), but
this does not take into account that files on a running system are being
modified or added. On a small form factor, the number of files is
limited, but would this scale well? In addition, what protects that one
file containing all the hashes from being modified? So, if you limit
the types of files to those that don't change, and the number of file
hashes, then using a single file would be faster.

> >> 3. Persistant to offline attacks
> >> EA can be delete. If not all files has EA then it is not possible to
> >> detect removal
> >>
> > Availability of EA on all file systems needs some effort but it's not
> > a big deal. I have even seen patches for yaffs2.
> >
> >
> The issue was removal.

Yes, exactly. The original EVM required that all files be labeled.
Anything unlabeled was untrusted. The current implementation is based
upon policy with the default policy being to only appraise all files
owned by root.

> >> 4. Do not use EA.
> >> IIRC it was some problems with EA on our system and we could not use them..
> >>
> > This is a bad excuse :)
> >
> >
> Yes. not so convincing...
> >> EVM looks very interesting and I would like also to review the code and
> >> understand the architecture.
> >> We consider possibility to use EVM if it is going to be in the kernel.

Am looking forward to your questions/comments.

> > Please do have a look because we need these features too but in a
> > light weight manner. We are trying to make available similar
> > functionality for OpenMoko based software stacks.

Glad to hear it.

Mimi

2010-06-04 00:58:00

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Tue, 1 Jun 2010, Mimi Zohar wrote:

> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
> purpose of EVM is to detect offline tampering of these security extended
> attributes.

One issue mentioned to me off-list is that if EVM is only protecting
against offline attacks, why not just encrypt the entire volume ?

This would provide confidentiality and integrity protection for all data
and metadata, rather than just integrity for xattr metadata.


- James
--
James Morris
<[email protected]>

2010-06-04 06:53:24

by Shaz

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

> Yes, verifying one file containing the hashes would be faster than
> verifying individual hashes stored as extended attributes (xattrs), but
> this does not take into account that files on a running system are being
> modified or added. On a small form factor, the number of files is
> limited, but would this scale well? In addition, what protects that one
> file containing all the hashes from being modified? ?So, if you limit

How about sealing to protect this file?

> the types of files to those that don't change, and the number of file
> hashes, then using a single file would be faster.



--
Shaz

2010-06-04 06:57:01

by Shaz

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Fri, Jun 4, 2010 at 5:57 AM, James Morris <[email protected]> wrote:
> On Tue, 1 Jun 2010, Mimi Zohar wrote:
>
>> SELinux, Smack, Capabilities, and IMA all use extended attributes. The
>> purpose of EVM is to detect offline tampering of these security extended
>> attributes.
>
> One issue mentioned to me off-list is that if EVM is only protecting
> against offline attacks, why not just encrypt the entire volume ?

Are you sure that EVM protects against offline attacks only?

Why and why not encrypt the whole volume?

> This would provide confidentiality and integrity protection for all data
> and metadata, rather than just integrity for xattr metadata.


--
Shaz

2010-06-04 14:47:16

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH 04/14] evm: re-release

On Wed, 2010-04-21 at 17:49 -0400, Mimi Zohar wrote:
> EVM protects a file's security extended attributes against integrity
> attacks. It maintains an HMAC-sha1 value across the extended attributes,
> storing the value as the extended attribute 'security.evm'. EVM has gone
> through a number of iterations, initially as an LSM module, subsequently
> as a LIM integrity provider, and now, when co-located with a security_
> hook, embedded directly in the security_ hook, similar to IMA.
>
> This is the first part of a local file integrity verification system.
> While this part does authenticate the selected extended attributes, and
> cryptographically bind them to the inode, coming extensions will bind
> other directory and inode metadata for more complete protection. The
> set of protected security extended attributes is configured at compile.
>
> EVM depends on the Kernel Key Retention System to provide it with the
> kernel master key for the HMAC operation. The kernel master key is
> securely loaded onto the root's keyring, typically by 'loadkernkey',
> which either uses the TPM sealed secret key, if available, or a
> password requested from the console. To signal EVM, that the key has
> been loaded onto the keyring, 'echo 1 > <securityfs>/evm'. This is
> normally done in the initrd, which has already been measured as part
> of the trusted boot. (Refer to http://linux-ima.sourceforge.net/#EVM.)

I don't remember this dependency on the kernel key system in prior
incarnations of EVM. Can you explain the rationale for using it, and
the implications?

--
Stephen Smalley
National Security Agency

2010-06-04 14:53:37

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 04/14] evm: re-release

On Fri, 2010-06-04 at 10:28 -0400, Stephen Smalley wrote:
> On Wed, 2010-04-21 at 17:49 -0400, Mimi Zohar wrote:
> > EVM protects a file's security extended attributes against integrity
> > attacks. It maintains an HMAC-sha1 value across the extended attributes,
> > storing the value as the extended attribute 'security.evm'. EVM has gone
> > through a number of iterations, initially as an LSM module, subsequently
> > as a LIM integrity provider, and now, when co-located with a security_
> > hook, embedded directly in the security_ hook, similar to IMA.
> >
> > This is the first part of a local file integrity verification system.
> > While this part does authenticate the selected extended attributes, and
> > cryptographically bind them to the inode, coming extensions will bind
> > other directory and inode metadata for more complete protection. The
> > set of protected security extended attributes is configured at compile.
> >
> > EVM depends on the Kernel Key Retention System to provide it with the
> > kernel master key for the HMAC operation. The kernel master key is
> > securely loaded onto the root's keyring, typically by 'loadkernkey',
> > which either uses the TPM sealed secret key, if available, or a
> > password requested from the console. To signal EVM, that the key has
> > been loaded onto the keyring, 'echo 1 > <securityfs>/evm'. This is
> > normally done in the initrd, which has already been measured as part
> > of the trusted boot. (Refer to http://linux-ima.sourceforge.net/#EVM.)
>
> I don't remember this dependency on the kernel key system in prior
> incarnations of EVM. Can you explain the rationale for using it, and
> the implications?

This changed very early on, so that people without a TPM could 'play'
with it.

Mimi

2010-06-04 15:09:46

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Fri, 2010-06-04 at 11:53 +0500, Shaz wrote:
> > Yes, verifying one file containing the hashes would be faster than
> > verifying individual hashes stored as extended attributes (xattrs), but
> > this does not take into account that files on a running system are being
> > modified or added. On a small form factor, the number of files is
> > limited, but would this scale well? In addition, what protects that one
> > file containing all the hashes from being modified? So, if you limit
>
> How about sealing to protect this file?

Was just indicating that the file needs to be protected. So, yes sealing
the file, based on PCRs, would work in a trusted boot environment.

> > the types of files to those that don't change, and the number of file
> > hashes, then using a single file would be faster.

Mimi

2010-06-04 15:20:12

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH 04/14] evm: re-release

On Fri, 2010-06-04 at 10:53 -0400, Mimi Zohar wrote:
> On Fri, 2010-06-04 at 10:28 -0400, Stephen Smalley wrote:
> > On Wed, 2010-04-21 at 17:49 -0400, Mimi Zohar wrote:
> > > EVM protects a file's security extended attributes against integrity
> > > attacks. It maintains an HMAC-sha1 value across the extended attributes,
> > > storing the value as the extended attribute 'security.evm'. EVM has gone
> > > through a number of iterations, initially as an LSM module, subsequently
> > > as a LIM integrity provider, and now, when co-located with a security_
> > > hook, embedded directly in the security_ hook, similar to IMA.
> > >
> > > This is the first part of a local file integrity verification system.
> > > While this part does authenticate the selected extended attributes, and
> > > cryptographically bind them to the inode, coming extensions will bind
> > > other directory and inode metadata for more complete protection. The
> > > set of protected security extended attributes is configured at compile.
> > >
> > > EVM depends on the Kernel Key Retention System to provide it with the
> > > kernel master key for the HMAC operation. The kernel master key is
> > > securely loaded onto the root's keyring, typically by 'loadkernkey',
> > > which either uses the TPM sealed secret key, if available, or a
> > > password requested from the console. To signal EVM, that the key has
> > > been loaded onto the keyring, 'echo 1 > <securityfs>/evm'. This is
> > > normally done in the initrd, which has already been measured as part
> > > of the trusted boot. (Refer to http://linux-ima.sourceforge.net/#EVM.)
> >
> > I don't remember this dependency on the kernel key system in prior
> > incarnations of EVM. Can you explain the rationale for using it, and
> > the implications?
>
> This changed very early on, so that people without a TPM could 'play'
> with it.

The last time EVM was posted to lsm list (2005, AFAICS) prior to this
year's round, David Safford said:
"Since the kernel master key is unsealed by the hardware TPM only as a
result of a valid trusted boot, and the key is never visible outside the
kernel, the EVM HMAC attribute cannot be forged in an offline attack."

So that seemed to be an important property to you at the time. I
understand providing alternatives for TPM-less systems, but that doesn't
necessarily mean changing the properties on systems with TPMs.

--
Stephen Smalley
National Security Agency

2010-06-04 18:08:16

by David Safford

[permalink] [raw]
Subject: Re: [PATCH 04/14] evm: re-release

On Fri, 2010-06-04 at 11:20 -0400, Stephen Smalley wrote:
> On Fri, 2010-06-04 at 10:53 -0400, Mimi Zohar wrote:
> > On Fri, 2010-06-04 at 10:28 -0400, Stephen Smalley wrote:
> > > On Wed, 2010-04-21 at 17:49 -0400, Mimi Zohar wrote:
> > > > EVM depends on the Kernel Key Retention System to provide it with the
> > > > kernel master key for the HMAC operation. The kernel master key is
> > > > securely loaded onto the root's keyring, typically by 'loadkernkey',
> > > > which either uses the TPM sealed secret key, if available, or a
> > > > password requested from the console. To signal EVM, that the key has
> > > > been loaded onto the keyring, 'echo 1 > <securityfs>/evm'. This is
> > > > normally done in the initrd, which has already been measured as part
> > > > of the trusted boot. (Refer to http://linux-ima.sourceforge.net/#EVM.)
> > >
> > > I don't remember this dependency on the kernel key system in prior
> > > incarnations of EVM. Can you explain the rationale for using it, and
> > > the implications?
> >
> > This changed very early on, so that people without a TPM could 'play'
> > with it.
>
> The last time EVM was posted to lsm list (2005, AFAICS) prior to this
> year's round, David Safford said:
> "Since the kernel master key is unsealed by the hardware TPM only as a
> result of a valid trusted boot, and the key is never visible outside the
> kernel, the EVM HMAC attribute cannot be forged in an offline attack."
>
> So that seemed to be an important property to you at the time. I
> understand providing alternatives for TPM-less systems, but that doesn't
> necessarily mean changing the properties on systems with TPMs.
>
In it's earliest form we did have an extension to the tpm device driver
to do the unseal, keeping the resultant key entirely in the kernel.
When IMA and EVM were compiled in, and all initialization was done
within the initrd (which was measured by grub), we thought that doing
the unseal while in the initrd init script which was still single
threaded, was safe enough. (EVM does zero out the key on the ring
after it copies it, so that it is not visible after the boot).
One of our thoughts was to have a common interface for loading the
key, independent of whether it came from a passphrase or TPM.

If you think this is undesirable for TPM based systems, we could
certainly add a securityfs file to evm, through which the init script
could load the encrypted blob. Then evm could have the TPM unseal it
without the key ever being visible outside kernel memory. Alternately
we could simply redefine the contents of the ring evm_key to be the
encrypted blob, and add a line to evm's initialization to unseal the
blob, if there is a tpm.

dave

2010-06-04 18:47:27

by Shaz

[permalink] [raw]
Subject: Re: [PATCH 00/14] EVM

On Fri, Jun 4, 2010 at 8:09 PM, Mimi Zohar <[email protected]> wrote:
> On Fri, 2010-06-04 at 11:53 +0500, Shaz wrote:
>> > Yes, verifying one file containing the hashes would be faster than
>> > verifying individual hashes stored as extended attributes (xattrs), but
>> > this does not take into account that files on a running system are being

What if the sensitive files (binary or data) are compared with IMA
measurements after trusted boot or at anytime a stakeholder wants to?
The comparisons made with IMA will be the sha1 (or ....) of the files
stored in that one verification file. The stakeholder's key determines
which measurements can be compared by her (privacy protection and
confidentiality). Better use this key for an equivalence mechanism for
the factor of performance. The stakeholder's key as an identity can
help to make remote attestation more sensible as well. And here you
will be moving towards TCG MPWG standards .....

Combine this with SELinux or some RBAC mechanism and hopefully you
will get something closer to what MeeGo is trying to achieve. Consider
a trusted package manager with a registry sort of functionality for
files and it's owners and users and you've got a complete solution.

The worst part is that achieving performance is tough, while space is
not a serious issue.

>> > modified or added. On a small form factor, the number of files is
>> > limited, but would this scale well? In addition, what protects that one
>> > file containing all the hashes from being modified? ?So, if you limit


--
Shaz

2010-06-04 20:26:28

by David Safford

[permalink] [raw]
Subject: Re: [ProbableSpam] Re: [PATCH 00/14] EVM

On Fri, 2010-06-04 at 10:57 +1000, James Morris wrote:
> On Tue, 1 Jun 2010, Mimi Zohar wrote:
>
> > SELinux, Smack, Capabilities, and IMA all use extended attributes. The
> > purpose of EVM is to detect offline tampering of these security extended
> > attributes.
>
> One issue mentioned to me off-list is that if EVM is only protecting
> against offline attacks, why not just encrypt the entire volume ?
>
> This would provide confidentiality and integrity protection for all data
> and metadata, rather than just integrity for xattr metadata.

Software whole disk encryption is slower, and doesn't really provide
integrity protection. While there are encryption modes, such as
IAPM (Integrity Aware Parallelizable Mode) which can provide both
confidentially and integrity guarantees, they are hard to use for
random access devices. An example attack is replaying previously
valid encrypted blocks, to revert to a previous version with a known
vulnerabiity. With EVM, these attacks can be defeated
efficiently with directory level binding. (This is on our TODO list).

With encryption in the hard disk, the performance is less of an issue,
but the integrity problem is still there. Plus, most systems don't
have the encrypting drives, and need a software solution.

dave